418454 - Add support for Aero Glass effects to Widget layer

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

To do Aero Glass, we need two pieces:

1) Compositor
2) The way to specify Glass regions at the widget layer.

The way Glass works is that you hand the OS a list of rectangles that you want the glass effect applied to in your window.  Because of this API, it would be fairly difficult to get the effect in place before compositor.

Ideally, once this is in place, we will have an associated CSS property, e.g. background: -moz-glass or something to specify when glass should be used.

Note that there may be ways to apply glass to the entire background of the window, but I'm not sure if this is such a good idea -- that could cause unexpected issues with extensions and other things that want to interact with the core chrome.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The other approach is to use a color key, with a certain color that's replaced with the glass effect.  This is potentially even harder.

Comment 2

[Rob Arnold [:robarnold]]

There are problems with the first two options:
1. The outer frame remains, plus we need to keep track of the rectangles with glass.
2. The color keying makes text rendering looks funny. We'd have to find a color for the key that blends well given the text color.

Here's the third option which I have almost completed (except for the fallback cases). Using DwmExtendFrameIntoClientArea(mWnd, { -1, -1, -1, -1}), the entire window becomes a sheet of glass. The glass regions are controled by the alpha values of the back buffer.

Achieving glass now becomes easy: the root window is given -moz-appearance: glass and any areas of the window that want to be on glass need background: transparent. This also has the benefit of not depending on Compositor.

When non-opaque glass is not available (i.e. no Vista compositor), we have to fake the opaque glass effect by filling the surface with a solid color (the color of the window frame). In classic windows, this is the frame color. In Aero Basic, the frame color changes when the window is focused and unfocused but I'm pretty sure it's a solid color that fills the frame. We'd also want a nice way to let the ui know somehow that glass is disabled.

Comment 3

[Rob Arnold [:robarnold]]

Attachment: v1.0

First draft

Comment 4

[Rob Arnold [:robarnold]]

Attachment: the real v1.0

I forgot to qrefresh

Comment 5

[Rob Arnold [:robarnold]]

Comment on attachment 330308 [details] [diff] [review]
the real v1.0

Hmm, turns out this is still a WIP. Almost there though

Comment 6

[Rob Arnold [:robarnold]]

Attachment: Ok, this works (really, I swear!)

Fixed the frame/transparency issues and some stale data bug with theme data

Comment 7

[Rob Arnold [:robarnold]]

Comment on attachment 330598 [details] [diff] [review]
Ok, this works (really, I swear!)

There are some odd redraw issues in the fallback cases. I think this will be better handled by letting CSS handle the fallback just as it does with the native theme

Comment 8

[Rob Arnold [:robarnold]]

Attachment: v1.1

Fallback code is eliminated. Added a -moz-system-metric value to query if the DWM compositor is enabled. Moves mHasAeroGlass from nsWindow to sHasCompositor in nsUXThemeData which simplified a lot of code.

Comment 9

[Rob Arnold [:robarnold]]

Attachment: v1.2

Removed some extra cruft that came about during development

Comment 10

[Rob Arnold [:robarnold]]

Attachment: v1.3

Forgot to remove some of the fallback bits (the invalidate calls in WM_SETFOCUS and WM_KILLFOCUS)

Comment 11

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

Rename FrameHasTransparency to GetTransparencyMode.

+      if(rootFrame && NS_THEME_GLASS == rootFrame->GetStyleDisplay()->mAppearance)
+        mode = eTransparencyGlass;

Why is this needed? Shouldn't we just pass rootFrame instead of aFrame to FrameHasTransparency?

+CSS_KEY(glass,glass)

Shouldn't this be -moz-win like the others?

+    sSystemMetrics->AppendElement(do_GetAtom("dwm-compositor"));

For consistency, "windows-dwm-compositor"

