368840 - Fallback paint should be used for degenerate objectBoundingBox gradients

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[tor]

Attachment: testcase (with and without fallback specified)

The last paragraph of the following section indicates that objectBoundingBox gradients should be ignored when the width or height of the object is zero.  We should use the fallback color in that case.

  http://www.w3.org/TR/SVG11/coords.html#ObjectBoundingBox

Comment 1

[tor]

Attachment: expected rendering

Comment 2

[tor]

Attachment: set fallback differently for stroke/fill, use fallback if pserver failed

Comment 3

[tor]

Similar webkit bug:  http://bugs.webkit.org/show_bug.cgi?id=12488

Comment 4

[Robert Longson [:longsonr]]

(In reply to comment #2)
> Created an attachment (id=253489) [details]
> set fallback differently for stroke/fill, use fallback if pserver failed
> 

If you are going to make the default stroke fallback colour transparent then you should change nsStyleSVG::nsStyleSVG in nsStyleStruct.cpp to set the mStroke.mFallbackColor transparent too.

Having said that, I'd be happy to take the style changes you want and incorporate them into bug 368836 instead of having them here.

Comment 5

[tor]

Attachment: change nsStyleSVG::nsStyleSVG

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 253761 [details] [diff] [review]
change nsStyleSVG::nsStyleSVG

r=dbaron.  Sorry for the delay.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 253761 [details] [diff] [review]
change nsStyleSVG::nsStyleSVG

Actually, I take that back.  review- since SetupPaintServer returns a boolean, not an nsresult, although some of the implementations mix returning of booleans and nsresults.  (Maybe that's changed since, but you need to fix this first.)

Also, the change to GetGradientTransform needs updating.

Comment 8

[tor]

Attachment: update to tip

Updated to tip, but it seems that cairo's behavior may have changed in regards to line extents with zero strokewidth (either that, or we've changed related mozilla code in past couple months).  Mailed cairo list about this; I'll include the URL when their mail archives get back online.

Comment 9

[Robert Longson [:longsonr]]

(In reply to comment #8)

cairo has changed. See bug 377085.

Comment 10

[tor]

cairo discussion about this can be found at:

  http://lists.freedesktop.org/archives/cairo/2007-May/thread.html

Look for "Extents of degenerate paths" (two thread chunks).

Comment 11

[Robert Longson [:longsonr]]

Attachment: update to tip add comments and reftests and split for review

Comment 12

[Robert Longson [:longsonr]]

Attachment: update to tip add reftest and split for review

Comment 13

[Robert Longson [:longsonr]]

Comment on attachment 299546 [details] [diff] [review]
update to tip add comments and reftests and split for review

Simple compliance fix with reftests. Should be pretty safe as it just causes fall-throughs to an existing code path.

Comment 14

[Robert Longson [:longsonr]]

Comment on attachment 299547 [details] [diff] [review]
update to tip add reftest and split for review

The style patch restores bug 354295 which got lost when fallback colours were implemented.

Comment 15

[Robert Longson [:longsonr]]

Comment on attachment 299546 [details] [diff] [review]
update to tip add comments and reftests and split for review

code change checked in

Comment 16

[Robert Longson [:longsonr]]

Comment on attachment 299547 [details] [diff] [review]
update to tip add reftest and split for review

This patch breaks http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-17-b.html

Comment 17

[Robert Longson [:longsonr]]

Attachment: fix gradients with 0 stops and transparent patterns

The previous patch broke http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-16-b.html

gradients with 0 stops end up getting a fallback colour rather than being transparent. Looks like transparent patterns have a related problem.

Comment 18

[Robert Longson [:longsonr]]

Attachment: don't test for stops if gradient is invalid

Comment 19

[Robert Longson [:longsonr]]

Comment on attachment 302061 [details] [diff] [review]
don't test for stops if gradient is invalid

Small fix for an edge case regression from the previous patch.

Comment 20

[Robert Longson [:longsonr]]

Comment on attachment 302061 [details] [diff] [review]
don't test for stops if gradient is invalid

checked in.

Comment 21

[Robert Longson [:longsonr]]

Checked in reftests:
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/fallback-color-01a.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/fallback-color-01b.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/fallback-color-02a.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/fallback-color-02b.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/fallback-color-03.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/linearGradient-basic-01.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/linearGradient-basic-02.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/radialGradient-basic-01.svg
http://lxr.mozilla.org/seamonkey/source/layout/reftests/svg/radialGradient-basic-02.svg