FNA.Core: Fix display change out of bounds exception

[jfmu]

i have this tested. This fixes an out of bounds exception when a display adapter suddenly disappears (i tested with unplugging miniDP cable). For some people this seems to happen during powersave (they afk in a game for a while and crash)

[flibitijibibo]

When this gets fired, do we get any events for displays getting added/removed? I think there's an event SDL fires when the display array changes without GetNumDisplays getting called first.

[jfmu]

Hi, im back from work and i investigated a bit. So scenario first.

I dont have the exact setup of the player that reported the bug , i just tried until i got the same error. What i did:

I have a notebook and a mini-DP plug to an external monitor. I have the external monitor marked as the primary monitor. When the game is on the laptop-monitor (the seconary), i unplug the external (primary) monitor. You are right, there is an event:

and this event is not in the SDL_DisplayEventID enum. But in SDL2 sourcecode i found:

And after that, displayIndex 1 gets a disconnect Displayevent:

So im not really sure about it but if you move it here:

then the crash bug is gone and i believe this is because after the current invocation of AdaptersChanged() there is an access to .Adapters (yellow), and SDL didnt delete the adapter yet.

Should i just edit the pull request and just add AdaptersChanged() at the end?

[flibitijibibo]

The part that makes the timing weird is that the event in that last screenshot is queued, so by the time we read it in SDL_PollEvent it should have already done the changes to the display list within SDL, and AdaptersChanged is what allows us to reinit the list of adapters. It almost seems like the bug is that the display index of the window is wrong; if there's only 1 adapter at that moment and the display index of the window is still 1 (as in, the second display) then we've got a timing bug in SDL.

So, to confirm: At the moment we assign currentAdapter, we want to know the values of Adapters.Count as well as displayIndex. If we can confirm that displayIndex is too high then we've got an SDL bug to fix!

Alternatively, we can modify AdaptersChanged to do the query right then and there, to ensure that SDL_GetNumDisplays is called before calling SDL_GetWindowDisplayIndex, meaning this line...

https://github.com/FNA-XNA/FNA/blob/master/src/Graphics/GraphicsAdapter.cs#L258

... will look more like this block:

https://github.com/FNA-XNA/FNA/blob/master/src/Graphics/GraphicsAdapter.cs#L147-L149

[jfmu]

Yeah. You are right. I believe the timing issue is that the SDL_WINDOWEVENT_MOVED comes before DISCONNECT or CONNECT displayevents. I can see it in console log output with printf debugging if i just call AdaptersChanged() every time.

First, sorry, my laptop is kind of half-closed so i peeked in there and saw it crashed only when the primary monitor comes back online. and this is because the events come like this:

SDL_WINDOWEVENT_MOVED (where no AdaptersChanged() is) crash <-> SDL_DISPLAYEVENT (connect).

[flibitijibibo]

Huh, I wonder if it should be filtered to put the display events first... might be worth asking Sam about this. For now, let's do a bounds check in window moved, and call AdaptersChanged if it's out of bounds. We can file an SDL report for the timing issue afterward.

[jfmu]

it is because they use WM_DISPLAYCHANGE on windows for reenumerating display devices internally (thats resolution change). Im currently looking for a satifactory solution for display hotplugging.

[jfmu]

Okay so i think the only, sadly, solution to this getting the messages in order is either rearranging them in sdl, which may prove difficult or doing it like this: https://learn.microsoft.com/en-us/windows/win32/devio/registering-for-device-notification

with this GUID.

Sorry for all the text, and thanks for fixing it in FNA for now =)