Bug 18700

Summary: [CAIRO] No pattern-support for SVG
Product: WebKit Reporter: Dirk Schulze <krit>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: Cairo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Pattern for SVG in Cairo
none
Pattern for SVG in Cairo
none
Pattern for SVG in Cairo
none
Pattern for SVG/Cairo
eric: review-
Pattern for SVG/Cairo
none
Pattern for SVG/Cairo
none
Pattern for SVG/Cairo
none
Pattern for SVG/Cairo zimmermann: review+

Description Dirk Schulze 2008-04-23 12:11:26 PDT
There is no pattern-supportfor SVG in Cairo.
Comment 1 Dirk Schulze 2008-04-23 12:23:49 PDT
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.
Comment 2 Dirk Schulze 2008-04-23 12:58:27 PDT
Created attachment 20774 [details]
Pattern for SVG in Cairo

Added pattern-support for stroke.
Comment 3 Dirk Schulze 2008-04-24 01:18:25 PDT
Created attachment 20789 [details]
Pattern for SVG in Cairo

Adds pattern-support for SVG in Cairo.
Comment 4 Dirk Schulze 2008-04-28 13:04:45 PDT
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).
Comment 5 Eric Seidel (no email) 2008-04-30 23:15:32 PDT
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.
Comment 6 Eric Seidel (no email) 2008-04-30 23:16:00 PDT
Otherwise the patch looked great!  Thanks!
Comment 7 Dirk Schulze 2008-05-02 05:35:31 PDT
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?
Comment 8 Dirk Schulze 2008-05-06 12:22:24 PDT
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.
Comment 9 Dirk Schulze 2008-05-19 12:04:09 PDT
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.
Comment 10 Dirk Schulze 2008-05-30 03:37:01 PDT
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.
Comment 11 Nikolas Zimmermann 2008-07-16 14:59:56 PDT
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...
Comment 12 Mark Rowe (bdash) 2008-07-26 22:38:36 PDT
Landed in r35390.