Closed Bug 1841322 Opened 2 years ago Closed 2 years ago

Failing WPTs in html/canvas/offscreen/path-objects/2d.path.stroke.prune.[arc|curve|line][.worker].html

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: twisniewski, Unassigned)

References

Details

Summary: Failing WPTs in html/canvas/offscreen/path-objects/2d.path.stroke.prune.[arc|curve|][.worker].html → Failing WPTs in html/canvas/offscreen/path-objects/2d.path.stroke.prune.[arc|curve|line][.worker].html

As per bug 691187 and bug 1837729, these tests are broken and out of date with the spec. They violate behavior concerning the proper handling of end-caps, see bug 1837729 comment 6.

Severity: -- → S4
Status: NEW → RESOLVED
Closed: 2 years ago
Depends on: 691187, 1837729
Resolution: --- → INVALID
Resolution: INVALID → WONTFIX

:lsalzman, should we at least file an issue on the WPT repo to fix or remove the tests? Or file bugs against Chromium and WebKit?

Flags: needinfo?(lsalzman)

It would be best to ask whoever is in charge of raising WPT test issues. I am not really involved with that.

Flags: needinfo?(lsalzman)
Flags: needinfo?(james)

(In reply to Lee Salzman [:lsalzman] from comment #3)

It would be best to ask whoever is in charge of raising WPT test issues. I am not really involved with that.

I'm also needinfo'ing Jonathan Kew since he filed the PR to change two html/canvas/offscreen/text/2d.text.measure.advances.* tests.

Thanks for flagging this, Tom!

Flags: needinfo?(jfkthame)

To be precise, I filed an issue suggesting that a couple of the tests should be dropped: see https://github.com/web-platform-tests/interop/issues/365. A good first step here would be to open a similar issue regarding these path tests, pointing out where they deviate from the spec.

I haven't opened a PR to do the actual removal of tests from the Interop-2023 list yet. It looks like the way to do that is to open a PR in https://github.com/web-platform-tests/wpt-metadata to modify the metadata for the relevant directory. I'm away for a few days this week, but will look at doing that when I'm back, if the issue hasn't been addressed by then.

Alternatively, rather than removing these path tests from the list, we could correct them to match the current spec. That can be done by fixing them within mozilla-central (and then the WPT sync bot will merge them) or directly by opening a PR in https://github.com/web-platform-tests/wpt. (Either way, note that many of these tests are generated by scripts rather than maintained as individual test files, so to update them the proper approach is to modify the manifest and re-run the generator script.)

Flags: needinfo?(jfkthame)

Yes, what jfkthame says is correct. The only addition I'd make is that if we're planning to modify the pass condition and the tests are in Interop we should still file a test change proposal as a heads-up for the other vendors (it's not really reasonable to object to a bugfix, but we don't want people to be surprised if they see new failures).

Flags: needinfo?(james)

Looking into the canvas spec here, it is unfortunately not really clear. On the one hand, the beginning of the trace a path algorithm:

  1. Let path be a copy of the path being traced.
  2. Prune all zero-length line segments from path.
  3. Remove from path any subpaths containing no lines (i.e. subpaths with just one point).

would clearly eliminate a path such as

  ctx.beginPath();
  ctx.moveTo(50, 25);
  ctx.lineTo(50, 25);
  ctx.stroke();

entirely, leaving nothing to put end-caps on; but later in step 9 it specifically mentions "points with no lines coming out of them", which seems to be intended to address examples like this.

Turns out there's been an open spec issue about this since 2016....

(In reply to Lee Salzman [:lsalzman] from comment #1)

As per bug 691187 and bug 1837729, these tests are broken and out of date with the spec. They violate behavior concerning the proper handling of end-caps, see bug 1837729 comment 6.

Not sure this is the final word on the subject..... that comment references https://bugs.chromium.org/p/chromium/issues/detail?id=577655, but more recently in https://bugs.chromium.org/p/chromium/issues/detail?id=644067, they've reverted the behavior to no longer paint caps for zero-length lines.

So it looks like in 2016 there was an intention to change this; Chrome 52 aligned its behavior with ours, and with the proposed spec revision; but the canvas spec never actually got updated, and now they've restored the old behavior that the spec (and tests) currently expect.

Interesting stuff... maybe the tests should just be removed from the interop2023 list for now?

I've added a comment on the Blink issue where this was recently changed, questioning whether that was the right thing to do; curious to see what response, if any, we get to that.

None of the 6 tests linked in comment 0 have the interop-2023 tag anymore. It seems jfkthame successfully proposed that they be removed in
https://github.com/web-platform-tests/interop/issues/392

--> Dropping the association with the interop-2023-offscreencanvas metabug.

No longer blocks: interop-2023-offscreencanvas
See Also: → https://github.com/web-platform-tests/interop/issues/392
You need to log in before you can comment on or make changes to this bug.