Bug 12221 – Webkit ToT fails to render this pattern example

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

[Rob Buis]

The svg doesnt show the pattern correctly, the tiles seem to small.

[Eric Seidel (no email)]

Created attachment 12380 [details]
even simpler example

It looks like we simply don't apply the viewbox transform when drawing.  Rob is in the middle of splitting RenderSVGContainer into two classes (one which establishes viewports and one which can have transforms applied).  Pattern should definitely use the viewport one.

[reid]

Not to sound like a broken record, but as far as I can tell, patterns are broken pretty fundamentally when it comes to multiple viewports and any coordinate remapping which is how they would typically be used IMHO.  I have lots of examples of things that don't work, though perhaps they all boil down to what is illustrated in the attached example already.

In some cases, the rendered results seem totally bizarre and I really cannot make any sense of what is being rendered, i.e. it seems to bear little relationship to the specified SVG.

I'd be happy to try out some example and post results once the fundamentals are working, but I'm not sure if its useful for me to attach more broken examples at this point.  In addition, Grand Paradiso is not exactly doing a spectacular job either, so it is difficult to know with any certainty the correct result.

[Eric Seidel (no email)]

We'll look at this after bug 12207 is solved.

[Nikolas Zimmermann]

Heya!

Just added "reid" to the list of CC. I'm just in the process of fixing patterns and would love to see more "broken examples". I'm hoping "reid" can mail me some examples in private: [e-mail].

Looking forward to get this fixed very soon.
Greetings,
Niko

[Nikolas Zimmermann]

Created attachment 16231 [details]
Layout test results

[Nikolas Zimmermann]

Created attachment 16232 [details]
Initial pattern rework patch

Add viewBox support to patterns, support all combinations of patternUnits/patternContentUnits. Properly support overflow="visible" & take into account stroke-width for patterns.

[Oliver Hunt]

Comment on attachment 16232 [details]
Initial pattern rework patch

Is this safe?
         if (!attributes.hasX() && current->hasAttribute(SVGNames::xAttr))

This seems really bad to me:

+    CGContextSetPatternPhase(contextRef, CGSizeMake(0.0, -0.01));
What happens under heavy scaling?



-            attributes.setX(current->x().valueAsPercentage());
+            attributes.setX(current->x());

[Oliver Hunt]

is
-            attributes.setX(current->x().valueAsPercentage());
+            attributes.setX(current->x());
safe?

[Nikolas Zimmermann]

Yo Olier,

> Is this safe?
>          if (!attributes.hasX() && current->hasAttribute(SVGNames::xAttr))
... 
> 
> -            attributes.setX(current->x().valueAsPercentage());
> +            attributes.setX(current->x());
> 

All of this happens in a while (current) loop, so all is safe.

> +    CGContextSetPatternPhase(contextRef, CGSizeMake(0.0, -0.01));
> What happens under heavy scaling?

This is passed in user-space, and not influenced by scaling.
The comment says it's a crude hack, but it tried some three days to workaround that, though it boils down to a little user-space offset to be applied - the pattern phase - to make it work, as expected.

Greetings,
Niko

[Oliver Hunt]

Comment on attachment 16231 [details]
Layout test results

rs=me for layout test-fu

[Nikolas Zimmermann]

Landed in r25460.