Bug 18700 – [CAIRO] No pattern-support for SVG

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Dirk Schulze]

There is no pattern-supportfor SVG in Cairo.

[Dirk Schulze]

Created attachment 20773 [details]
Pattern for SVG in Cairo

Adds support for pattern in SVG. It's only a preview. Perhaps there are missing some things. Have to test it a bit.

[Dirk Schulze]

Created attachment 20774 [details]
Pattern for SVG in Cairo

Added pattern-support for stroke.

[Dirk Schulze]

Created attachment 20789 [details]
Pattern for SVG in Cairo

Adds pattern-support for SVG in Cairo.

[Dirk Schulze]

Created attachment 20877 [details]
Pattern for SVG/Cairo

This fixes a problem with referenced patterns (e.g. http://www.w3.org/Graphics/SVG/Test/20061213/htmlEmbedHarness/full-pservers-grad-03-b.html).

[Eric Seidel (no email)]

Comment on attachment 20877 [details]
Pattern for SVG/Cairo

A couple comments:

1. What does it mean to be "too early"?  Comments should explain why, not just what.  That comment sorta does... but the reader doesn't have enough context to know what "too early" means here:
if (!image) // If it's too early we won't have an image yet.

2.  You can be the first platform to support platform extents!
+    cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);

I'm not sure that it would actually work, but you can try. :)  The repeat rule is on the style, you can grab it and turn it into the right cairo enum and it might "just work"

3.  This patch should do at least part of this:
+    // TODO: share this code with other PaintServers

Just move the "applyStrokeStyleToContext()" code out into its own separate funciton.  our CG code does this, you can follow a similar model.  You might not yet use that function from other paint servers yet, but you might as well write that code as a function instead of just inline.

4.  Use a OwnArrayPtr for your dash array instead of manual delete.  Using the scoped pointers is strongly encouraged to make it impossible to leak memory.

5.  Is cairo_pattern_destroy really safe?  If so, that's a very badly named method.  Since you're either assuming that the pattern has already been "copied" into the cairo context, or that _destroy actually means _release (and that cairo_patterns are refcounted).

Also, creating a new pattern every time you paint is rather inefficient.  We try to avoid that for the CG code path.  Maybe it doesn't matter yet for cairo...

r- for the lack of separate function for the applyStrokeStyleToContext code block.

[Eric Seidel (no email)]

Otherwise the patch looked great!  Thanks!

[Dirk Schulze]

Created attachment 20921 [details]
Pattern for SVG/Cairo

Added most important improvements.

point 1 and 4 are fixed.

point 2
Shouldn't be made with this patch, in my opinion.
 
point 3
In Cairo more code can be shared than in Cg, or is it better to share less code?

[Dirk Schulze]

Created attachment 20988 [details]
Pattern for SVG/Cairo

This patch leans on the Cg-implementation. May be better than 20921.

Testcases I used:
*W3C-SVG-1.1
*http://apike.ca/prog_svg_patterns.html
*http://svg.tutorial.aptico.de/grafik_svg/kap11_3.svg
and own SVGs.

[Dirk Schulze]

Created attachment 21230 [details]
Pattern for SVG/Cairo

This patch solves problems with one-dimensional shapes like horizontal or vertical lines, or paths without with or height in the boundingbox.

[Dirk Schulze]

Created attachment 21428 [details]
Pattern for SVG/Cairo

Think it is the best to use the style of the Cg-implementation. The fill operation in the seperate function of https://bugs.webkit.org/attachment.cgi?id=20921 isn't needed by gradients on every call.

[Nikolas Zimmermann]

Comment on attachment 21428 [details]
Pattern for SVG/Cairo

Looks sane to me, r+. Dirk, do you feel like working a bit on the paint server area? We have a few cleanup plans (as well as merging with GrapicsContext!) yet looking for a volunteer...

[Mark Rowe (bdash)]

Landed in r35390.