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

Fix DXGI present failures caused by race conditions #12

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Fix DXGI present failures caused by race conditions #12

merged 4 commits into from
Apr 15, 2024

Conversation

praydog
Copy link
Contributor

@praydog praydog commented Apr 14, 2024

In some scenarios (seen while loading or particularly high, uncapped framerate scenarios), resetting the command allocator before the previous command list had finished executing on the GPU would cause DXGI present to fail with DXGI_ERROR_DEVICE_HUNG or some other similar error, and the game would subsequently crash.

Seen 100% during loading screens with uncapped, variable framerate in Dragon's Dogma 2. This could've also happened sometimes when the backbuffer was resized (aka on_device_reset), then reframework-d2d would release all of its resources sometimes before the command lists using them were finished.

This PR does the following things:

  • Splits the original command allocator and command list into multiple command contexts with their own fences.
  • Likewise, splits the vertex buffers into the same amount of command queues allocated to prevent conflicting access of the GPU buffer. This part might not have been necessary if it's just a constant vertex buffer.

CommandContext.hpp/.cpp are slimmed down versions of the ones used in REFramework/UEVR/FF7R-UEVR.

This could arguably be much simpler if 1 fence was just used for the original command list/allocator, however this has the drawback of lowering performance in double/triple-buffered scenarios (as is common on D3D12 with multiple backbuffers)

@cursey cursey merged commit a62e36f into cursey:main Apr 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants