On all platforms, this would result in out of bounds accesses when getting the component sizes (which uses stuff from VertexLoader_Position.h/VertexLoader_TextCoord.h/VertexLoader_Normal.h). On platforms other than x64 and ARM64, this would also be out of bounds accesses when getting function pointers for the non-JIT vertex loader (in VertexLoader_Position.cpp etc.). Usually both of these would get data from other entries in the same multi-dimensional array, but the last few entries would be truly out of bounds. This does mean that an out of bounds function pointer can be called on platforms that don't have a JIT vertex loader, but it is limited to invalid component formats with values 5/6/7 due to the size of the bitfield the formats come from, so it seems unlikely that this could be exploited in practice.
This issue affects a few games; Def Jam: Fight for New York (https://bugs.dolphin-emu.org/issues/12719) and Fifa Street are known to be affected.
I have not done any hardware testing for this PR specifically, though I *think* I previously determined that at least a value of 5 behaves the same as float (4). That's what I implemented in any case. I did previously determine that both Def Jam: Fight for New York and Fifa Street use an invalid normal format, but don't actually have lighting enabled when that normal vector is used, so it doesn't change rendering in practice.
The color component format also has two invalid values, but VertexLoader_Color.h/.cpp do check for those invalid ones and return a default value instead of doing an out of bounds access.
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.
To further increase the accuracy of the post process phase, I've added (scRGB) HDR support, which is necessary
to fully display the PAL and NTSC-J color spaces, and also to improve the quality of post process texture samplings and
do them in linear space instead of gamma space (which is very important when playing at low resolutions).
For SDR, the quality is also slightly increased, at least if any post process runs, as the buffer is now
R10G10B10A2 (on Vulkan, DX11 and DX12) if supported; previously it was R8G8B8A8 but the alpha bits were wasted.
Gamma correction is arguably the most important thing as Dolphin on Windows outputted in "sRGB" (implicitly)
as that's what Windows expects by default, though sRGB gamma is very different from the gamma commonly used
by video standards dating to the pre HDR era (roughly gamma 2.35).
Additionally, the addition of HDR support (which is pretty straight forward and minimal), added support for
our own custom AutoHDR shaders, which would allow us to achieve decent looking HDR in Dolphin games without
having to use SpecialK or Windows 11 AutoHDR. Both of which don't necessarily play nice with older games
with strongly different and simpler lighting. HDR should also be supported in Linux.
Development of my own AutoHDR shader is almost complete and will come next.
This has been carefully tested and there should be no regression in any of the different features that Dolphin
offers, like multisampling, stereo rendering, other post processes, etc etc.
Fixes: https://bugs.dolphin-emu.org/issues/8941
Co-authored-by: EndlesslyFlowering <EndlesslyFlowering@protonmail.com>
Co-authored-by: Dogway <lin_ares@hotmail.com>
DX12 would often crash when starting and stopping the emulation many times, due to the device enumerator failing for some reason. Checking for success fixes it.
The whole ownership model was getting a bit of a mess, with a some
of special cases to deal with. And I'm planning to make it even more
complex in the future.
So here is some upfront work to convert it over to reference counted
pointers.
BindFramebuffer depends on the pipeline which might not be set yet.
That's why the framebuffer dirty flag exists in the first place.
I assume BindFramebuffer was called directly here, in order to handle
the texture state transitions necessary for DiscardResource.
The state is tracked anyway, so we can just issue those transitions there
too and defer binding the actual framebuffer.
Fixes an issue in Zelda Twilight Princess with EFB depth peeks.
Dolphin would bind a frame buffer which doesn't have an integer format
descriptor for the color target before binding the new pipeline.
So it would accidentally use the 0 descriptor.
Debug layer error:
D3D12 ERROR: ID3D12CommandList::OMSetRenderTargets:
Specified CPU descriptor handle ptr=0x0000000000000000 does not refer to
a location in a descriptor heap. pRenderTargetDescriptors[0] is the issue.
[ EXECUTION ERROR #646: INVALID_DESCRIPTOR_HANDLE]
Fixes the following error in the D3D12 debug layer:
D3D12 WARNING: ID3D12Device::CreateCommittedResource:
Ignoring InitialState D3D12_RESOURCE_STATE_UNORDERED_ACCESS.
Buffers are effectively created in state D3D12_RESOURCE_STATE_COMMON.
[ STATE_CREATION WARNING #1328: CREATERESOURCE_STATE_IGNORED]
Fixes the following error in the D3D12 debug layer:
D3D12 ERROR: ID3D12DescriptorHeap::GetGPUDescriptorHandleForHeapStart:
GetGPUDescriptorHandleForHeapStart is invalid to call on a descriptor
heap that does not have DESCRIPTOR_HEAP_FLAG_SHADER_VISIBLE set.
If the heap is not supposed to be shader visible, then
GetCPUDescriptorHandleForHeapStart would be the appropriate method
to call. That call is valid both for shader visible and non shader
visible descriptor heaps.
[ STATE_GETTING ERROR #1315: DESCRIPTOR_HEAP_NOT_SHADER_VISIBLE]
TellowKrinkle