Bug 12488 - Paint server in objectBoundingBox mode not ignored if path has an empty bounding box
Summary: Paint server in objectBoundingBox mode not ignored if path has an empty bound...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://www.w3.org/Graphics/SVG/Test/2...
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-30 14:09 PST by Rémi Zara
Modified: 2007-09-03 13:17 PDT (History)
1 user (show)

See Also:


Attachments
Patch v1 (77.54 KB, patch)
2007-01-30 14:33 PST, Rémi Zara
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rémi Zara 2007-01-30 14:09:13 PST
A gradient should be ignored if gradientUnits is objectBoundingBox and path has an empty bounding box.
This makes w3c test pservers-grad-17-b fail.
Comment 1 Rémi Zara 2007-01-30 14:33:20 PST
Created attachment 12812 [details]
Patch v1

This patch fixes the W3C test, and add a new one that test pattern, and gradient/pattern attribute inheritance. I've not looked at filters, clips and masks.

I'm not quite sure why in my new test the last pattern is blank and not green.

The patch use a black solid color when the objectBoundingBox mode is "on" and the path has an empty boundingbox. The spec says that the effect should be ignored. Does this mean the stroke or fill should be inherited ?

I also find the "building" of paint server not ideal. It seems to be redone every time the paint server is used. Why isn't there a "computed attributes" on the Element, which would be calculated on demand (or when a referenced element is changed) ?
Comment 2 Rémi Zara 2007-01-30 14:36:04 PST
And also, filling a path with an empty boundingbox is a no-op, no ? so maybe this case could be optimized away in RenderPath.
Comment 3 Eric Seidel (no email) 2007-01-30 22:38:02 PST
I think we should talk about this over IRC.
Comment 4 Eric Seidel (no email) 2007-01-31 04:16:25 PST
Comment on attachment 12812 [details]
Patch v1

I think this is OK.  I'm not a huge fan of tying buildEffectiveAttributes() dependencies into the server resolution.  I think it fit better inside a setup() call.

Also, I dont' think this is right:

+    if (!strokePaintServer) {
+        // default value (black)
+        strokePaintServer = sharedSolidPaintServer();
+        static_cast<SVGPaintServerSolid*>(strokePaintServer)->setColor(Color::black);
+    }

that should just be special cased into 
+            if (strokePaintServer->boundingBoxMode() && item->relativeBBox(false).isEmpty())
+                strokePaintServer = 0;
instead.

Otherwise I think the patch is OK.

You should definitely have a reference to the spec about the empty bounding box.

Oh, and I don't think you need to special case this for fill servers.  I don't see how you could ever end up drawing a fill when there is no bounding box.
Comment 5 Eric Seidel (no email) 2007-01-31 05:48:35 PST
Overheard in #svg:

MacDome: see http://www.w3.org/TR/SVG11/coords.html#ObjectBoundingBox (last paragraph)
[5:25am] <ed_work> "When the geometry of the applicable element has no width or height and objectBoundingBox is specified, then the given effect (e.g., a gradient or a filter) will be ignored."
[5:26am] <MacDome> ed_work: yup, I saw it
[5:26am] <MacDome> ed_work: what does "ignored" mean
[5:26am] <MacDome> ed_work: default stroke is "none"
[5:26am] <MacDome> so ignored would mean "none" to me
[5:26am] <ed_work> yes, but there is a stroke on the g element above it
[5:27am] <ed_work> it's entirely possible that the reference/testcase is incorrect
[5:28am] <MacDome> ed_work: checking
[5:28am] <tor> ed_work: fallback doesn't work up the hierarchy - if it had stroke="url(#grad),#f00" then I'd expect to see something rendered
[5:28am] <ed_work> tor: yes, I was thinking along that line too now
[5:29am] <ed_work> since the cascade has already been made
Comment 6 Eric Seidel (no email) 2007-01-31 16:15:22 PST
The Moz bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=368840
Comment 7 Eric Seidel (no email) 2007-02-02 08:22:17 PST
According to ed_work in #svg, the test case will be changed to match our (and firefox's) behavior here.
Comment 8 Rob Buis 2007-06-12 10:11:46 PDT
The jury is still out on this one, our behaviour may be correct.
Comment 9 Nikolas Zimmermann 2007-09-03 13:17:34 PDT
We actually match the test in feature-branch nowadays. We _could_ close this bug now, depending on wheter the testcase should be changed or not...

Greetings,
Niko