+enum nsTransparencyMode {
+  eTransparencyOpaque = 0,
+  eTransparencyTransparent,
+  eTransparencyGlass

Document these please

+    return sHaveCompositor = (compositionIsEnabled ? PR_TRUE : PR_FALSE);

Can't you just return compositionIsEnabled? If you can't, at least write it as compositionIsEnabled != 0.

-  mIsTransparent      = PR_FALSE;
+  mTransparencyMode      = eTransparencyOpaque;

Fix indent

+    if (eTransparencyTransparent == mTransparencyMode)
+    {
+      SetupTranslucentWindowMemoryBitmap(eTransparencyOpaque);

Hmm. Shouldn't we really be testing for mTransparencyMode != Opaque && CompositionEnabled? I asssume that when composition is enabled, transparency-without-glass works the same way as with-glass, i.e. cairo draws RGBA and stuff just works. It would be great if we can use that. If so, we should abstract that check to a new inline function or something.

I think nsIWidget::GetHasTransparentBackground should return nsTransparencyMode. And both it and SetHasTransparentBackground should be renamed to Get/SetTransparencyMode.

Comment 12

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

> Shouldn't we really be testing for mTransparencyMode != Opaque &&
> CompositionEnabled?

I mean "mTransparencyMode != Opaque && !CompositionEnabled"

Comment 13

[TimoT]

On VS 2008 this fails to build with

ON=\"1.9a1\" -DMOZILLA_SKIN_VERSION=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIE
NT /e/mozilla-build/mozilla/src/layout/generic/nsContainerFrame.cpp
nsContainerFrame.cpp
e:/mozilla-build/mozilla/src/layout/generic/nsContainerFrame.cpp(502) : error C2
039: 'GetRootElementStyleFrame' : is not a member of 'nsCSSFrameConstructor'
        e:/mozilla-build/mozilla/src/layout/generic/../base\nsCSSFrameConstructo
r.h(84) : see declaration of 'nsCSSFrameConstructor'
make[6]: *** [nsContainerFrame.obj] Error 2

Comment 14

[TimoT]

Argh, nevermind, I'm blind. Sorry for spam.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #7)
> (From update of attachment 330598 [details] [diff] [review])
> There are some odd redraw issues in the fallback cases. I think this will be
> better handled by letting CSS handle the fallback just as it does with the
> native theme

The default theme detection is a hack that allows us to hardcode colors, borders etc. that would better be handled by the native theme code. Please don't take it as a leading case. If there's a way to fix the odd redraw issues, that would be great.

Comment 16

[Rob Arnold [:robarnold]]

(In reply to comment #11)
> Rename FrameHasTransparency to GetTransparencyMode.
> 
> +      if(rootFrame && NS_THEME_GLASS ==
> rootFrame->GetStyleDisplay()->mAppearance)
> +        mode = eTransparencyGlass;
> 
> Why is this needed? Shouldn't we just pass rootFrame instead of aFrame to
> FrameHasTransparency?

I found that if I passed rootFrame instead, ordinary firefox windows would be transparent.

> 
> +CSS_KEY(glass,glass)
> 
> Shouldn't this be -moz-win like the others?

I don't think there are any -moz-appearance values which have a -moz-win prefix even though some are windows specific.

> 
> +    sSystemMetrics->AppendElement(do_GetAtom("dwm-compositor"));
> 
> For consistency, "windows-dwm-compositor"

I figured dwm implied windows. How about windows-compositor?

> +    return sHaveCompositor = (compositionIsEnabled ? PR_TRUE : PR_FALSE);
> 
> Can't you just return compositionIsEnabled? If you can't, at least write it as
> compositionIsEnabled != 0.

I was hoping to avoid implicit BOOL to PRBool conversions. Also, the assignment is the important part of the function. Returning the value is just a convenience to the caller in the WM_DWMCOMPOSITIONCHANGE handler.

> 
> +    if (eTransparencyTransparent == mTransparencyMode)
> +    {
> +      SetupTranslucentWindowMemoryBitmap(eTransparencyOpaque);
> 
> Hmm. Shouldn't we really be testing for mTransparencyMode != Opaque &&
> CompositionEnabled? I asssume that when composition is enabled,
> transparency-without-glass works the same way as with-glass, i.e. cairo draws
> RGBA and stuff just works. It would be great if we can use that. If so, we
> should abstract that check to a new inline function or something.

Transparency paints differently than glass. With transparency, we render with alpha to an offscreen DC and use ::UpdateLayeredWindow to blit to the window. Glass is rendered much like an opaque window except that we push a color-alpha group instead of a color group to draw on.

Comment 17

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

(In reply to comment #16)
> I found that if I passed rootFrame instead, ordinary firefox windows would be
> transparent.

> > +CSS_KEY(glass,glass)
> > 
> > Shouldn't this be -moz-win like the others?
> 
> I don't think there are any -moz-appearance values which have a -moz-win
> prefix even though some are windows specific.

Isn't -moz-win-media-toolbox directly above an example of that?

> > +    sSystemMetrics->AppendElement(do_GetAtom("dwm-compositor"));
> > 
> > For consistency, "windows-dwm-compositor"
> 
> I figured dwm implied windows. How about windows-compositor?

Sure.

> I was hoping to avoid implicit BOOL to PRBool conversions. Also, the 
> assignment is the important part of the function. Returning the value is just
> a convenience to the caller in the WM_DWMCOMPOSITIONCHANGE handler.

Just use != 0 then.

> Transparency paints differently than glass. With transparency, we render with
> alpha to an offscreen DC and use ::UpdateLayeredWindow to blit to the window.
> Glass is rendered much like an opaque window except that we push a color-alpha
> group instead of a color group to draw on.

I'm saying, when composition is available should we render transparency the same way as glass? It seems plausible that might work better than UpdateLayeredWindow.

Comment 18

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

(In reply to comment #17)
> (In reply to comment #16)
> > I found that if I passed rootFrame instead, ordinary firefox windows would
> > be transparent.

Oops ... I understand now. The issue here is that the CSS 'background' propagates from the root element's frame (rootFrame) to the real root frame (nsViewportFrame), so we need to call FrameHasTransparency on that. But -moz-appearance does not propagate so we need to check that directly on rootFrame.

So I'm OK with this part of your patch, but you might want to write what I just said in as a comment.

Comment 19

[Rob Arnold [:robarnold]]

(In reply to comment #17)

> > I don't think there are any -moz-appearance values which have a -moz-win
> > prefix even though some are windows specific.
> 
> Isn't -moz-win-media-toolbox directly above an example of that?

Oops, yes. I was thinking of my menu changes from last year which were Vista/Windows only but had no -moz prefix. I'm not sure if I like having a second -moz prefix (since there's one for -moz-appearance already). What about aero-glass?

> > Transparency paints differently than glass. With transparency, we render with
> > alpha to an offscreen DC and use ::UpdateLayeredWindow to blit to the window.
> > Glass is rendered much like an opaque window except that we push a color-alpha
> > group instead of a color group to draw on.
> 
> I'm saying, when composition is available should we render transparency the
> same way as glass? It seems plausible that might work better than
> UpdateLayeredWindow.
> 
I don't think we can. Transparency requires layered windows (because hit detection works differently with transparency vs glass and transparency works with the old window manager). I'll try it out though to be sure.

Comment 20

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

(In reply to comment #19)
> (In reply to comment #17)
> 
> > > I don't think there are any -moz-appearance values which have a -moz-win
> > > prefix even though some are windows specific.
> > 
> > Isn't -moz-win-media-toolbox directly above an example of that?
> 
> Oops, yes. I was thinking of my menu changes from last year which were
> Vista/Windows only but had no -moz prefix. I'm not sure if I like having a
> second -moz prefix (since there's one for -moz-appearance already). What about
> aero-glass?

We already had that discussion about media-toolbox and friends, let's not reopen it.

> I don't think we can. Transparency requires layered windows (because hit
> detection works differently with transparency vs glass and transparency works
> with the old window manager). I'll try it out though to be sure.

Good point about hit detection.

Comment 21

[Rob Arnold [:robarnold]]

(In reply to comment #15)
> 
> The default theme detection is a hack that allows us to hardcode colors,
> borders etc. that would better be handled by the native theme code. Please
> don't take it as a leading case. If there's a way to fix the odd redraw issues,
> that would be great.
> 

I hope we don't end up hardcoding colors. The colors needed for the fallback aren't documented however, so I'm not sure that I found the "right" ones in the theme. I think giving the theme the ability to change itself based on the availability of the compositor is better than doing the fallback in the code because the fallback I pick might not be what they want (ex: they might want to alter the layout or swap images around or whatnot). I also feel that the fix for the redraw issues would be a larger hack than hard coding colors.

Comment 22

[Rob Arnold [:robarnold]]

Attachment: v1.4

Roc's comments are addressed. Rendering of content breaks the glass effect still.

Comment 23

[Rob Arnold [:robarnold]]

Attachment: v1.4 - reduced

This patch breaks the rendering of content on transparent windows but I can't quite figure out why. I've tried to reduce the patch to eliminate the changes I made except for the change from a boolean to enum for transparency and the addition of the windows-compositor css selector.

Comment 24

[Rob Arnold [:robarnold]]

Attachment: v1.5

Content no longer breaks glass. Instead content is now rendered non-opaquely. Since content never quite rendered properly on transparent windows before, I don't really care too much. Supposedly compositor will fix this/make the fix easier.

Comment 25

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 332447 [details] [diff] [review]
v1.5


Looks fine, with some comments:
 
> #ifdef MOZ_XUL
>-      if (mIsTransparent) {
>+      if (eTransparencyGlass == mTransparencyMode && nsUXThemeData::sHaveCompositor) {
>+        thebesContext->PushGroup(gfxASurface::CONTENT_COLOR_ALPHA);
>+        thebesContext->SetOperator(gfxContext::OPERATOR_CLEAR);
>+        thebesContext->Paint();
>+        thebesContext->SetOperator(gfxContext::OPERATOR_OVER);
>+      } else if (eTransparencyTransparent == mTransparencyMode) {

When you PushGroup, it should already be cleared -- you shouldn't need the SetOperator/Paint/SetOperator bits here.

>+  NS_IMETHOD              GetTransparencyMode(nsTransparencyMode& aMode);

Since you're changing this anyway, should we make it sane and just have it be 'nsTransparencyMode GetTransparencyMode()' everywhere?  That would simplify a lot of the:

nsTransparencyMode trans = eTransparencyModeOpaque;
GetTransparencyMode(trans);
if (trans == ..) { ... }

patterns a lot as well.  roc, what do you think?  nsIWindow isn't a real XPCOM interface any more anyway, right?

Comment 26

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

nsIWidget? Absolutely not a real XPCOM interface. deCOMtaminate at will.

Comment 27

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

Rename FrameHasTransparency to GetFrameTransparency.

+     * The value of this metric is not used on other platforms. These platforms
+     * should return NS_ERROR_NOT_IMPLEMENTED when queried for this metric.

You need to mention "Windows" somewhere around here.

+    if(nsUXThemeData::CheckForCompositor() && mTransparencyMode == eTransparencyGlass) {

Space after "if"

+      } else if (eTransparencyGlass == mTransparencyMode && result) {
+        thebesContext->PopGroupToSource();
+        thebesContext->SetOperator(gfxContext::OPERATOR_SOURCE);
+        thebesContext->Paint();

Why is this needed? The following "if (result)" clause does the same thing...

Otherwise looks great.

Comment 28

[Rob Arnold [:robarnold]]

Attachment: v1.6

Review comments addressed. Patch went through try server ok. There's probably more deCOMtamination to be done for nsIWidget, but that's outside the scope of this patch.

Comment 29

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/460fce61ebc7

Comment 30

[[e-mail]]

Comment on attachment 333422 [details] [diff] [review]
v1.6

>+  // UXTheme.dll Function typedefs and declarations
>   typedef HANDLE (WINAPI*OpenThemeDataPtr)(HWND hwnd, LPCWSTR pszClassList);
>   typedef HRESULT (WINAPI*CloseThemeDataPtr)(HANDLE hTheme);
>   typedef HRESULT (WINAPI*DrawThemeBackgroundPtr)(HANDLE hTheme, HDC hdc, int iPartId, 
>                                             int iStateId, const RECT *pRect,
>                                             const RECT* pClipRect);
>   typedef HRESULT (WINAPI*DrawThemeEdgePtr)(HANDLE hTheme, HDC hdc, int iPartId, 
>                                             int iStateId, const RECT *pDestRect,
>                                             uint uEdge, uint uFlags,
>@@ -120,21 +127,37 @@ public:
>                                      int iStateId, int iPropId, OUT COLORREF* pFont);
>   typedef HRESULT (WINAPI*GetThemeMarginsPtr)(HANDLE hTheme, HDC hdc, int iPartId,
>                                            int iStateid, int iPropId,
>                                            LPRECT prc, MARGINS *pMargins);
>   typedef BOOL (WINAPI*IsAppThemedPtr)(VOID);
>   typedef HRESULT (WINAPI*GetCurrentThemeNamePtr)(LPWSTR pszThemeFileName, int dwMaxNameChars,
>                                                   LPWSTR pszColorBuff, int cchMaxColorChars,
>                                                   LPWSTR pszSizeBuff, int cchMaxSizeChars);
>+  typedef COLORREF (WINAPI*GetThemeSysColorPtr)(HTHEME hTheme, int iColorID);
Any chance we can change this to HANDLE a) for consistency with the other entries b) because it doesn't work with some older supported SDKs?

Comment 31

[[e-mail]]

Attachment: HTHEME (pushed: 198b8dde7a94)

Comment 32

[Serge Gautherie (:sgautherie)]

(In reply to comment #30)
> b) because it doesn't work with some older supported SDKs?

Trying to build "default" Firefox, with VC++ 2005/v8 + PSDK 2003R2:
{{
nsWindow.cpp
...\widget\src\windows\nsUXThemeData.h(135)
: error C2061: syntax error : identifier 'HTHEME'
make[6]: *** [nsWindow.obj] Error 2
}}

Comment 33

[Serge Gautherie (:sgautherie)]

(In reply to comment #32)

V.Fixed, on this part, after additional checkin.

Comment 34

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: small crash fix

Small crash fix that shows up if you use -chrome with a .html file without a background on the body element.  The transparency bitmap isn't created by the time we get to OnPaint, and things blow up.  This isn't really well-supported (-chrome with .html), but we should avoid the rash in any case.

Comment 35

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/8d8ef01ef749

Comment 36

[sibbl]

I guess this code is really buggy. Why does it make black colored labels shown in glass too? Have a look at the source of the Addon glasser - it shows glass on the top of the window AND let's the black labels of eg. the menus shown in black.

Also I think there should be a possibility to only show glass for x pixels from top. This is not that difficult to develop...

Comment 37

[Martijn Wargers (dead)]

Could you file new bugs for that?

Comment 38

[Rob Arnold [:robarnold]]

I'm not sure I understand what you mean by black colored labels being shown on glass. Do you want them shown or not? Please file a bug and explain there.

As for only showing glass x pixels from the top, that's not entirely desirable (we'd like something more flexible). See bug 450767 for that.

Comment 39

[sibbl]

https://bugzilla.mozilla.org/show_bug.cgi?id=458407