Closed
Bug 949661
Opened 9 years ago
Closed 8 years ago
PathBuilderCG::Arc bug causes rotation transforms applied to circle with stroke-dasharray to be treated as zero degrees at multiples of 90 degrees
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jwatt, Assigned: jwatt)
References
Details
(Keywords: regression)
Attachments
(5 files)
374 bytes,
image/svg+xml
|
Details | |
1.29 KB,
patch
|
Details | Diff | Splinter Review | |
1.06 KB,
text/html
|
Details | |
774 bytes,
patch
|
Details | Diff | Splinter Review | |
2.16 KB,
patch
|
jrmuizel
:
review+
jwatt
:
checkin+
|
Details | Diff | Splinter Review |
Works in Firefox 25. Broken in Nightly. In the testcase the second circle should have a notch at the 6 o'clock position, not the 3 o'clock position.
Comment 1•9 years ago
|
||
WORKSFORME on yesterday's Linux Nightly (i.e. the second circle has a notch at the 6 o'clock position). Mac-only?
Comment 2•8 years ago
|
||
WFM in Linux but can reproduce on Mac with the same build (2014-01-22). So it appears to be OS-specific? I'll find a regression window shortly.
Comment 3•8 years ago
|
||
Last good revision: ce0759a746fb First bad revision: 39f4bb8c55d8 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ce0759a746fb&tochange=39f4bb8c55d8 Jonathan, looks like this is from your own push.
Flags: needinfo?(jwatt)
Keywords: regressionwindow-wanted
![]() |
Assignee | |
Comment 4•8 years ago
|
||
Unbelievably, this was caused by bug 929441 - in other words multiplying the identity matrix onto the gfxContext makes a difference! The change was this: // Apply any stroke-specific transform - aContext->Multiply(GetStrokeTransform(aFrame)); + gfxMatrix strokeTransform = GetStrokeTransform(aFrame); + if (!strokeTransform.IsIdentity()) { + aContext->Multiply(strokeTransform); + }
Blocks: 929441
Flags: needinfo?(jwatt)
![]() |
Assignee | |
Comment 6•8 years ago
|
||
All the old Thebes code is gone from these SVG code paths in Nightly so the snippet in comment 4 isn't so helpful any more. Here's a patch that proves that on Mac calling TransformedCopyToBuilder with the identity transform and using that (in principal identical) Path gets things working. Jeff, any idea what's rotten in the CG Moz2D backend here?
Flags: needinfo?(jmuizelaar)
![]() |
Assignee | |
Comment 7•8 years ago
|
||
![]() |
Assignee | |
Comment 8•8 years ago
|
||
So it seems that HTML canvas manages to avoid this bug because it uses the ArcToBezier Moz2D helper instead of PathSink::Arc. This patch changes HTML canvas to use PathSink::Arc after which the attached canvas test fails.
![]() |
Assignee | |
Updated•8 years ago
|
Summary: Rotation transforms applied to circle with stroke-dasharray treated as zero degrees at multiples of 90 degrees → PathBuilderCG::Arc bug causes rotation transforms applied to circle with stroke-dasharray to be treated as zero degrees at multiples of 90 degrees
Comment 9•8 years ago
|
||
WebKit seems to use CGPathAddArc and works on this test case. For now it might be best to just use the helper.
Flags: needinfo?(jmuizelaar)
Comment 10•8 years ago
|
||
Or even better replace the implementation of PathCG::Arc with a working implemenation that uses the helper.
![]() |
Assignee | |
Comment 11•8 years ago
|
||
Assignee: nobody → jwatt
Attachment #8512304 -
Flags: review?(jmuizelaar)
Updated•8 years ago
|
Attachment #8512304 -
Flags: review?(jmuizelaar) → review+
![]() |
Assignee | |
Comment 12•8 years ago
|
||
Comment on attachment 8512304 [details] [diff] [review] patch https://hg.mozilla.org/integration/mozilla-inbound/rev/1240f21309cd
Attachment #8512304 -
Flags: checkin+
![]() |
Assignee | |
Comment 13•8 years ago
|
||
Failures in layout/base/tests/test_getBoxQuads_convertPointRectQuad.html caused by rounding errors due to this change meant that it was necessary to fix bug 1090211 in order to land this.
Depends on: 1090211
Comment 14•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1240f21309cd
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•7 years ago
|
QA Whiteboard: [good first verify]
You need to log in
before you can comment on or make changes to this bug.
Description
•