renderdoc
renderdoc copied to clipboard
Pixel history fixes
Description
D3D12
Fixes found when testing pixel history on Texture 1D, 2D, 3D and different formats
- Increase the padding to make the event info structures and members aligned to 16-bytes and 12-bytes (48-bytes) to fix problem when copying from a source texture with 12-bytes per pixel i.e. R32G32B32.
- Force the scratch render targets to be Texture2D to fix device timeouts when performing history on Texture1D and Texture3D targets
- Fixed copy source parameters and subresource for pixel history on slice > 0 of a Texture3D (thanks to Jovan for the patch).
- Pixel History on UINT/SINT targets had incorrect "Tex After" in the per fragment data because the integer data is being interpreted as float data but it needs to be cast (Vulkan has the same problem)
- Simplified the image state tracking code. The CPU tracking of image states does not include state changes which haven't been replayed on the CPU yet but will be submitted to the GPU earlier than the current CPU event i.e. a command list with barrier changes is recorded after the current command list but submitted before it.
- Change the D3D12_Pixel_History test to have multiple command lists to demonstrate the problem with CPU tracking of image states.
UI
- Draw the background colour for UINT and SINT targets
Example of history after the fix for a R32G32B32 target mip = 2
Example of the UI drawing the background colour for UINT target
Regarding the changes to getting source image state, it appears states like resolve source/dest aren't covered, though I don't believe that's an exhaustive list of all the possible states the target in question could be in.
For instance this sample: https://drive.google.com/file/d/1P81Nd4_BjuNoIUIScDEPT1SjqTUHbahg/view?usp=drive_link
produces the following validation error for EID 42:
D3D12 ERROR: ID3D12CommandList::ResourceBarrier: Before state (0x400: D3D12_RESOURCE_STATE_COPY_DEST) of resource (0x000001FDFBA5ADF0:'Committed Resource D3D12_RESOURCE_DIMENSION_TEXTURE2D ResourceId::309') (subresource: 0) specified by transition barrier does not match with the state (0x1000: D3D12_RESOURCE_STATE_RESOLVE_DEST) specified in the previous call to ResourceBarrier [ RESOURCE_MANIPULATION ERROR # 527: RESOURCE_BARRIER_BEFORE_AFTER_MISMATCH]
Hey Jovan. Thanks for pointing this out. Updated the state possible options to include this state. I had a look at the possible write states and I think this is now covered, please do suggest any more write states that have been missed. The current logic for choosing the RenderTarget state is this (with depth-stencil states being handled on top of this).
D3D12_RESOURCE_STATES resourceState = D3D12_RESOURCE_STATE_RENDER_TARGET;
if(IsUavWrite(usage))
resourceState = D3D12_RESOURCE_STATE_UNORDERED_ACCESS;
else if(IsResolveWrite(usage))
resourceState = D3D12_RESOURCE_STATE_RESOLVE_DEST;
else if(IsCopyWrite(usage))
resourceState = D3D12_RESOURCE_STATE_COPY_DEST;
I think the other place that the source states might be wrong is if a DSV has the read only flag set but is in the DEPTH_WRITE state (assuming that is allowed by the API). I haven't confirmed that this is possible though.
I think the other place that the source states might be wrong is if a DSV has the read only flag set but is in the DEPTH_WRITE state (assuming that is allowed by the API). I haven't confirmed that this is possible though.
Thanks Jovan. I would say that sounds like an edge case (if it is possible) and we can wait for users to report the bug with reproduction steps.
It's not something I would consider a blocker to getting these fixes in, I will make a note to test it when I get a chance.