Bug 23547 – REGRESSION: SVG gradient transformation/BoundingBox can cause ugly stroke thickness

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Dirk Schulze]

If we are in boundingBoxmode, the context is transformed to match the gradient to the shape that is filled with it.
We do the same for stroking. To get the gradient match the shape, we have to translate and scale it. Like mentioned above, we do it with transforming the context. The path is not effected by that.
But on stroking, the strokeThickness is scaled too. And this can cause different strokethikness from top to bottom or left to right.

[Dirk Schulze]

Created attachment 27040 [details]
stroke a rect with a gradient in boundingBoxMode

[Dirk Schulze]

Created attachment 27043 [details]
real boudingBox mode example

the example above was the example with a gradientTransform attribute. This one is with the boundingBox

[Dirk Schulze]

*** Bug 22956 has been marked as a duplicate of this bug. ***

[Dirk Schulze]

There was some confusion about this bug.

To the examples: The stroke thickness of each side should be identical.

Complain the example:
We have a gradient in boundinBoxMode. This means the gradient has a dimension of 1 to 1 (width to height).
And we have a rect with the dimension of 300 to 100.
Now we want to stroke the rect with the gradient. That means the gradient size must match the size of the rect with stroke width.
To do that we have to scale the gradient by 300 for width and 100 for height.
The current code just transforms the context, by scaling the context. The path (our rect) is not affected by this scaling.
After that we stroke the path with the gradient.

But the scaling of the context affects the stroke thickness. Once the scaling of width and height have differnet dimensions, the stroke thikness differs on the top to bottom and left to right.

This wrong behavior was integrated in r38952.

The important difference between the old and the new code is the handling of paths. The old code calls clipToStrokePath(contextRef, path) right before the transformation of the context. This creates a new path, the stroke path. After the transformation, the path is filled and not stroked. Thats why the old code works unlike the new one. The stroke path isn't affected by the transformation.

How to fix it:
There are two ways to fix this issue. 

1. only transform the gradient instead of the whole context. This way the context isn't touched and we have no problems with stroke thickness. We need to implement a new method on Gradient.cpp applyTransformation. This would apply the transformation during creating the gradient or afterwards. This will work for Qt, Cairo, Skia. Not sure about CG

2. Use the old variant and create a new stroke path. This works CG,Qt but won't work for cairo. cairo_stroke_to_path wasn't implemented yet.

Maybe we can use both variants, depending on the platform.

[Dirk Schulze]

Created attachment 27195 [details]
transform the gradient instead of the context

This patch works for Cairo, Qt, Skia but I couldn't get gradients transformed in CG. Tried to transform the context, creating the CGShadingRef (gradient) and restore the context before filling the path, without success. I haven't found a way to transform the CGShadingRef with a TransformationMatrix directly.

This patch fails on one pixeltest, that we _nearly_ pass with the incorrect patch, but don't in last Safari. 
This bug fix passes all other tests now.

[Dirk Schulze]

Created attachment 27492 [details]
ransform the gradient instead of the context with CG-fix

same patch as above, but with CG fix.

[Eric Seidel (no email)]

Comment on attachment 27492 [details]
ransform the gradient instead of the context with CG-fix

Looks good.  r=me assuming it passes the pixel tests.

[Dirk Schulze]

landed in r40795.