Summary: | [CAIRO] No pattern-support for SVG | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Schulze <krit> | ||||||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | Keywords: | Cairo | ||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | PC | ||||||||||||||||||||
OS: | Linux | ||||||||||||||||||||
Attachments: |
|
Description
Dirk Schulze
2008-04-23 12:11:26 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.
Created attachment 20774 [details]
Pattern for SVG in Cairo
Added pattern-support for stroke.
Created attachment 20789 [details]
Pattern for SVG in Cairo
Adds pattern-support for SVG in Cairo.
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 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.
Otherwise the patch looked great! Thanks! 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?
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. 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.
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 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...
Landed in r35390. |