Bug 5361 – Pattern element coordinate problems

• Component: SVG
• Status: RESOLVED FIXED

[Nicholas Shanks]

At the URL given, in Opera 8.5, the character talking in the first frame has a beard, which isn't drawn in 
Safari.

[Nicholas Shanks]

Actually, both of them have missing stubble. It might be that it's supposed to have an alpha value, but 
Opera is rendering it as 1.0 (black) instead of grey, and Safari is rendering it as 0.0 (white).

[Nicholas Shanks]

Created attachment 4336 [details]
reduced test‐case

The bug is when a fill attribute refers to a <pattern> element by fragment ID
using "url(#fragment)"

[Eric Seidel (no email)]

Adding NeedsReduction keyword, the test case ideally should only be a couple lines.

[Julien Palmas]

Created attachment 4343 [details]
Reduced test case

There are 2 know bugs in
http://www.pandimaniacs.com/index.php?mode=svg&lang=en.

The first one is related with the use of viewBox. We really have to fix the
coordinates calculation when using this attribute as many SVGs use this.

The second bug (see the atteched reduced test case) is a coordinate generation
problem as well, but when using the pattern element. In this example, the first
(top-left) red <rect> should be rendered at (10.10) but it is rendered at
(0.0).

NOTE : the patternUnits="userSpaceOnUse" is important in this example. It means
that the coordinate system used for the pattern element (ie: for its x, y,
width and height values) is the same as the coordinate system of the black
rectangle.

[Julien Palmas]

Created attachment 4344 [details]
Wrong result

[Julien Palmas]

Created attachment 4345 [details]
Good result

[Julien Palmas]

MacDome, could you set the keyword to "HasReduction"
and the summary to something like "Pattern element coordinate problem" ?

Thanks

[Julien Palmas]

Created attachment 4438 [details]
Proposed patch

KRenderingPaintServerPatternQuartz was checking boundingBoxMode() of
KRenderingPaintServerPattern to shift the coordinates of the <pattern>.

That's not necessary as the x() and y() coordinates are already shifted thanks
to the baseVal() method.

By the way, the same code was already commented out  a line 135

[Julien Palmas]

Created attachment 4439 [details]
Better patch

Removes unnecessary comments

[Eric Seidel (no email)]

Comment on attachment 4438 [details]
Proposed patch

We don't generally commit commented out code.

[Eric Seidel (no email)]

Comment on attachment 4439 [details]
Better patch

Looks fine, I'll test again before committing.

[Eric Seidel (no email)]

Fixed.