Fallback paint should be used for degenerate objectBoundingBox gradients

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: tor, Assigned: longsonr)

Tracking

Trunk
x86
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 5 obsolete attachments)

(Reporter)

Description

12 years ago
Created attachment 253487 [details]
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
(Reporter)

Comment 1

12 years ago
Created attachment 253488 [details]
expected rendering
(Reporter)

Comment 2

12 years ago
Created attachment 253489 [details] [diff] [review]
set fallback differently for stroke/fill, use fallback if pserver failed
Attachment #253489 - Flags: review?
(Reporter)

Updated

12 years ago
Attachment #253489 - Flags: review? → review?(dbaron)
(Reporter)

Comment 3

12 years ago
Similar webkit bug:  http://bugs.webkit.org/show_bug.cgi?id=12488
(Assignee)

Comment 4

12 years ago
(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.


(Reporter)

Comment 5

12 years ago
Created attachment 253761 [details] [diff] [review]
change nsStyleSVG::nsStyleSVG
Attachment #253489 - Attachment is obsolete: true
Attachment #253761 - Flags: review?(dbaron)
Attachment #253489 - Flags: review?(dbaron)
Comment on attachment 253761 [details] [diff] [review]
change nsStyleSVG::nsStyleSVG

r=dbaron.  Sorry for the delay.
Attachment #253761 - Flags: review?(dbaron) → review+
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.
Attachment #253761 - Flags: review+ → review-
(Reporter)

Comment 8

11 years ago
Created attachment 264027 [details] [diff] [review]
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.
Attachment #253761 - Attachment is obsolete: true
(Assignee)

Comment 9

11 years ago
(In reply to comment #8)

cairo has changed. See bug 377085.
(Reporter)

Comment 10

11 years ago
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).
(Assignee)

Updated

11 years ago
Depends on: 377085
(Assignee)

Comment 11

11 years ago
Created attachment 299546 [details] [diff] [review]
update to tip add comments and reftests and split for review
Attachment #264027 - Attachment is obsolete: true
Attachment #299546 - Flags: superreview?(tor)
Attachment #299546 - Flags: review?(tor)
(Assignee)

Comment 12

11 years ago
Created attachment 299547 [details] [diff] [review]
update to tip add reftest and split for review
Attachment #299547 - Flags: superreview?(dbaron)
Attachment #299547 - Flags: review?(dbaron)
(Reporter)

Updated

11 years ago
Attachment #299546 - Flags: superreview?(tor)
Attachment #299546 - Flags: superreview+
Attachment #299546 - Flags: review?(tor)
Attachment #299546 - Flags: review+
(Assignee)

Comment 13

11 years ago
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.
Attachment #299546 - Flags: approval1.9?
(Assignee)

Comment 14

11 years ago
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.

Updated

11 years ago
Attachment #299546 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 15

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

code change checked in
(Assignee)

Comment 16

11 years ago
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
Attachment #299547 - Attachment is obsolete: true
Attachment #299547 - Flags: superreview?(dbaron)
Attachment #299547 - Flags: review?(dbaron)
(Assignee)

Comment 17

11 years ago
Created attachment 302052 [details] [diff] [review]
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.
Assignee: tor → longsonr
Status: NEW → ASSIGNED
Attachment #302052 - Flags: superreview?(tor)
Attachment #302052 - Flags: review?(tor)
(Assignee)

Comment 18

11 years ago
Created attachment 302061 [details] [diff] [review]
don't test for stops if gradient is invalid
Attachment #302052 - Attachment is obsolete: true
Attachment #302061 - Flags: superreview?(tor)
Attachment #302061 - Flags: review?(tor)
Attachment #302052 - Flags: superreview?(tor)
Attachment #302052 - Flags: review?(tor)
(Reporter)

Updated

11 years ago
Attachment #302061 - Flags: superreview?(tor)
Attachment #302061 - Flags: superreview+
Attachment #302061 - Flags: review?(tor)
Attachment #302061 - Flags: review+
(Assignee)

Comment 19

11 years ago
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.
Attachment #302061 - Flags: approval1.9?

Updated

11 years ago
Attachment #302061 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 20

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

checked in.
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.