Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix (stackY): Fix the point offset when set stackY. #6523

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

Runtus
Copy link
Contributor

@Runtus Runtus commented Nov 18, 2024

Checklist
  • npm test passes
  • benchmarks are included
  • commit message follows commit guidelines
  • documents are updated
Description of change
  • 修复stackY生效时,散点位移和实际不重合的情况。
  • 经过源码阅读发现,是因为散点图的底层渲染会取y和y1的均值作为最终的y,响应的point响应逻辑代码在此。所以导致他实际出来的效果会发生偏移,为了让改动最小化,所以直接在stackY的渲染代码中做了判断处理。
image

fixed: #6506

@hustcc hustcc requested a review from pearmini November 18, 2024 15:33
@pearmini
Copy link
Member

pearmini commented Nov 18, 2024

@Runtus 优秀!不过可以看看这个 #4515 ,这样改分箱散点图会有问题吗?

@Runtus
Copy link
Contributor Author

Runtus commented Nov 19, 2024

@Runtus 优秀!不过可以看看这个 #4515 ,这样改分箱散点图会有问题吗?

好嘞,我这边看看

@Runtus
Copy link
Contributor Author

Runtus commented Nov 19, 2024

@Runtus 优秀!不过可以看看这个 #4515 ,这样改分箱散点图会有问题吗?

我刚刚看了下, #4515 展示没啥问题,因为其中代码指定了把y1指定为y的通道,原理其实和我改的地方一样,就是让散点图只有一个y通道,防止取均值。
image
关于分箱散点图,刚刚去测试了下,不会有影响,它的transform走的是bin,我只改了stackY的渲染逻辑,所以不会有影响。

@Runtus
Copy link
Contributor Author

Runtus commented Nov 19, 2024

@pearmini 对了,再麻烦老哥看看CI报错,我在本地跑和这里跑是一样的错误,不知道是不是某个依赖包更新导致的CI跑挂了

@pearmini
Copy link
Member

@Runtus 那挺好的!虽然在 transform 里面判断 mark 类型不是很推荐,但是这样对用户来讲确实是更好滴。CI 问题可能需要 @hustcc 看看了,是不是底层依赖升级了?

Comment on lines -102 to +117
y0: inferredColumn(Y, fy), // Store original Y.
y: column(V, fy),
y1: column(V1, fy1),
...newEncode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里还有一个种写法,仅供参考:

encode: {
  y0: inferredColumn(Y, fy), // Store original Y.
  y: column(V, fy),
  ...mark.type === 'point' && { y1: column(V1, fy1) }
}

@hustcc
Copy link
Member

hustcc commented Nov 19, 2024

@pearmini 对了,再麻烦老哥看看CI报错,我在本地跑和这里跑是一样的错误,不知道是不是某个依赖包更新导致的CI跑挂了

ci 统一修复~

@hustcc hustcc changed the title transform(stackY): Fix the point offset when set stackY. fix (stackY): Fix the point offset when set stackY. Nov 19, 2024
@hustcc hustcc merged commit 5d8810c into antvis:v5 Nov 19, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Points on the chart are not displayed correctly if stackY is enabled
3 participants