Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure renderer-api-driven frame wait is only invoked when a frame is actually drawn #5846

Merged
merged 3 commits into from
Jun 17, 2023

Conversation

peppy
Copy link
Member

@peppy peppy commented Jun 17, 2023

I noticed very long pauses when loading song select on my windows PC. Tracking this down, the game was getting stuck on the D3D11Swapchain wait handle until the full one second timeout.

It turns out that the wait handle will not be set unless a frame finishes rendering. The fail case is where the update thread takes over 100ms to prepare a frame, causing TripleBuffer.GetForRead to timeout and return null, which early exits the draw frame method.

This would happen anywhere the game takes slightly too long to render an update frame. I could reproduce:

  • Entering song select
  • Opening settings
  • Opening beatmap listing

To confirm / reproduce this issue, remove the 1000ms timeout from WaitOne in veldrid and add a single Thread.Sleep(150) in any update thread code, and watch the game deadlock indefinitely.

For good measure, here's a visual example of this happening:

image

… actually drawn

I noticed very long pauses when loading song select on my windows PC.
Tracking this down, the game was getting stuck on the D3D11Swapchain
wait handle
(https://github.com/ppy/veldrid/blob/d4340776eba0541aa3598856dafccf25633072f2/src/Veldrid/D3D11/D3D11Swapchain.cs#L303)
until the full one second timeout.

It turns out that the wait handle will not be set unless a frame
finishes rendering. The fail case is where the update thread takes over
100ms to prepare a frame, causing `TripleBuffer.GetForRead` to timeout
and return `null`, which early exits the draw frame method.

This would happen anywhere the game takes slightly too long to render an
update frame. I could reproduce:

- Entering song select
- Opening settings
- Opening beatmap listing

To confirm / reproduce this issue, remove the 1000ms timeout from
`WaitOne` in veldrid and add a single `Thread.Sleep(150)` in any update
thread code, and watch the game deadlock indefinitely.
@peppy peppy requested a review from a team June 17, 2023 15:19
@peppy peppy added the next-release Pull requests which are almost there label Jun 17, 2023
@bdach bdach self-requested a review June 17, 2023 15:32
@bdach bdach requested a review from frenzibyte June 17, 2023 16:04
@frenzibyte
Copy link
Member

On Metal, the display link callback is always signalled regardless of whether a frame was drawn, so this doesn't quite have an effect on renderer. Testing confirms that anyways.

@frenzibyte frenzibyte merged commit f5e12df into ppy:master Jun 17, 2023
@peppy peppy deleted the fix-excess-frame-wait branch June 21, 2023 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants