1055585 - Add support for clearing window contents behind themed frames with certain -moz-appearance values

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Markus Stange [:mstange]]

This is for the Mac OS 10.10 behind-window vibrancy style (bug 1051522), which looks similar to the Windows glass effect. It's primarily used for sidebars.

Vibrancy works by declaring vibrant regions to the window server, and during painting, clearing the background of these areas and painting any contents into full transparency.

I'd like a way for nsITheme to tell layers "Please clear the window background behind this widget". Then we can put vibrant widgets onto any opaque widget in the window, and our stylesheets don't have to make sure that the stuff that's behind the widget is also transparent. It also means that the window itself won't have to be transparent, so things that are outside the vibrant parts still know that they paint onto something opaque. (I'm not really sure how this part works with glass.)

My suggested approach is to add an nsITheme API, nsITheme::NeedToClearBackgroundBehindWidget, and a new display item nsDisplayClearBackground, which translates into a transparent ColorLayer that has a blend mode of OP_SOURCE. Then we declare that background clearing is only supported if the stacking context around the widget is the window itself and forget about the hard cases.
Does that sound like a sensible approach?

Comment 1

[Markus Stange [:mstange]]

Attachment: part 1: Add nsDisplayClearBackground

This adds nsDisplayClearBackground. nsDisplayClearBackground is opaque in the sense that it doesn't matter what's painted behind it.
I think the parent layer scale comment in the patch probably isn't worth worrying about, because if there's a parent layer then it won't work anyway because it's not directly drawing into the window.

Comment 2

[Markus Stange [:mstange]]

Attachment: part 2: Support operator source in CompositorOGL

Comment 3

[Markus Stange [:mstange]]

Attachment: part 3: Make nsDisplayClearBackground force drawing into a transparent surface

Comment 4

[Markus Stange [:mstange]]

Attachment: part 4: Call nsDisplayList::SetNeedsTransparentSurface

It seems this got broken at some point. At the moment, the only caller of this is only calling it if there already is an nsDisplayList that NeedsTransparentSurface().
I also don't know if this is a good place to add the call. Thoughts?

Comment 5

[Markus Stange [:mstange]]

The purpose of part 3 and 4 is to flag the root ContainerLayer as not supporting component alpha children, because 2-pass component alpha layer rendering simply doesn't work on top of transparent window backgrounds.

Comment 6

[Markus Stange [:mstange]]

Attachment: part 5: Add nsITheme API and create nsDisplayClearBackground

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is very similar to Windows' nsWindow::UpdateThemeGeometries, which calls LayerManager::SetRegionToClear, which should work on GL too. Can you use that approach instead?

Comment 8

[Markus Stange [:mstange]]

I think that will clear a region in the final composited result, whereas I'd like to clear between painting the window background and the sidebar foreground.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm, OK.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8475224 [details] [diff] [review]
part 4: Call nsDisplayList::SetNeedsTransparentSurface

Review of attachment 8475224 [details] [diff] [review]:
-----------------------------------------------------------------

This probably got lost during a rebase or something.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8475220 [details] [diff] [review]
part 2: Support operator source in CompositorOGL

Review of attachment 8475220 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the default dstBlendAlpha value fixed.

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +976,5 @@
>    }
>  
>    GLenum srcBlend;
>    GLenum dstBlend;
> +  GLenum dstBlendAlpha = LOCAL_GL_ONE_MINUS_SRC_ALPHA;

Shouldn't this just be LOCAL_GL_ONE?

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 8475220 [details] [diff] [review]
part 2: Support operator source in CompositorOGL

oops yes

Comment 13

[Markus Stange [:mstange]]

I had to merge part 2 on top of bug 1002823 which added OP_SOURCE support to SetBlendMode just before me, but with bad variable names (I think) and without the aIsPremultiplied check.

https://hg.mozilla.org/integration/mozilla-inbound/rev/728d6583d8d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5178d4e9f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6e68c3679e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/35e778ffed08
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d4e2c22ed89

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/728d6583d8d8
https://hg.mozilla.org/mozilla-central/rev/ff5178d4e9f9
https://hg.mozilla.org/mozilla-central/rev/e6e68c3679e0
https://hg.mozilla.org/mozilla-central/rev/35e778ffed08
https://hg.mozilla.org/mozilla-central/rev/7d4e2c22ed89