Last Comment Bug 418454 - Add support for Aero Glass effects to Widget layer
: Add support for Aero Glass effects to Widget layer
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows Vista
: -- normal with 1 vote (vote)
: mozilla1.9.1a2
Assigned To: Rob Arnold [:robarnold]
:
Mentors:
Depends on: 438987 444013 444679 447045 458407
Blocks: 367993 450767
  Show dependency treegraph
 
Reported: 2008-02-19 11:09 PST by Vladimir Vukicevic [:vlad] [:vladv]
Modified: 2008-10-03 11:07 PDT (History)
23 users (show)
vladimir: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1.0 (16.85 KB, patch)
2008-07-18 15:02 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
the real v1.0 (22.97 KB, patch)
2008-07-18 15:04 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
Ok, this works (really, I swear!) (23.04 KB, patch)
2008-07-21 10:44 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.1 (25.77 KB, patch)
2008-07-22 16:46 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.2 (24.82 KB, patch)
2008-07-22 17:09 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.3 (24.15 KB, patch)
2008-07-22 17:20 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.4 (38.22 KB, patch)
2008-07-23 15:51 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.4 - reduced (38.61 KB, patch)
2008-07-24 17:10 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
v1.5 (59.50 KB, patch)
2008-08-05 17:04 PDT, Rob Arnold [:robarnold]
vladimir: superreview+
Details | Diff | Review
v1.6 (59.96 KB, patch)
2008-08-12 08:41 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Review
HTHEME (pushed: 198b8dde7a94) (741 bytes, patch)
2008-08-14 03:57 PDT, neil@parkwaycc.co.uk
tellrob: review+
vladimir: review+
Details | Diff | Review
small crash fix (558 bytes, patch)
2008-09-30 15:36 PDT, Vladimir Vukicevic [:vlad] [:vladv]
tellrob: review+
Details | Diff | Review

Description Vladimir Vukicevic [:vlad] [:vladv] 2008-02-19 11:09:26 PST
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] 2008-02-19 11:12:06 PST
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] 2008-07-07 17:38:45 PDT
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] 2008-07-18 15:02:58 PDT
Created attachment 330305 [details] [diff] [review]
v1.0

First draft
Comment 4 Rob Arnold [:robarnold] 2008-07-18 15:04:57 PDT
Created attachment 330308 [details] [diff] [review]
the real v1.0

I forgot to qrefresh
Comment 5 Rob Arnold [:robarnold] 2008-07-19 12:12:37 PDT
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] 2008-07-21 10:44:56 PDT
Created attachment 330598 [details] [diff] [review]
Ok, this works (really, I swear!)

Fixed the frame/transparency issues and some stale data bug with theme data
Comment 7 Rob Arnold [:robarnold] 2008-07-21 13:00:26 PDT
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] 2008-07-22 16:46:05 PDT
Created attachment 330860 [details] [diff] [review]
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] 2008-07-22 17:09:29 PDT
Created attachment 330864 [details] [diff] [review]
v1.2

Removed some extra cruft that came about during development
Comment 10 Rob Arnold [:robarnold] 2008-07-22 17:20:41 PDT
Created attachment 330866 [details] [diff] [review]
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) (Exited; email my personal email if necessary) 2008-07-22 17:36:06 PDT
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) (Exited; email my personal email if necessary) 2008-07-22 17:38:05 PDT
> Shouldn't we really be testing for mTransparencyMode != Opaque &&
> CompositionEnabled?

I mean "mTransparencyMode != Opaque && !CompositionEnabled"
Comment 13 Timo Tolonen 2008-07-22 18:09:57 PDT
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 Timo Tolonen 2008-07-22 20:22:37 PDT
Argh, nevermind, I'm blind. Sorry for spam.
Comment 15 Dão Gottwald [:dao] 2008-07-22 23:30:41 PDT
(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] 2008-07-23 11:14:00 PDT
(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) (Exited; email my personal email if necessary) 2008-07-23 11:46:34 PDT
(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) (Exited; email my personal email if necessary) 2008-07-23 11:51:28 PDT
(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] 2008-07-23 14:02:39 PDT
(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) (Exited; email my personal email if necessary) 2008-07-23 14:07:15 PDT
(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] 2008-07-23 14:35:14 PDT
(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] 2008-07-23 15:51:46 PDT
Created attachment 331010 [details] [diff] [review]
v1.4

Roc's comments are addressed. Rendering of content breaks the glass effect still.
Comment 23 Rob Arnold [:robarnold] 2008-07-24 17:10:11 PDT
Created attachment 331234 [details] [diff] [review]
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] 2008-08-05 17:04:42 PDT
Created attachment 332447 [details] [diff] [review]
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] 2008-08-07 11:03:57 PDT
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) (Exited; email my personal email if necessary) 2008-08-07 15:52:07 PDT
nsIWidget? Absolutely not a real XPCOM interface. deCOMtaminate at will.
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-08-07 16:19:59 PDT
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] 2008-08-12 08:41:49 PDT
Created attachment 333422 [details] [diff] [review]
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] 2008-08-12 17:51:39 PDT
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/460fce61ebc7
Comment 30 neil@parkwaycc.co.uk 2008-08-14 02:27:37 PDT
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 neil@parkwaycc.co.uk 2008-08-14 03:57:36 PDT
Created attachment 333729 [details] [diff] [review]
HTHEME (pushed: 198b8dde7a94)
Comment 32 Serge Gautherie (:sgautherie) 2008-08-14 12:59:30 PDT
(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) 2008-08-15 09:51:50 PDT
(In reply to comment #32)

V.Fixed, on this part, after additional checkin.
Comment 34 Vladimir Vukicevic [:vlad] [:vladv] 2008-09-30 15:36:52 PDT
Created attachment 341167 [details] [diff] [review]
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] 2008-09-30 23:07:31 PDT
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/8d8ef01ef749
Comment 36 sibbl 2008-10-03 02:22:07 PDT
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 [:mwargers] (gone per 2016-05-31 :-( ) 2008-10-03 05:27:55 PDT
Could you file new bugs for that?
Comment 38 Rob Arnold [:robarnold] 2008-10-03 08:14:19 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.