Repository navigation
[Shaders] fix: SV_Coverage maps to SampleMask in the fragment stage - #3450
Conversation
|
🤖 Draft PR — automatic CI is skipped to save runner minutes.
|
xen2
left a comment
xen2
left a comment
There was a problem hiding this comment.
I think the output side doesn't work: InterfaceProcessor only marks a pixel shader stream as an output for SV_Target* and SV_Depth, so the write to SV_Coverage in the test shader is dropped silently (the generated HLSL has no SV_Coverage output).
Please add SV_COVERAGE to that check and assert the output in the test (or limit the PR to input only).
-if ((semantic.ToUpperInvariant().StartsWith("SV_TARGET") || semantic.ToUpperInvariant() == "SV_DEPTH") && stream.Value.Write)
+if ((semantic.ToUpperInvariant().StartsWith("SV_TARGET")
+ || semantic.ToUpperInvariant() == "SV_DEPTH"
+ || semantic.ToUpperInvariant() == "SV_COVERAGE")
+ && stream.Value.Write)
stream.Value.Output = true;(InterfaceProcessor.cs, line 180)
| { | ||
| var streamPointer = buffer.Add(new OpAccessChain(context.GetOrRegister(new PointerType(stream.Info.Type, Specification.StorageClass.Private)), context.Bound++, streamLayout.StreamsVariableId, [context.CompileConstant(stream.Info.StreamStructFieldIndex).Id])).ResultId; | ||
| var inputResult = buffer.Add(new OpLoad(context.Types[stream.Info.Type], context.Bound++, stream.Id, null, [])).ResultId; | ||
| var inputResult = buffer.Add(new OpLoad(context.GetOrRegister(stream.InterfaceType), context.Bound++, stream.Id, null, [])).ResultId; |
There was a problem hiding this comment.
Good fix. Note that the same load pattern exists in two other places in this file: line 130 (GS and tessellation array inputs) and line 224 (patch inputs). Both still load with stream.Info.Type instead of stream.InterfaceType.
No builtin with a type override reaches them today, but it would be good to fix them the same way you did for this one.
|
Both done in 7df901c: SV_COVERAGE added to the output check, the test asserts the SV_Coverage output and compiles the HLSL with fxc (ps_5_0), and the two other loads (GS/tessellation array inputs, patch inputs) use the interface type. |
HLSL reads and writes the coverage as a uint; SPIR-V wants a one-element array decorated SampleMask. The wrapper now loads a builtin input with the interface type before converting it, which is what the output side already did. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…y and patch inputs load with the interface type InterfaceProcessor only knew SV_Target* and SV_Depth as pixel shader outputs, so the write to SV_Coverage was dropped. The GS/tessellation array inputs and the patch inputs now load with the interface type before converting, like the other input paths. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
PR Details
A pixel shader declaring
stream uint coverage : SV_Coveragefailed to compile withSystem-value Semantic not implemented: SV_COVERAGE for stage Fragment as Input. In the engine,VoxelAttributeDirectionalCoverageShaderuses it, so any voxel volume with the anti-aliasing modifier hit it (#3374 after #3449).BuiltinProcessormapsSV_Coveragein the fragment stage, input and output, toBuiltIn.SampleMask, with the one-element array SPIR-V expects;ConvertInterfaceVariablealready handles the array/scalar conversion both ways.EntryPointWrapperGeneratorloads a builtin input with its interface type before converting it, which is what the output side already did. Until now no fragment input had a type override, so this path was never exercised.CoverageIsSampleMaskInFragmentStage, which validates the SPIR-V and checks the HLSL translation carriesSV_Coverage.Verified on Direct3D 11 with the MyGame2 project attached to #3374, together with #3449: it renders, shadows and voxel bounce included.
Related Issue
Part of #3374.
Types of changes
Checklist