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

[offscreen-canvas] Remove 2d.path.stroke.prune.* tests for open paths from interop-2023 #392

Closed
jfkthame opened this issue Aug 10, 2023 · 11 comments
Labels
focus area: Offscreen Canvas test-change-proposal Proposal to add or remove tests for an interop area

Comments

@jfkthame
Copy link

Test List

https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.arc.html
https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.arc.worker.html
https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.curve.html
https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.curve.worker.html
https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.line.html
https://wpt.fyi/results/html/canvas/offscreen/path-objects/2d.path.stroke.prune.line.worker.html

Rationale

The behavior of 0-length subpaths with line-caps is subject to a longstanding canvas2d spec issue that has never been resolved, although discussion there appeared to converge on a consensus that would align behavior with SVG rendering and would not match the existing WPT tests here.

Given that the existing spec text is confusing/ambiguous, and discussion in the open spec issue favored a different result than the existing tests expect, I think we should remove these tests from the interop-2023 set until such time as there's a clear resolution of the spec issue.

@jfkthame jfkthame added the test-change-proposal Proposal to add or remove tests for an interop area label Aug 10, 2023
@jfkthame jfkthame changed the title [offscreen-canvas] [offscreen-canvas] Remove 2d.path.stroke.prune.* tests for open paths Aug 10, 2023
@jfkthame jfkthame changed the title [offscreen-canvas] Remove 2d.path.stroke.prune.* tests for open paths [offscreen-canvas] Remove 2d.path.stroke.prune.* tests for open paths from interop-2023 Aug 10, 2023
@nt1m
Copy link
Member

nt1m commented Aug 17, 2023

Seems ok

jfkthame added a commit to web-platform-tests/wpt-metadata that referenced this issue Aug 17, 2023
See web-platform-tests/interop#392 for details. These tests relate to a longstanding open spec issue, and should not be included in interop-2023 as long as that is unresolved.
@chrishtr
Copy link
Contributor

@fserb can you review for Chromium?

@fserb
Copy link

fserb commented Aug 30, 2023

lgtm

nt1m pushed a commit to web-platform-tests/wpt-metadata that referenced this issue Sep 5, 2023
See web-platform-tests/interop#392 for details. These tests relate to a longstanding open spec issue, and should not be included in interop-2023 as long as that is unresolved.
@nt1m nt1m closed this as completed Sep 5, 2023
@yiyix
Copy link

yiyix commented Sep 11, 2023

This misses the 4 following tests. Could we remove them from interop as well?
2d.path.stroke.prune.closed.html
2d.path.stroke.prune.closed.worker.html
2d.path.stroke.prune.rect.html
2d.path.stroke.prune.rect.worker.html

@jfkthame
Copy link
Author

Is there ambiguity about how those should behave? The others were about unclosed paths, where there may potentially be line-caps involved, but that's not an issue for closed paths.

@yiyix
Copy link

yiyix commented Sep 11, 2023

Hi, I am very new to the Path spec. Please help me to understand the spec for closed curves. In the spec, https://html.spec.whatwg.org/multipage/canvas.html#trace-a-path, item 5 says

Add a straight closing line to each closed subpath in path connecting the last point and the first point of that subpath; change the last point to a join (from the previously last line to the newly added closing line), and change the first point to a join (from the newly added closing line to the first line).

since the start and the end is the same point, it basically added a subpath of length 0. Will the line caps be drawn in this case?

@jfkthame
Copy link
Author

I don't think it would ever make sense to draw line-caps for a closed subpath (whatever its length). Caps go on open ends of a path. A closed subpath doesn't have open ends, so it doesn't get caps.

@Kaiido
Copy link

Kaiido commented Sep 11, 2023

Caps go on each lines of the dash strokes, so you can have these even on closed pathes, e.g. when the dash lines are smaller than the path's total length. What happens for zero length closed pathes is relatively unclear when you don't prune them at the beginning. Note that SVG does render the caps in this case.

@jfkthame
Copy link
Author

Yes, caps would go on dash strokes. But intuitively, it seems to me that if the path length is zero, then there would be no instances of the dash, and hence no caps either.

@Kaiido
Copy link

Kaiido commented Sep 11, 2023

I'm not saying your reasoning isn't sound, but I fear that both interpretations are, and that would maintain an interop issue with SVG which I believe is against most views expressed in the issue there.
So if the tests aren't passing everywhere today I guess it's fine to remove these too, along with the other problematic ones, until it's all sorted out (though I'd personally rather prefer to see it sorted out quickly...)

@yiyix
Copy link

yiyix commented Sep 18, 2023

nt1m could you review the change web-platform-tests/wpt-metadata#4775? Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: Offscreen Canvas test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

No branches or pull requests

6 participants