prevents WM_DISPLAYCHANGE message being sent when a call to ChangeDisplaySettingsEx fails due to the requested change to the display not occurring.

chris morgan cmorgan at mail.leather-wallet.co.uk
Thu Mar 27 15:00:03 CDT 2008


thanks for the feedback, i'm new to this so it's very much appreciated - 
helped a lot.

I've updated the patch which hopefully covers the issues raised, but 
wanted to ask about the returning of an error:
> w.r.t. the implementation:
>   *  ChangeDisplaySettings returns a long
> (http://msdn2.microsoft.com/en-us/library/ms533260(VS.85).aspx), so
> the correct value must be returned on error;
>   
as failure to change the colour depth currently isn't treated as an 
error which seems sensible as it enables many older apps to run 
presumably without issue so thought it better not to change that.

from X11DRV_desktop_SetCurrentMode

        /* Ignore the depth mismatch
         *
         * Some (older) applications require a specific bit depth, this 
will allow them
         * to run. X11drv performs a color depth conversion if needed.
         */
>   *  the fix does not match the test cases (screen size vs colour depth). 
oops, bad oversight. to fix this i've created a variable to store the 
state of screens colour depth as it would be perceived by a client. this 
is used to detect changes to screen depth and prevent unexpected 
WM_DISPLAYCHANGE from being sent.

i.e. a program calls ChangeDisplaySettings changing the colour depth and 
receives back a WM_DISPLAYCHANGE message. As far as the client is 
concerned the screen is at the new colour depth as it's call to 
ChangeDisplaySettings succeeded.

If the program called ChangeDisplaySettings with setting the same colour 
depth as it's previous call then, if no other setting had changed, it 
would not expect to receive a WM_DISPLAYCHANGE message as it's view of 
the screen's colour depth had not changed.

> In addition, your tests should:
>   *  check that changing the screen resolution causes a
> WM_DISPLAYCHANGE message to be sent;
>   *  if there are no other tests, ChangeDisplaySettings should be
> checked to see what happens with invalid arguments (for completeness);
>   *  the return value is not checked for expected values (see
> http://msdn2.microsoft.com/en-us/library/ms533260(VS.85).aspx);
>   *  GetLastError() is not checked to see what it is set to (see other
> tests for examples);
>   *  restore the display settings back to what they were before the tests.  
found the existing test now which seem to cover much of this but will 
try and extend them further. the new patch seems to pass ok

new patch:

---
 dlls/winex11.drv/desktop.c     |   12 +++++++++++-
 dlls/winex11.drv/event.c       |    1 +
 dlls/winex11.drv/winpos.c      |    4 ++--
 dlls/winex11.drv/x11drv.h      |    1 +
 dlls/winex11.drv/x11drv_main.c |    3 ++-
 dlls/winex11.drv/xrandr.c      |   12 +++++++++++-
 6 files changed, 28 insertions(+), 5 deletions(-)
--------------1.5.2.5
Content-Type: text/x-patch; 
name="c65ac7a7c58d8d36c05bf789daf2c5f3cbb64959.diff"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; 
filename="c65ac7a7c58d8d36c05bf789daf2c5f3cbb64959.diff"

diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c
index 1699318..04ac482 100644
--- a/dlls/winex11.drv/desktop.c
+++ b/dlls/winex11.drv/desktop.c
@@ -94,8 +94,18 @@ static LONG X11DRV_desktop_SetCurrentMode(int mode)
          * to run. X11drv performs a color depth conversion if needed.
          */
     }
+
     TRACE("Resizing Wine desktop window to %dx%d\n", 
dd_modes[mode].dwWidth, dd_modes[mode].dwHeight);
-    X11DRV_resize_desktop(dd_modes[mode].dwWidth, dd_modes[mode].dwHeight);
+
+    if (client_bpp != dd_modes[mode].dwBPP ||
+        screen_width != dd_modes[mode].dwWidth ||
+        screen_height != dd_modes[mode].dwHeight)
+    {
+        // Only call if the desktop has acutally changed
+        client_bpp = dd_modes[mode].dwBPP;
+        X11DRV_resize_desktop(dd_modes[mode].dwWidth, 
dd_modes[mode].dwHeight);
+    }
+
     return DISP_CHANGE_SUCCESSFUL;
 }

diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c
index cd6eaf6..6795d90 100644
--- a/dlls/winex11.drv/event.c
+++ b/dlls/winex11.drv/event.c
@@ -1106,6 +1106,7 @@ LRESULT X11DRV_WindowMessage( HWND hwnd, UINT msg, 
WPARAM wp, LPARAM lp )
     case WM_X11DRV_SET_WIN_FORMAT:
         return X11DRV_set_win_format( hwnd, (XID)wp );
     case WM_X11DRV_RESIZE_DESKTOP:
