Closed
Bug 977418
Opened 10 years ago
Closed 10 years ago
Special case opacity:0.99 to treat it as opacity:1 for graphics
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
Developer use this trick sometimes to get a stacking context. Since it doesn't really change the rendering, it will be nice if layout special cased this and told gfx that there is no transparency involved so that gfx could avoid doing the expensive things it needs to do to handle transparency.
Comment 1•10 years ago
|
||
FWIW, from a quick skim, I suspect the spot we'd want to change to make this happen would be nsIFrame::HasOpacity() http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=5c277d91fcd6&mark=1005-1005#1002
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to comment #1) > FWIW, from a quick skim, I suspect the spot we'd want to change to make this > happen would be nsIFrame::HasOpacity() > http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=5c277d91fcd6&mark=1005-1005#1002 Is that where we communicate this information to graphics? I wouldn't actually mind writing a patch here if it's that simple! I haven't written any layout patches in a while. :-)
Comment 3•10 years ago
|
||
Per my "from a quick skim" / "suspect" language, it's just a guess. :) Explaining my reasoning more: HasOpacity() is at least what determines whether we create a nsDisplayOpacity in the display list (inside of nsIFrame::BuildDisplayListForStackingContext), which is at the interface between layout and graphics. So I'd guess that might be what ends up triggering graphics to do something different for opacity:1 vs. [non-1]. It's the first thing I'd try, if I were writing this patch, but I wouldn't be surprised if it turns out there's more that needs to be done. :)
Updated•10 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 4•10 years ago
|
||
Maybe just change this caller instead - http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#1861 HasOpacity() is also used to decide that we need to create a stacking context.
Comment 5•10 years ago
|
||
It would be nice if we could use 0.999 as the threshold, since at that value there is actually no difference. Unfortunately, I've just been debugging http://davidad.github.io/blog/2014/02/25/overkilling-the-8-queens-problem/, which applies opacity:0.99 to every div. Sadness.
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #5) > It would be nice if we could use 0.999 as the threshold, since at that value > there is actually no difference. 0.99 seems to be the "wisdom" spread in webdev literature. :-) So, Vlad, what do I need to check for in the gfx side to know if my patch has worked as expected?
Flags: needinfo?(vladimir)
Flags: needinfo?(vladimir)
Hm I tried move that needinfo the jrmuizel, but I think I failed.
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 8•10 years ago
|
||
Nevermind, the code is actually pretty simple. All we need to do here is to avoid creation of the nsDisplayOpacity.
Flags: needinfo?(jmuizelaar)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Comment 9•10 years ago
|
||
This helps avoid significant costs in our graphics code when websites use opacity:0.99 as a hint for creating a stacking context only.
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8384399 [details] [diff] [review] Special case opacity:0.99 to treat it as opacity:1 for display purposes; r=roc https://tbpl.mozilla.org/?tree=Try&rev=e4ceb7068023
Attachment #8384399 -
Flags: review?(roc)
Comment on attachment 8384399 [details] [diff] [review] Special case opacity:0.99 to treat it as opacity:1 for display purposes; r=roc Review of attachment 8384399 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsIFrame.h @@ +1140,5 @@ > + /** > + * Returns true if the frame is translucent for the purposes of creating a > + * stacking context. > + */ > + bool HasLogicalOpacity() const Let's keep calling this HasOpacity.
Attachment #8384399 -
Flags: review?(roc) → review+
Assignee | ||
Comment 12•10 years ago
|
||
Landed with adjusting the reftest manifests instead of modifying test opacity values: https://hg.mozilla.org/integration/mozilla-inbound/rev/56f9890e7fd9
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/56f9890e7fd9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•