Improve handling of surface change/resize events in graphics backends #6330
Conversation
|
@stenzek just tested this on Shield TV and it works perfectly using both backends, great work! |
| std::lock_guard<std::mutex> guard(m_surface_change_mutex); | ||
| new_surface_handle = m_new_surface_handle; | ||
| m_new_surface_handle = nullptr; | ||
| m_surface_handle = m_new_surface_handle; |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| std::lock_guard<std::mutex> lock(m_surface_change_mutex); | ||
| m_new_surface_handle = new_surface_handle; | ||
| m_surface_needs_change.Set(); | ||
| } |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
We now differentiate between a resize event and surface change/destroyed event, reducing the overhead for resizes in the Vulkan backend. It is also now now safe to change the surface multiple times if the video thread is lagging behind.
Previously, this could cause a race condition which resulted in the Vulkan backend attempting to acquire a swap chain image from a now non-existant surface. By ensuring the backend knows about the surface before a frame is presented, this race does not happen.
|
Tested this commit / build 5.0-6350 on macOS 10.12+: when entering FullScreen Mode it will only display a black screen with one tile on the upper left displaying the current game. |
|
@gilcel Thanks, I will look into it. |
|
Here's a screenshot of the fullscreen issue (I built and tested it with WX and QT5 5.1.0 = same problem)
|
|
YES! PR #6394 works. I edited Render.cpp to add the GLInterface->Update(); code and Fullscreen mode works again properly on macOS. Thanks! |
Currently, our handling of surface change and resize events is kind of a mess. We have different methods depending on the selected backend, as well as platform (e.g. OpenGL on Android can handle surface changes but no other platform can). The implementation also enabled a race with the video thread, where Dolphin could render to an invalid system window surface, leading to display corruption when the surface was recreated, for example, when the phone was rotated.
The new implementation ensures all platforms behave similar in handling surface changes. Currently, the surface change is only hooked up to Qt, but on the desktop the surface will not change during runtime. However, this enables us to change the window in the future, which can enable possibilities such as toggling render-to-main-window at runtime.
Resize events have also been reworked, instead of relying on the polled backbuffer dimensions from GLInterface, we propogate resize events from the UI to the graphics backend, and respond accordingly. This can reduce overhead very slightly, as we no longer need to repeatedly call GetClientRect() on Windows. In X11, we still need the event loop as we're using a separate display. I'm planning on eliminating this in the future by passing the display from the UI instead, and using XCB instead.
Most importantly, this should fix the bug on Android where rotating the phone or pausing/resuming, when using Vulkan, would cause the app to crash.