949661 - PathBuilderCG::Arc bug causes rotation transforms applied to circle with stroke-dasharray to be treated as zero degrees at multiples of 90 degrees

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

Attachment: rotate.svg

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

[Daniel Holbert [:dholbert] (blocking needinfo/review, due to AFK on parental leave 12/9 - 1/6)]

WORKSFORME on yesterday's Linux Nightly (i.e. the second circle has a notch at the 6 o'clock position).

Mac-only?

Comment 2

[Logan Rosen [:Logan]]

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

[Logan Rosen [:Logan]]

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.

Comment 4

[Jonathan Watt [:jwatt]]

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);
+  }

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: proof of concept fix

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?

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: canvas test which only fails with the upcoming patch

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: patch to CanvasRenderingContext2D.cpp to update it to use PathSink::Arc()

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.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

WebKit seems to use CGPathAddArc and works on this test case. For now it might be best to just use the helper.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Or even better replace the implementation of PathCG::Arc with a working implemenation that uses the helper.

Comment 11

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 8512304 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/1240f21309cd

Comment 13

[Jonathan Watt [:jwatt]]

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.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1240f21309cd