Re: [PATCH 1/4] ddraw: Restore resolution on EXCLUSIVE->NORMAL switch

[Henri Verbeet]

On 21 May 2013 13:18, Stefan Dösinger  wrote:
> The new code does not deliver the resolution change messages to the
> correct window, see the todo_wine markers in the next patch. Getting
> this right isn't simply a matter of passing the correct window to
> wined3d_device_restore_fullscreen_window because the delivery of the
> correct messages in the new window == old window case is mostly a lucky
> side effect of the window style hacks. Those hacks fail if we pass a
> different window, because the style of the new window is different from
> what wined3d expects, so it doesn't send any messages.

Well, wined3d_device_restore_fullscreen_window() isn't really supposed
to generate messages at all, the resize on mode changes happens in
device_parent_mode_changed(). The implication seems to be that
restoring the display mode is supposed to happen after the new
swapchain is created, but before "ddraw->cooperative_level" is
updated.
> /*****************************************************************************
> -
> - IDirectDraw7::RestoreDisplayMode
> -
> -
> -
> - Restores the display mode to what it was at creation time. Basically.
> -
> -
> -
> - Returns
> -
> - DD_OK on success
> -
> - DDERR_NOEXCLUSIVE mode if the device isn't in fullscreen mode
> -
> -
> - *****************************************************************************/

I don't think that really adds anything.
> -static HRESULT WINAPI ddraw7_SetCooperativeLevel(IDirectDraw7 *iface, HWND window, DWORD cooplevel)
> +static HRESULT WINAPI ddraw_set_cooperative_level(struct ddraw *ddraw, HWND window,
> -
>    DWORD cooplevel, BOOL restore_mode)

Shadowing the global "restore_mode" is a bit unfortunate.
> - TRACE("iface %p, window %p, flags %#x.\n", iface, window, cooplevel);
> - TRACE("ddraw %p, window %p, flags %#x, restore_mode %s.\n", ddraw, window, cooplevel,
> -
>        restore_mode ? "true" : "false");

I think %#x is generally clear enough for BOOLs.
>  if(cooplevel & DDSCL_ALLOWREBOOT)
> -
>    WARN("(%p) Unhandled flag DDSCL_ALLOWREBOOT, harmless\n", This);
> -
>    WARN("(%p) Unhandled flag DDSCL_ALLOWREBOOT, harmless\n", ddraw);
> if(cooplevel & DDSCL_ALLOWMODEX)
> -
>    WARN("(%p) Unhandled flag DDSCL_ALLOWMODEX, harmless\n", This);
> -
>    WARN("(%p) Unhandled flag DDSCL_ALLOWMODEX, harmless\n", ddraw);
> if(cooplevel & DDSCL_FPUSETUP)
> -
>    WARN("(%p) Unhandled flag DDSCL_FPUSETUP, harmless\n", This);
> -
>    WARN("(%p) Unhandled flag DDSCL_FPUSETUP, harmless\n", ddraw);

The "(%p)" there is pretty pointless, I usually remove them if I'm
editing the line anyway.
> @@ -86,6 +86,7 @@ struct ddraw
>  /* DirectDraw things, which are not handled by WineD3D */
>  DWORD                   cooperative_level;
> - BOOL                    setcooplevel_ddraw1;

We should probably consolidate the various BOOLs in struct ddraw into
a flags field.