+        client_bpp = wp;
         X11DRV_resize_desktop( LOWORD(lp), HIWORD(lp) );
         return 0;
     default:
diff --git a/dlls/winex11.drv/winpos.c b/dlls/winex11.drv/winpos.c
index ad62236..6c68858 100644
--- a/dlls/winex11.drv/winpos.c
+++ b/dlls/winex11.drv/winpos.c
@@ -564,7 +564,7 @@ void X11DRV_resize_desktop( unsigned int width, 
unsigned int height )

     if (GetWindowThreadProcessId( hwnd, NULL ) != GetCurrentThreadId())
     {
-        SendMessageW( hwnd, WM_X11DRV_RESIZE_DESKTOP, 0, MAKELPARAM( 
width, height ) );
+        SendMessageW( hwnd, WM_X11DRV_RESIZE_DESKTOP, client_bpp, 
MAKELPARAM( width, height ) );
     }
     else
     {
@@ -573,7 +573,7 @@ void X11DRV_resize_desktop( unsigned int width, 
unsigned int height )
                       virtual_screen_rect.right - virtual_screen_rect.left,
                       virtual_screen_rect.bottom - virtual_screen_rect.top,
                       SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE );
-        SendMessageTimeoutW( HWND_BROADCAST, WM_DISPLAYCHANGE, screen_bpp,
+        SendMessageTimeoutW( HWND_BROADCAST, WM_DISPLAYCHANGE, client_bpp,
                              MAKELPARAM( width, height ), 
SMTO_ABORTIFHUNG, 2000, NULL );
     }

diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index 52670e8..8c28f81 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -525,6 +525,7 @@ extern Window root_window;
 extern unsigned int screen_width;
 extern unsigned int screen_height;
 extern unsigned int screen_bpp;
+extern unsigned int client_bpp;
 extern unsigned int screen_depth;
 extern RECT virtual_screen_rect;
 extern unsigned int text_caps;
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
index d445e64..58dd680 100644
--- a/dlls/winex11.drv/x11drv_main.c
+++ b/dlls/winex11.drv/x11drv_main.c
@@ -71,6 +71,7 @@ Visual *visual;
 unsigned int screen_width;
 unsigned int screen_height;
 unsigned int screen_bpp;
+unsigned int client_bpp;
 unsigned int screen_depth;
 RECT virtual_screen_rect;
 Window root_window;
@@ -517,7 +518,7 @@ static BOOL process_attach(void)
         }
     }
     if (!screen_depth) screen_depth = DefaultDepthOfScreen( screen );
-    screen_bpp = depth_to_bpp( screen_depth );
+    client_bpp = screen_bpp = depth_to_bpp( screen_depth );

     XInternAtoms( display, (char **)atom_names, NB_XATOMS - 
FIRST_XATOM, False, X11DRV_Atoms );

diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
index fd930a2..e977aa1 100644
--- a/dlls/winex11.drv/xrandr.c
+++ b/dlls/winex11.drv/xrandr.c
@@ -179,6 +179,7 @@ static LONG X11DRV_XRandR_SetCurrentMode(int mode)
     unsigned int i;
     int j;
     DWORD dwBpp = screen_bpp;
+    DWORD modeBPP = dd_modes[mode].dwBPP;

     wine_tsx11_lock();
     root = RootWindow (gdi_display, DefaultScreen(gdi_display));
@@ -188,6 +189,7 @@ static LONG X11DRV_XRandR_SetCurrentMode(int mode)
     {
         FIXME("Cannot change screen BPP from %d to %d\n", dwBpp, 
dd_modes[mode].dwBPP);
     }
+
     mode = mode%real_xrandr_modes_count;

     TRACE("Changing Resolution to %dx%d @%d Hz\n",
@@ -229,7 +231,15 @@ static LONG X11DRV_XRandR_SetCurrentMode(int mode)
     wine_tsx11_unlock();
     if (stat == RRSetConfigSuccess)
     {
-        X11DRV_resize_desktop( dd_modes[mode].dwWidth, 
dd_modes[mode].dwHeight );
+        if (client_bpp != modeBPP ||
+            screen_width != dd_modes[mode].dwWidth ||
+            screen_height != dd_modes[mode].dwHeight)
+        {
+            // Only call if the desktop has acutally changed
+            client_bpp = modeBPP;
+            X11DRV_resize_desktop( dd_modes[mode].dwWidth, 
dd_modes[mode].dwHeight );
+        }
+
         return DISP_CHANGE_SUCCESSFUL;
     }







More information about the wine-devel mailing list