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

Grid border not closed at left-top corner (no z in SVG path) #15023

Closed
mindon opened this issue May 24, 2021 · 5 comments · Fixed by ecomfe/zrender#767
Closed

Grid border not closed at left-top corner (no z in SVG path) #15023

mindon opened this issue May 24, 2021 · 5 comments · Fixed by ecomfe/zrender#767
Labels
bug difficulty: easy Issues that can be fixed more easily than the average. en This issue is in English SVG
Milestone

Comments

@mindon
Copy link

mindon commented May 24, 2021

Version

v5.1.1

Reproduction link

https://qp9zl.csb.app/

Steps to reproduce

render with option

const option = {
        grid: {
          show: true,
          borderWidth: 3,
          borderColor: "#000000"
        }
      };

What is expected?

corner closed

What is actually happening?

corner not closed

@echarts-bot
Copy link

echarts-bot bot commented May 24, 2021

Hi! We've received your issue and please be patient to get responded. 🎉
The average response time is expected to be within one day for weekdays.

In the meanwhile, please make sure that it contains a minimum reproducible demo and necessary images to illustrate. Otherwise, our committers will ask you to do so.

A minimum reproducible demo should contain as little data and components as possible but can still illustrate your problem. This is the best way for us to reproduce it and solve the problem faster.

You may also check out the API and chart option to get the answer.

If you don't get helped for a long time (over a week) or have an urgent question to ask, you may also send an email to dev@echarts.apache.org. Please attach the issue link if it's a technical question.

If you are interested in the project, you may also subscribe our mailing list.

Have a nice day! 🍵

@echarts-bot echarts-bot bot added bug en This issue is in English pending We are not sure about whether this is a bug/new feature. waiting-for: community labels May 24, 2021
@plainheart plainheart added SVG and removed pending We are not sure about whether this is a bug/new feature. waiting-for: community labels May 24, 2021
@plainheart
Copy link
Member

I'm not sure why this commit ecomfe/zrender@da94b64 removed ctx.closePath(). @pissang

@pissang
Copy link
Contributor

pissang commented May 25, 2021

@plainheart I thought the first point and the last point has been connected in the roundRect.ts helper so there is no need to do closePath again, which is an extra cost in the case that drawing lots of rect(like in benchmark.html). But it seems to be a mistake in this case.

Also, only the SVG renderer that drawing a rectangle without border-radius will have this issue. Because canvas renderer will use ctx.rect command, which is closed underlying automatically. And drawing rectangle with border-radius also has no issue because there is an arc command connecting the first point and last point, which is like:

image

So I think what we need is adding an Z command when rendering rect command in the SVG renderer https://github.com/ecomfe/zrender/blob/9780bd81795010b7e99f77b907c7530552184942/src/svg/graphic.ts#L234
It can keep both performance in the canvas and correctness in the svg

@pissang pissang added this to the 5.2.0 milestone May 25, 2021
@pissang pissang added the difficulty: easy Issues that can be fixed more easily than the average. label May 25, 2021
@echarts-bot
Copy link

echarts-bot bot commented May 25, 2021

This issue is labeled with difficulty: easy.
@mindon Would you like to debug it by yourself? This is a quicker way to get your problem fixed. Or you may wait for the community to fix.

Please have a look at How to debug ECharts if you'd like to give a try. 🤓

@plainheart
Copy link
Member

@pissang Yes. I agree with you. That's right.

plainheart added a commit to ecomfe/zrender that referenced this issue May 28, 2021
1) fix apache/echarts#15023, rect path can't be closed.
2) fix apache/echarts#15029 normalize color when using SVG renderer to support more cases, for example, some tools/platforms can't recognize the alpha in color.
3) fix eslint error about `for-in`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug difficulty: easy Issues that can be fixed more easily than the average. en This issue is in English SVG
Projects
None yet
3 participants