Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

renderer_vulkan: Address vulkan surface recreation issues #56

Closed
wants to merge 3 commits into from
Closed

renderer_vulkan: Address vulkan surface recreation issues #56

wants to merge 3 commits into from

Conversation

raphaelthegreat
Copy link
Collaborator

@raphaelthegreat raphaelthegreat commented Apr 6, 2024

Finally decided to stop procrastinating on this issue and decided to take a proper look on why rotation was such a pain area on android. I think it should be actually fixed now, but needs testing to ensure. While testing I encountered and fixed the following errors:

  • vkQueuePresentKHR didn't handle vk::Result::eSurfaceLostKHR like vkAcquireNextImageKHR did. It is entirely possible for the rotation to occur while the window thread is presenting and it would crash when that occurred. Fixed that by handling the exception and skipping presentation of that frame. The next loop is responsible for acquiring a new image (which will recreate the swapchain).

  • This sadly didn't fix the rotation crashes entirely. It would still crash with the following error in logcat:
    native_window_api_connect returned an error: Invalid argument (-22)
    The crash occurred inside vkCreateAndroidSurfaceKHR. Reading through the code, it seems this function complains that we are trying to create 2 surface handles for the same ANativeWindow, which made me suspect that android doesn't always change the surface (current code assumes this is always the case). And lo and behold, it doesn't, so added a check to only notify the renderer if it gets changed (mainly saw it when quickly flipping the phone, so the surface dimensions stay the same)

  • On my device there also an additional error that can crash the app, which doesn't look related to vulkan:
    android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{32d5cff u0 org.citra.citra_emu.canary.debug/org.citra.citra_emu.utils.ForegroundService}

This might be Samsung specific, or it may not. Please test this PR and check if the crashes are fixed to confirm that
Fixes
#55

@stepsy
Copy link

stepsy commented Apr 6, 2024

I'll test it.
Edit: Nope. rotating for 20+ times Crashed the app. Doesn't matter what orientation you started from or what renderer is being used.
(maybe it would be best to put a orientation option in general setting? Most people play in landscape mode.)
citra_log.txt.old.txt

@raphaelthegreat
Copy link
Collaborator Author

@JaffaCat Can you also test this to see if crashes get fixed in your device?

* Using so many threads might starve other system work, especially if pipelines take way to long
@JaffaCat
Copy link

JaffaCat commented Apr 7, 2024

@JaffaCat Can you also test this to see if crashes get fixed in your device?

Tested the fix in Yo-kai Watch 3, Super Mario 3D Land and Pokemon: Alpha Sapphire with no crashes.

@raphaelthegreat raphaelthegreat closed this by deleting the head repository Apr 12, 2024
@Miguel-hrvs
Copy link
Contributor

Why this wasn't merged?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants