Closed
Bug 368840
Opened 18 years ago
Closed 17 years ago
Fallback paint should be used for degenerate objectBoundingBox gradients
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tor, Assigned: longsonr)
References
Details
Attachments
(4 files, 5 obsolete files)
(deleted),
image/svg+xml
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
tor
:
review+
tor
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
tor
:
review+
tor
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
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
Attachment #253489 -
Flags: review?
Attachment #253489 -
Flags: review? → review?(dbaron)
Similar webkit bug: http://bugs.webkit.org/show_bug.cgi?id=12488
Assignee | ||
Comment 4•18 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.
Attachment #253489 -
Attachment is obsolete: true
Attachment #253761 -
Flags: review?(dbaron)
Attachment #253489 -
Flags: review?(dbaron)
Comment 6•18 years ago
|
||
Comment on attachment 253761 [details] [diff] [review]
change nsStyleSVG::nsStyleSVG
r=dbaron. Sorry for the delay.
Attachment #253761 -
Flags: review?(dbaron) → review+
Comment 7•18 years ago
|
||
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-
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•18 years ago
|
||
(In reply to comment #8)
cairo has changed. See bug 377085.
Reporter | ||
Comment 10•18 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 | ||
Comment 11•17 years ago
|
||
Attachment #264027 -
Attachment is obsolete: true
Attachment #299546 -
Flags: superreview?(tor)
Attachment #299546 -
Flags: review?(tor)
Assignee | ||
Comment 12•17 years ago
|
||
Attachment #299547 -
Flags: superreview?(dbaron)
Attachment #299547 -
Flags: review?(dbaron)
Attachment #299546 -
Flags: superreview?(tor)
Attachment #299546 -
Flags: superreview+
Attachment #299546 -
Flags: review?(tor)
Attachment #299546 -
Flags: review+
Assignee | ||
Comment 13•17 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•17 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•17 years ago
|
Attachment #299546 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 15•17 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•17 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•17 years ago
|
||
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•17 years ago
|
||
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)
Attachment #302061 -
Flags: superreview?(tor)
Attachment #302061 -
Flags: superreview+
Attachment #302061 -
Flags: review?(tor)
Attachment #302061 -
Flags: review+
Assignee | ||
Comment 19•17 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•17 years ago
|
Attachment #302061 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 20•17 years ago
|
||
Comment on attachment 302061 [details] [diff] [review]
don't test for stops if gradient is invalid
checked in.
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 21•17 years ago
|
||
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
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•