Fix D3D12 pixel history MSAA test issues.

[jristic]

• Add missing resource transitions for the dispatch copy to fix validation issues encountered while running the test.
• Adjust the component mapping to fix stencil copies failing to output anything, which manifested as the fragment only showing one primitive without correct output data.

Description

Before:

After:

Sampling stencil via an SRV was different in D3D11 and I've had trouble finding information about how this is intended to be done in D3D12, the shader component mapping is the best I've come up with thus far. It appears that the plane slice works for a Texture2D, but Texture2DMS does not provide such a specifier. I'm happy to change it if there is a different preferred method.

[jristic]

Actually looking back the D3D11 method, this appears to work in D3D12 as well. If the stencil's texture resource in the copy shader is instead declared as a Texture2DMSArray of uint2 and then we explicitly use the second component, that works as well. So that's another option.

[Zorro666]

Thanks for chasing this down to get a fix. I think both approaches are achieving the same goal which is to read the green (second component) from the texture.

My opinion (and it is subjective and not objective and looking to have a discussion on the topic) is changing the D3D12 shader to match the D3D11 shader is more consistent instead of using SRV shader component mapping.

It achieves the same effect, locally I did this change and it also fixed the problem (locally I have re-enabled the MSAA tests and they are now passing)

diff --git a/renderdoc/data/hlsl/d3d12_pixelhistory.hlsl b/renderdoc/data/hlsl/d3d12_pixelhistory.hlsl
index deb79b61e..af7206f28 100644
--- a/renderdoc/data/hlsl/d3d12_pixelhistory.hlsl
+++ b/renderdoc/data/hlsl/d3d12_pixelhistory.hlsl
@@ -40,7 +40,7 @@ Texture2DArray<float> copyin_depth : register(t0);
 Texture2DArray<uint> copyin_stencil : register(t1);

 Texture2DMSArray<float> copyin_depth_ms : register(t2);
-Texture2DMSArray<uint> copyin_stencil_ms : register(t3);
+Texture2DMSArray<uint2> copyin_stencil_ms : register(t3);

 Texture2DArray<float4> copyin_float : register(t4);
 Texture2DMSArray<float4> copyin_float_ms : register(t5);
@@ -70,7 +70,7 @@ RWBuffer<int> copyout_int : register(u4);
     }
     else if(copy_stencil)
     {
-      uint val = copyin_stencil_ms.sample[src_coord.z][uint3(src_coord.xy, src_coord.w)];
+      uint val = copyin_stencil_ms.sample[src_coord.z][uint3(src_coord.xy, src_coord.w)].g;
       copyout_stencil[dst_slot] = val;
     }
     else

Perhaps not for this PR but I wonder if changing the non-MSAA stencil read in the shader to use uint2 and be more like D3D11 is worthwhile

[jristic]

I agree that the second option is preferable and have updated the commit.

As for the non-MSAA path, that's currently unused because the dispatch copy path is only used for MSAA targets so I'm not sure it has been verified to work as written. It's a bit odd to me to have both the CopyTextureRegion path and the dispatch copy path if the latter could handle both.
I could change it but testing that it works would be a different matter, as when I tried trivially flipping it over to have everything use the dispatch copy things started failing in strange ways.

[jristic]

I identified an additional problem using this sample when attempting to get history on the depth stencil, and have added a fix for that. It's a knock-on from my my previous fix to copy the stencil value on depth-stencil history. There's no before because without the fix it asserts.

After:

I hope I'm not interrupting your holidays, I don't mind at all if these PRs sit in the queue until things resume in the new year.

[Zorro666]

Thank you for the follow up fix for the memory stomp. I walked the code and checked other instances of D3D12CopyPixelParams and didn't spot any other places where depthCopy had not been set to true

I will enable the MSAA automated tests for D3D12 Pixel History now