Closed
Bug 895135
Opened 11 years ago
Closed 11 years ago
Avoid normalizing gradients with all stops starting at the same location to the same position on the 0-1 line.
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: jrmuizel, Assigned: jrmuizel)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
6.60 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Currently if we have all the stops at the same location. We end up putting them all at 0. This makes things harder for backends that implement gradients with a lookup table because they don't have room for multiples entries at the zero location. If instead we leave the stops at the same location on the 0-1 line we'll have a better chance of drawing them correctly.
Assignee | ||
Comment 1•11 years ago
|
||
This works, but was made by fiddling with things until it worked. I plan on writing something that makes more sense. One of things that I'm undecided about is whether to avoid normalizing all non-repeating gradients that have stops inbetween 0 and 1 or to just handle the case mentioned above.
I don't understand. Your patch maps stops in the same place to 0.0, which is what you said in comment #0 you don't want to do.
Flags: needinfo?(roc)
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3) > I don't understand. Your patch maps stops in the same place to 0.0, which is > what you said in comment #0 you don't want to do. For repeating gradients it continues to do so. For non-repeating ones it shouldn't. Either way, I'm more interested in feedback on the idea and not the patch.
Assignee | ||
Comment 5•11 years ago
|
||
Here's a better implementation of this.
Attachment #777397 -
Attachment is obsolete: true
Attachment #777789 -
Flags: feedback?(roc)
Assignee | ||
Comment 6•11 years ago
|
||
Here are the reftest results for this change: https://tbpl.mozilla.org/?tree=Try&rev=4de1667a3f18
Assignee | ||
Comment 7•11 years ago
|
||
This just basically does what the bug title asks for and is much better than the previous versions A stylistic question: if (stopOrigin > 0) stopOrigin = 0; or stopOrigin = min(stopOrigin, 0);
Attachment #777789 -
Attachment is obsolete: true
Attachment #777789 -
Flags: feedback?(roc)
Attachment #778231 -
Flags: review?(roc)
Comment on attachment 778231 [details] [diff] [review] A much nicer version that finally makes things clear Review of attachment 778231 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsCSSRendering.cpp @@ +2285,5 @@ > + if (!aGradient->mRepeating || stopDelta == 0) { > + if (stopOrigin > 0) > + stopOrigin = 0; > + if (stopEnd < 1) > + stopEnd = 1; use min/max
Attachment #778231 -
Flags: review?(roc) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9497f8c1a115
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9497f8c1a115
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 11•11 years ago
|
||
This patch seems to have caused a substantial performance gain for gradients. At least it's fixed (or mostly fixed) bug 896703 on the trunk -- an old bug that goes back at least to FF 16. Could this patch be backported to aurora (the 24 branch), or even beta (the 23 branch), at least in principle?
You need to log in
before you can comment on or make changes to this bug.
Description
•