444005 - Disable double buffering when Vista compositor is enabled

Status: REOPENED

(Core :: Widget: Win32, defect)

Description

[Rob Arnold [:robarnold]]

The compositor does the double buffering for us when enabled

Comment 1

[Rob Arnold [:robarnold]]

Attachment: v1.0

This disables double buffering when the Vista compositor is enabled

Comment 2

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

Comment on attachment 329002 [details] [diff] [review]
v1.0

>diff --git a/widget/src/windows/nsWindow.cpp b/widget/src/windows/nsWindow.cpp
>--- a/widget/src/windows/nsWindow.cpp
>+++ b/widget/src/windows/nsWindow.cpp
>@@ -617,6 +617,21 @@
> 
> // End of the methods to dispatch global messages
> 
>+#ifndef WM_DWMCOMPOSITIONCHANGED
>+#define WM_DWMCOMPOSITIONCHANGED        0x031E
>+#endif
>+
>+// DWM function prototypes
>+typedef HRESULT (WINAPI*DwmIsCompositionEnabledPtr)(BOOL *pfEnabled);
>+static DwmIsCompositionEnabledPtr dwmIsCompositionEnabled = NULL;

Call the type "DwmIsCompositionEnabledProc", and the variable "DwmIsCompositionEnabledPtr" -- using a case to distinguish symbols makes me a little nervous, and the Proc/Ptr convention gets used elsewhere.

>+  PRBool haveCompositor = GetTopLevelWindow()->mHasAeroGlass;


>+
>   nsCOMPtr<nsIRegion> paintRgnWin;
>   if (paintRgn) {
>     paintRgnWin = ConvertHRGNToRegion(paintRgn);
>@@ -5798,8 +5828,10 @@
>         thebesContext->Paint();
>         thebesContext->SetOperator(gfxContext::OPERATOR_OVER);
>       } else {
>-        // If we're not doing translucency, then double buffer
>-        thebesContext->PushGroup(gfxASurface::CONTENT_COLOR);
>+        // If the Vista compositor is on, then we don't need to double buffer
>+        if(!haveCompositor)
>+          // If we're not doing translucency, then double buffer
>+          thebesContext->PushGroup(gfxASurface::CONTENT_COLOR);

Just make this } else if (!haveCompositor) { ...
instead of } else {  if (!haveC..) }.  (Also note the space after "if".)

Looks fine with those two changes.

Comment 3

[Rob Arnold [:robarnold]]

Attachment: v1.1

Review comments addressed

Comment 4

[Shawn Wilsher :sdwilsh]

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

Comment 5

[Jim Jeffery not reading bug-mail 1/2/11]

This bug caused:
https://bugzilla.mozilla.org/show_bug.cgi?id=445838

Comment 6

[Shawn Wilsher :sdwilsh]

This has been backed out because it caused bug 445838.

Comment 7

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

Matt looked into this during his internship while we were working on retained layers. On our Windows 7 machine, the DWM *does not* eliminate the need for double-buffering. Unless someone has experimental data showing we were wrong, I think we should close this bug WONTFIX.

Comment 8

[Matt Woodrow (:mattwoodrow)]

I definitely got very bad flickering on Windows 7 with a similar patch to this.

http://stackoverflow.com/questions/1840516/is-double-buffering-required-with-desktop-composition-enabled suggests that the backing buffer can/will be read from between Begin/EndPaint calls.

Comment 9

[Rob Arnold [:robarnold]]

My understanding was that the DWM was not notified of Begin/EndPaint which would be ok if there was double buffering going on under the hood with GDI but it seems like there's not. The DWM did change between Vista and 7 so it would be interesting to see if Vista actually double buffers but 7 doesn't.