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

D3D12: Only use framebuffer integer descriptor if allocated #12087

Conversation

Dentomologist
Copy link
Contributor

@Dentomologist Dentomologist commented Aug 4, 2023

Verify that DXFramebuffer's integer RTV descriptor's cpu_handle has been allocated before using it, and if it hasn't use the non-integer RTV descriptor instead. This fixes a Dolphin crash in Twilight Princess, and possibly other games (Issue 13312).

As an optimization to save space in the descriptor heap, DXFramebuffer's integer descriptor is only initialized if the given abstract texture format has different integer and non-integer RTV formats. This previously wasn't accounted for by GetIntRTVDescriptorArray, which could cause DX12::Gfx::BindFramebuffer to call OMSetRenderTargets with an invalid descriptor which would lead to a crash.

Triggering the bug was fortunately rare because integer formats are only used when blending is disabled and logic ops are enabled. Furthermore, the standard integer abstract format is RGBA8 which has different integer and non-integer RTV formats, causing the integer descriptor to be initialized and avoiding the bug.

The crash started appearing in #11850 because it changed the swapchain's abstract texture format from RGBA8 to RGB10_A2. Unlike RGBA8, RGB10_A2 has the same integer and non-integer RTV formats and so the bug can be triggered if the other requirements are met.

Verify that DXFramebuffer's integer RTV descriptor's cpu_handle has been
allocated before using it, and if it hasn't use the non-integer RTV
descriptor instead. This fixes a Dolphin crash in Twilight Princess, and
possibly other games (Issue 13312).

As an optimization to save space in the descriptor heap, DXFramebuffer's
integer descriptor is only initialized if the given abstract texture
format has different integer and non-integer RTV formats. This
previously wasn't accounted for by GetIntRTVDescriptorArray, which could
cause DX12::Gfx::BindFramebuffer to call OMSetRenderTargets with an
invalid descriptor which would lead to a crash.

Triggering the bug was fortunately rare because integer formats are only
used when blending is disabled and logic ops are enabled. Furthermore,
the standard integer abstract format is RGBA8 which has different
integer and non-integer RTV formats, causing the integer descriptor to
be initialized and avoiding the bug.

The crash started appearing in a2702c6 because it changed the
swapchain's abstract texture format from RGBA8 to RGB10_A2. Unlike
RGBA8, RGB10_A2 has the same integer and non-integer RTV formats and so
the bug can be triggered if the other requirements are met.
Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Code LGTM, untested.

@AdmiralCurtiss AdmiralCurtiss merged commit b0dc067 into dolphin-emu:master Aug 10, 2023
11 checks passed
@Dentomologist Dentomologist deleted the dx12_use_correct_framebuffer_descriptor branch August 10, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants