Open Bug 444005 Opened 12 years ago Updated 9 years ago

Disable double buffering when Vista compositor is enabled

Categories

(Core :: Widget: Win32, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

REOPENED
mozilla1.9.1a1

People

(Reporter: robarnold, Assigned: robarnold)

Details

Attachments

(1 file, 1 obsolete file)

The compositor does the double buffering for us when enabled
Assignee: nobody → tellrob
Attached patch v1.0 (obsolete) — Splinter Review
This disables double buffering when the Vista compositor is enabled
Attachment #329002 - Flags: review?(vladimir)
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.
Attachment #329002 - Flags: review?(vladimir) → review+
Attached patch v1.1Splinter Review
Review comments addressed
Attachment #329002 - Attachment is obsolete: true
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/e05e892261a8
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
This has been backed out because it caused bug 445838.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No longer blocks: 418454
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.