[SDL] fix : SetIsReallyFullscreen is not setting real fullscreen state if API is not DX

[Jklawreszuk]

PR Details

PR fixes following bug:

• Change your app to full screen mode using ALT+ENTER combination
• Expected behaviour : IsFullScreen variable is set to true
• Actual behaviour: Is FullScreen is overriden by isReallyFullscreen variable from false to false for the first time.
  This causes you to have to press ALT+ENTER twice to exit the fullscreen mode.

Applies for : OpenGL, Vulkan (Any OS)

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[Jklawreszuk]

Once I test all the Graphics APIs (as well as Resize feature) I'm gonna reopen for review :)

[Jklawreszuk]

I couldn't even switch to fullscreen mode with DX11 api using the lastest release package :/

[Jklawreszuk]

Alright, I removed isReallyFullscreen entirely across all API's and it still seems to work

[Kryptos-FR]

Does it work on all platforms? Would be interesting to see when that code was introduced.

[Jklawreszuk]

@Kryptos-FR I tested it on: Vulkan(Linux) + OpenGL (WIN/Linux) + DX11(WIN).

Would be interesting to see when that code was introduced.

Yup, I would be glad to understand why was introduced 😅
I ping @tebjan, because its an author of this change

[Ethereal77]

I would need to look at it better, but I think for DirectX (both 11 and 12), Stride uses SetWindowAssociation(false) for the swapchain, so it does not respond to Alt+Enter presses on its own.

[Jklawreszuk]

You're right, there is such a thing like an MakeWindowAssociation that "prevents alt+tab" combination. So general conclusion is that isReallyFullScreen is might being used specific for DX11/12 API.

I'm gonna mark that code as DirectX specific instead, and it can be discused later whether is important or not

[Jklawreszuk]

Anyone? Any thoughts? 🥺

[Eideren]

Kindly pinging @Ethereal77 to check this one out again :)

[Ethereal77]

Sorry for the delay. I've been looking into this a bit and I think the problem may be a bug or lack of functionality in
the SDL backend more than a bug or problem with GameWindow or GraphicsDeviceManager. Allow me
to do a quick recap of what I've understood, and excuse me if I have not the full picture:

GameWindow (the base type)

• OnClientSizeChanged updates the size of the window when !isFullscreen.
• SetIsReallyFullscreen seems to be just an internal method to set isFullscreen
  without using the IsFullScreen property setter, because
• IsFullscreen setter sets isFullscreen value and invokes the FullscreenChanged event.
  Keep this in mind, because maybe it is important a bit later.

GraphicsDeviceManager

• Keeps its own internal state and configuration for both the GraphicsDevice and the GraphicsPresenter.
• The IsFullScreen property sets the internal state and marks the configuration as dirty
  (i.e. you need to call ApplyChanges() for the configuration to take effect).
• Window_FullscreenChanged saves the GameWindow.IsFullScreen state as the internal full-screen state,
  setting the back-buffer size as necessary, and immediately applying any changes.
  But an important detail is that this method is the event handler for GameWindow.FullscreenChanged event! ⚠️
• ChangeOrCreateDevice does many things, but most notably:
  • Prepares the configuration for the GraphicsDevice based on its own state.
  • Calls GameWindow.BeginScreenDeviceChange(isFullScreen)
  • Sets the GraphicsPresenter.IsFullScreen value, which abstracts and configures the swap-chain.
  • Changes or creates the graphics device with the current and adjusted configuration, including the
    GraphicsPresenter if needed.
  • Reads back whether the GraphicsPresenter ended in full-screen mode (isReallyFullScreen).
  • Finally, notifies the window of this GraphicsPresenter state with SetIsReallyFullScreen.
    This is important, because if setting GameWindow.IsFullScreen directly, it will raise its event,
    which is Window_FullscreenChanged, which in turn sets its state at an inappropriate time, and
    applies the changes, which in turn can call this ChangeOrCreateDevice method again ⚠️.

GameWindowSDL

• Has its own state tracking whether it is in full-screen or if intends to be
  (isFullScreenMaximized and deviceChangeWillBeFullScreen).
• BeginScreenDeviceChange(willBeFullScreen) sets deviceChangeWillBeFullScreen to willBeFullScreen.
• EndScreenDeviceChange does several things:
  • Does nothing if deviceChangeWillBeFullScreen is not set (null).
  • Sets its own internal state isFullScreenMaximized to deviceChangeWillBeFullScreen.
  • Whether if the change is from / to full-screen / windowed mode, it does the following:
    • Sets the value of GameWindow.IsFullScreen property, which as I've detailed above raises events,
      invokes the GraphicsDeviceManager, and in turn can end up calling BeginScreenDeviceChange,
      maybe with an incorrect state (I've not debugged it yet, but possible) ⚠️

So, in conclusion, I think the issue is not with isReallyFullScreen, but with GameWindowSDL, that
has its own separate state to track the window mode, and does not play by the rules. It should
set the mode in a way similar to the base GameWindow, or interact in a different way with the
GraphicsDeviceManager or SDL to know whether to update the state (not its own, but the internal
state of GameWindow). And avoid using things thay may end up in unintended reentrancy, event calls, etc.

I hope this is useful for somebody. Excuse me for any omissions and inaccuracies, and sorry for the wall of text 😁

[Eideren]

@Jklawreszuk what's the current state for this PR ? And how does it fare now that #2845 is in ?

[Jklawreszuk]

@Eideren Thanks to the recent changes my PR becomes obsolete and the problem was resolved, so I can close now 😄