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)

defect
Not set
normal

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.
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
(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.  :-)
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. :)
OS: Mac OS X → All
Hardware: x86 → All
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.
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.
(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)
Hm I tried move that needinfo the jrmuizel, but I think I failed.
Flags: needinfo?(jmuizelaar)
Nevermind, the code is actually pretty simple.  All we need to do here is to avoid creation of the nsDisplayOpacity.
Flags: needinfo?(jmuizelaar)
Assignee: nobody → ehsan
This helps avoid significant costs in our graphics code when websites
use opacity:0.99 as a hint for creating a stacking context only.
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+
Landed with adjusting the reftest manifests instead of modifying test opacity values:

https://hg.mozilla.org/integration/mozilla-inbound/rev/56f9890e7fd9
https://hg.mozilla.org/mozilla-central/rev/56f9890e7fd9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Depends on: 983084
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: