1860763 - Offsetting start and end angle in canvas arc() causes position offset

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[jp]

Attachment: firefox_arc_bug_demo.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36

Steps to reproduce:

I was drawing a doughnut chart in a canvas element using the arc function. I was also adding an offset to the start and end angle arguments to not begin drawing the chart segments from 0.

Actual results:

When the angles were set to 2, and 2 + 2Math.PI, i.e. one segment that filled the entire chart and the segment started from 2 radians, the entire circle was moved a bit to the top left.
When the angles were instead set to 5, and 5 + 2Math.PI, the circle then instead moved over to the bottom right.

Expected results:

I expected the circle to stay in the same position in the canvas regardless of the angles I used as args in the arc function.
Using 1.9 for example moved the circle back to the expected position.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: Canvas2D' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

Can repro with D2d canvas.
Regression range:
Bug 1847149. Make PathBuilderD2D::Arc handle all start and end angles properly. r=lsalzman

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

:tnikkel, since you are the author of the regressor, bug 1847149, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1847149

Comment 5

[Timothy Nikkel (:tnikkel)]

This isn't so much a regression from bug 1847149 as it is from bug 1839470. This bug also exists after landing bug 1839470 (but before it was undone in bug 1846613). So the new code added in bug 1847149 is not to blame.

We seem to be hitting the underlying d2d when drawing full circles as mentioned in the code here

https://searchfox.org/mozilla-central/rev/aa8a99510c0686cdf9d42fb4b8f6d968884c961d/gfx/2d/PathD2D.cpp#188

However that workaround does not catch this case because the difference between the start and end angles is just slightly below 2*pi (due to floats only able to be accurate to a point). I think the fix here is to allow a small amount of leeway in detecting the full circle case.

Comment 6

[Timothy Nikkel (:tnikkel)]

I got a patch and a test for this now, just gotta upload it.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1860763. Expand workaround for d2d arcs that are full circles slightly. r?#gfx-reviewers

The other check for (close to) full circle arcs lower down in this function

https://searchfox.org/mozilla-central/rev/01a0d864a9442d0fe2dbd4beee5c88b9b46e96bd/gfx/2d/PathD2D.cpp#221

does not catch the testcase from this bug: the start and end points are just barely far enough away.

It only makes sense that if we are changing all arcs >= 2Pi to 1.9999Pi that we should also change arcs between 1.9999Pi and 2Pi, and that fixes this bug.

I also added some instructions to the wpt canvas tests gentest.py file, which I didn't find anywhere else.

Comment 8

[Timothy Nikkel (:tnikkel)]

I also modified to testcase to use requestAnimationFrame and checking increasing offsets, and random offsets to make sure this fix was robust.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b73ef4c8979f
Expand workaround for d2d arcs that are full circles slightly. r=gfx-reviewers,lsalzman

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42881 for changes under testing/web-platform/tests

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b73ef4c8979f

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1860763. Expand workaround for d2d arcs that are full circles slightly. r?#gfx-reviewers

The other check for (close to) full circle arcs lower down in this function

https://searchfox.org/mozilla-central/rev/01a0d864a9442d0fe2dbd4beee5c88b9b46e96bd/gfx/2d/PathD2D.cpp#221

does not catch the testcase from this bug: the start and end points are just barely far enough away.

It only makes sense that if we are changing all arcs >= 2Pi to 1.9999Pi that we should also change arcs between 1.9999Pi and 2Pi, and that fixes this bug.

I also added some instructions to the wpt canvas tests gentest.py file, which I didn't find anywhere else.

Original Revision: https://phabricator.services.mozilla.com/D192414

Comment 14

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: expand work around for underlying platform bug by 0.0001
• Risk associated with taking this patch: low
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• String changes made/needed: no
• Is Android affected?: no
• Code covered by automated testing: yes
• User impact if declined: canvas arc drawing bug

Comment 15

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9361392 [details]
Bug 1860763. Expand workaround for d2d arcs that are full circles slightly. r?#gfx-reviewers

Approved for 120.0b6

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/3de35294e81f