Re: [PATCH] wined3d: Restore the Topmost state upon exiting fullscreen, when used from DXGI.

[Gabriel Ivăncescu]

Hi Henri,

Thanks for the review!

On 02/07/2020 17:08, Henri Verbeet wrote:
> On Thu, 2 Jul 2020 at 17:14, Gabriel Ivăncescu  wrote:
> > @@ -416,6 +416,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d11_swapchain_SetFullscreen
> >       wined3d_swapchain_get_desc(swapchain->wined3d_swapchain, &swapchain_desc);
> >       swapchain_desc.output = dxgi_output->wined3d_output;
> >       swapchain_desc.windowed = !fullscreen;
> > - swapchain_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE;
> >  hr = wined3d_swapchain_state_set_fullscreen(state, &swapchain_desc, NULL);
> >  wined3d_mutex_unlock();
> >  if (FAILED(hr))
> >
> > @@ -2243,6 +2244,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d12_swapchain_SetFullscreen
> >           goto fail;
> >       wined3d_mutex_lock();
> >       wined3d_desc.windowed = !fullscreen;
> > - wined3d_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE;
> >  hr = wined3d_swapchain_state_set_fullscreen(swapchain->state, &wined3d_desc, NULL);
> >  wined3d_mutex_unlock();
> >  if (FAILED(hr))
>
> Any reason to not add this to DXGI_WINED3D_SWAPCHAIN_FLAGS?

No reason, I'll do that, thanks. I'm not very familiar with the code as 
you can probably figure it out. :-)
> > - /* DXGI restores WS_EX_TOPMOST, unlike Direct3D 9 and below */
> > - if ((state->desc.flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) && !(state->exstyle & WS_EX_TOPMOST))
> > -
> >    window_pos_flags &= ~SWP_NOZORDER;
>
> Wined3d doesn't particularly care about DXGI or d3d9. If you wanted to
> add a comment to this effect you'd add it to the place that sets the
> flag, but the flag also seems straightforward enough on its own.

Noted.
> > @@ -2243,6 +2247,8 @@ HRESULT CDECL wined3d_swapchain_state_set_fullscreen(struct wined3d_swapchain_st
> >           RECT *window_rect = NULL;
> >           if (state->desc.flags & WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT)
> >               window_rect = &state->original_window_rect;
> > -
> >    if (swapchain_desc->flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE)
> > -
> >        state->desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE;
>
> We currently ignore flag changes in
> wined3d_swapchain_state_set_fullscreen(). That could be changed, but
> then you'd have to handle other flags potentially changing as well,
> and I don't think there's a need.
> > diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h
> > index 346d1d0..cee46d3 100644
> > --- a/include/wine/wined3d.h
> > +++ b/include/wine/wined3d.h
> > @@ -905,6 +905,7 @@ enum wined3d_shader_type
> >   #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE                        0x00008000u
> >   #define WINED3D_SWAPCHAIN_IMPLICIT                              0x00010000u
> >   #define WINED3D_SWAPCHAIN_HOOK                                  0x00020000u
> > +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE                       0x00040000u
>
> Ideally WINED3D_SWAPCHAIN_TOPMOST_RESTORE would simply be merged with
> WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT, but the naming should at least
> be consistent.

So in that case I'd simply check to see if the rect is NULL or not, to 
restore TOPMOST, within the function that restores it, right?

As for the name, how about WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE? (I'll 
of course send it as a separate patch)

Thanks,
Gabriel