Bug 23472

Summary: SVG pattern transformation/BoundingBox can cause ugly stroke thickness or text positions
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://svg.tutorial.aptico.de/grafik_svg/kap11_3.svg
Attachments:
Description Flags
Transformed pattern on text
none
scaled patttern on stroke
none
transform the pattern instead of the context
none
transform the pattern instead of the context zimmermann: review+

Description Dirk Schulze 2009-01-21 23:49:25 PST
The translation of the pattern causes a offset of the left exclamation mark. This is caused by the transformation of the context.
Paths are not transformed by changing the CTM. We should make something similuar for texts, or change the way we transform patterns.
Comment 1 Dirk Schulze 2009-01-28 03:39:54 PST
Patterns have the same problem as the REGRESSION on gradients (https://bugs.webkit.org/show_bug.cgi?id=23547). The transformation of the pattern is applied to the CTM. This causes strange transformations of the text, or unwanted stroke thikness, depending on the transformation.
Stroke thickness is only influenced by scaling skewing or a transformationmatrix that scales or skewes.

We need a general redesign here, to get both fixed, gradients as well as patterns and to minimize code snippets for both.
Comment 2 Dirk Schulze 2009-01-28 03:40:42 PST
Created attachment 27099 [details]
Transformed pattern on text
Comment 3 Dirk Schulze 2009-01-28 03:42:01 PST
Created attachment 27100 [details]
scaled patttern on stroke

The stroke should be small line.
Comment 4 Dirk Schulze 2009-01-28 12:28:39 PST
Created attachment 27115 [details]
transform the pattern instead of the context

Just transform the pattern instead of the context. Texts and strokes are no longer affected by pattern transformations.
Comment 5 Nikolas Zimmermann 2009-01-29 12:15:51 PST
Looks fine to me, after a lengthy discussion on IRC. Eric should have a final look.
Some namings need to change, Pattern::setTransform, should get a better name (so it's visible it's a pattern-space transformation), and 'patternMatrix' should be renamed to sth like 'userSpaceTransformation' - to not confuse anyone with two matrices with nearly equal names.



Comment 6 Dirk Schulze 2009-01-29 14:48:06 PST
Created attachment 27163 [details]
transform the pattern instead of the context

renamed components
Comment 7 Nikolas Zimmermann 2009-01-29 15:22:16 PST
Comment on attachment 27163 [details]
transform the pattern instead of the context

Looks nice, some comments:
> +        // Pattern space is an abstract space that maps to the default user space by the transformation matrix 
> +        // you specify with the userSpaceTransformation parameter.
> +        PlatformPatternPtr createPlatformPattern(const TransformationMatrix& userSpaceTransformation) const;

I'd shorten the comment:
> +        // Pattern space is an abstract space that maps to the default user space by the transformation 'userSpaceTransformation' 

Just leave out the comment for setPatternSpaceTransform, and make the argument 'const TransformationMatrix&'.
> +        void setPatternSpaceTransform(TransformationMatrix patternSpaceTransformation) { m_patternSpaceTransformation = patternSpaceTransformation; }

r=me with thse fixups.
Comment 8 Dirk Schulze 2009-01-29 23:29:47 PST
landed in r40379