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

Canvas2D ellipse() specs seem to be broken #8408

Closed
Kaiido opened this issue Oct 20, 2022 · 2 comments
Closed

Canvas2D ellipse() specs seem to be broken #8408

Kaiido opened this issue Oct 20, 2022 · 2 comments

Comments

@Kaiido
Copy link
Member

Kaiido commented Oct 20, 2022

[Originally reported at StackOverflow by user flupe.]

The ellipse-method-steps seem to be broken.
It looks like a monkey-patch over the old dom-context-2d-arc method definition, with some leftovers that kind of break the algorithm.

For instance, in the step 4 it says

If [...] startAngle-endAngle is equal to or greater than 2π, then the arc is the whole circumference of this ellipse, and the point at startAngle along this circle's circumference, measured in radians clockwise from the ellipse's semi-major axis, acts as both the start point and the end point.

Otherwise, the points at startAngle and endAngle along this circle's circumference, measured in radians clockwise from the ellipse's semi-major axis, are the start and end points respectively, and the arc is the path along the circumference of this ellipse from the start point to the end point

There is no previous mention of any "circle". So one could think that it's simply a typo and that it should be read "this ellipse" instead. But that doesn't seem quite correct either. These points aren't the ones at the angle startAngle or endAngle on the circumference of the ellipse, they are the points at the "eccentric angle" startAngle or endAngle. These angles do only match for actual circles, not for other ellipses.

Implementations do agree on what to do here, only the specs don't reflect what the implementations are actually doing.

I'm not quite sure how to make the actual algorithm in prose, so if someone from @whatwg/canvas (maybe @junov?) could give it a shot or at least provide some directions, that'd be great.

@annevk
Copy link
Member

annevk commented Oct 20, 2022

Reusing language from https://svgwg.org/svg2-draft/shapes.html#EllipseElement might make sense here.

@Kaiido
Copy link
Member Author

Kaiido commented Oct 20, 2022

<ellipse> is always a whole ellipse, i.e it doesn't have to deal with startAngle and endAngle.
The Arc command would have been a better candidate, but that one uses "endpoint parameterization", while canvas2D uses "center parametrization", see this note. I don't think we can reuse much from it to describe what's really done in canvas.

Kaiido added a commit to Kaiido/html that referenced this issue Nov 11, 2022
This is an attempt at fixing whatwg#8408.
This goes the long way of defining the steps to determine the point on the ellipse, in a "theoretical" wording (as opposed to using algebra formulae).
This is based on what browsers seems to do.  so there shouldn't be a need for browser issues, yet a technical review from @whatwg/canvas is probably necessary.
Not sure either if editorially this is the proper way of fixing that.
teoli2003 added a commit to mdn/content that referenced this issue Feb 24, 2023
* Fix ellipse angles definition

As was found in whatwg/html#8408 the ellipse's `startAngle` and `endAngle` are not really angles on the ellipse, but angles on the eccentric circle of the ellipse.   
These are known as "eccentric angle", though this term *might* be a bit obscure at first sight, so I wouldn't mind a better proposition if reviewers have one.  
The specs got fixed in whatwg/html#8495 but as can be seen, the fix was quite verbose and I'm not sure the docs need to get that much details.  
So I hope this little fix is both enough to lead the ones that could get surprised by the actual behavior to the proper understanding, and not too obscure to lose in details the ones that wouldn't notice.

* Link "eccentric angle" to outside explanation

Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>

* Should have batched...

Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>

---------

Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants