Bug 12488 – Paint server in objectBoundingBox mode not ignored if path has an empty bounding box

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Rémi Zara]

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.

[Rémi Zara]

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) ?

[Rémi Zara]

And also, filling a path with an empty boundingbox is a no-op, no ? so maybe this case could be optimized away in RenderPath.

[Eric Seidel (no email)]

I think we should talk about this over IRC.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

The Moz bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=368840

[Eric Seidel (no email)]

According to ed_work in #svg, the test case will be changed to match our (and firefox's) behavior here.

[Rob Buis]

The jury is still out on this one, our behaviour may be correct.

[Nikolas Zimmermann]

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