Bug 5361

Summary: Pattern element coordinate problems
Product: WebKit Reporter: Nicholas Shanks <nickshanks>
Component: SVGAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.pandimaniacs.com/index.php?mode=svg&lang=en
Attachments:
Description Flags
reduced test&#8208;case
none
Reduced test case
none
Wrong result
none
Good result
none
Proposed patch
eric: review-
Better patch eric: review+

Description Nicholas Shanks 2005-10-13 01:35:58 PDT
At the URL given, in Opera 8.5, the character talking in the first frame has a beard, which isn't drawn in 
Safari.
Comment 1 Nicholas Shanks 2005-10-13 01:38:43 PDT
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).
Comment 2 Nicholas Shanks 2005-10-13 02:09:33 PDT
Created attachment 4336 [details]
reduced test&#8208;case

The bug is when a fill attribute refers to a <pattern> element by fragment ID
using "url(#fragment)"
Comment 3 Eric Seidel (no email) 2005-10-13 02:25:39 PDT
Adding NeedsReduction keyword, the test case ideally should only be a couple lines.
Comment 4 Julien Palmas 2005-10-13 08:28:26 PDT
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.
Comment 5 Julien Palmas 2005-10-13 08:29:29 PDT
Created attachment 4344 [details]
Wrong result
Comment 6 Julien Palmas 2005-10-13 08:29:49 PDT
Created attachment 4345 [details]
Good result
Comment 7 Julien Palmas 2005-10-13 08:33:16 PDT
MacDome, could you set the keyword to "HasReduction"
and the summary to something like "Pattern element coordinate problem" ?

Thanks
Comment 8 Julien Palmas 2005-10-21 10:36:16 PDT
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
Comment 9 Julien Palmas 2005-10-21 11:38:58 PDT
Created attachment 4439 [details]
Better patch

Removes unnecessary comments
Comment 10 Eric Seidel (no email) 2005-10-21 15:25:09 PDT
Comment on attachment 4438 [details]
Proposed patch

We don't generally commit commented out code.
Comment 11 Eric Seidel (no email) 2005-10-21 15:26:19 PDT
Comment on attachment 4439 [details]
Better patch

Looks fine, I'll test again before committing.
Comment 12 Eric Seidel (no email) 2005-10-23 23:54:52 PDT
Fixed.