Skip to content

Commit

Permalink
Merge pull request #805 from Sergio0694/dev/workaround-fxc-float-bug
Browse files Browse the repository at this point in the history
Always emit 'asfloat' for float literals in D2D shaders
  • Loading branch information
Sergio0694 authored Jun 5, 2024
2 parents deab9f5 + b4c5b4d commit 908d691
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public sealed override SyntaxNode VisitDefaultExpression(DefaultExpressionSyntax
}

/// <inheritdoc/>
public sealed override SyntaxNode? VisitLiteralExpression(LiteralExpressionSyntax node)
public sealed unsafe override SyntaxNode? VisitLiteralExpression(LiteralExpressionSyntax node)
{
CancellationToken.ThrowIfCancellationRequested();

Expand All @@ -219,6 +219,7 @@ public sealed override SyntaxNode VisitDefaultExpression(DefaultExpressionSyntax
// in HLSL, we can remove the suffix entirely. As for 64 bit values, we simply use the 'L' suffix.
if (type.SpecialType == SpecialType.System_Single)
{
#if D3D12_SOURCE_GENERATOR
string literal = updatedNode.Token.ValueText;

// If the numeric literal is neither a decimal nor an exponential, add the ".0" suffix
Expand All @@ -228,6 +229,25 @@ public sealed override SyntaxNode VisitDefaultExpression(DefaultExpressionSyntax
}

return updatedNode.WithToken(Literal(literal, 0f));
#else
// When compiling for D2D, we need to emit 'asfloat' expressions to work around an FXC
// bug that can cause it to sometimes emit incorrect values for float literals. For
// additional context, see: https://github.com/Sergio0694/ComputeSharp/issues/780.
//
// This code rewrites expressions as follows:
// C#: 3.14f
// HLSL: asfloat(1078523331)
float literalValue = (float)operation.ConstantValue.Value!;
uint literalValueAsUInt = *(uint*)&literalValue;

return
InvocationExpression(IdentifierName("asfloat"))
.AddArgumentListArguments(
Argument(
LiteralExpression(
SyntaxKind.NumericLiteralExpression,
Literal(literalValueAsUInt))));
#endif
}
else if (type.SpecialType == SpecialType.System_Double)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ partial struct MyShader : global::ComputeSharp.D2D1.Descriptors.ID2D1PixelShader
{
int2 xy = (int2)D2DGetScenePosition().xy;
float2 uv = xy / (float2)dispatchSize;
float3 col = 0.5 + (0.5 * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, 1.0);
float3 col = asfloat(1056964608U) + (asfloat(1056964608U) * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, asfloat(1065353216U));
}
""";
Expand Down Expand Up @@ -482,8 +482,8 @@ partial struct MyShader : global::ComputeSharp.D2D1.Descriptors.ID2D1PixelShader
{
int2 xy = (int2)D2DGetScenePosition().xy;
float2 uv = xy / (float2)dispatchSize;
float3 col = 0.5 + (0.5 * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, 1.0);
float3 col = asfloat(1056964608U) + (asfloat(1056964608U) * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, asfloat(1065353216U));
}
""";
Expand Down Expand Up @@ -830,8 +830,8 @@ partial struct MyShader : global::ComputeSharp.D2D1.Descriptors.ID2D1PixelShader
{
int2 xy = (int2)D2DGetScenePosition().xy;
float2 uv = xy / (float2)dispatchSize;
float3 col = 0.5 + (0.5 * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, 1.0);
float3 col = asfloat(1056964608U) + (asfloat(1056964608U) * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, asfloat(1065353216U));
}
""";
Expand Down Expand Up @@ -1124,8 +1124,8 @@ partial struct MyShader : global::ComputeSharp.D2D1.Descriptors.ID2D1PixelShader
{
int2 xy = (int2)D2DGetScenePosition().xy;
float2 uv = xy / (float2)dispatchSize;
float3 col = 0.5 + (0.5 * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, 1.0);
float3 col = asfloat(1056964608U) + (asfloat(1056964608U) * cos(time + float3(uv, uv.x) + float3(0, 2, 4)));
return float4(col, asfloat(1065353216U));
}
""";
Expand Down
Binary file added tests/ComputeSharp.D2D1.Tests/Assets/Green32x32.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 4 additions & 1 deletion tests/ComputeSharp.D2D1.Tests/ComputeSharp.D2D1.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,15 @@
<None Update="Assets\ContouredLayers.png">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Include="..\ComputeSharp.Tests\Assets\TerracedHills.png" Link="Assets\TerracedHills.png">
<None Include="Assets\TerracedHills.png">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Assets\RadialFadeOut.png">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Assets\Green32x32.png">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Include="..\..\samples\ComputeSharp.SwapChain\Textures\RustyMetal.png" Link="Assets\Textures\RustyMetal.png" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void GetShaderInfo()
float4 input1 = D2DGetInput(1);
float4 input2 = D2DSampleInputAtPosition(2, xy + offset);
float value3 = __reserved__buffer[0];
float4 value4 = __reserved__texture.Sample(__sampler____reserved__texture, float2(0, 0.5));
float4 value4 = __reserved__texture.Sample(__sampler____reserved__texture, float2(0, asfloat(1056964608U)));
return input0 + input1 + input2 + value3 + value4;
}
""", shaderInfo.HlslSource);
Expand Down
33 changes: 31 additions & 2 deletions tests/ComputeSharp.D2D1.Tests/ShaderRewriterTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using ComputeSharp.D2D1.Interop;
using ComputeSharp.D2D1.Tests.Helpers;
using Microsoft.VisualStudio.TestTools.UnitTesting;

#pragma warning disable CA1822
Expand Down Expand Up @@ -82,7 +83,7 @@ public void KnownNamedIntrinsic_ConditionalSelect()
D2D_PS_ENTRY(Execute)
{
bool4 mask4 = bool4(true, false, true, true);
float4 float4_1 = float4(1, 2, 3.14, 4);
float4 float4_1 = float4(1, 2, asfloat(1078523331U), 4);
float4 float4_2 = float4(5, 6, 7, 8);
float4 float4_r = (mask4 ? float4_1 : float4_2);
bool1x3 mask1x3 = bool1x3((bool)true, (bool)true, (bool)false);
Expand All @@ -93,7 +94,7 @@ public void KnownNamedIntrinsic_ConditionalSelect()
uint2x4 uint2x4_1 = uint2x4((uint)1, (uint)2, (uint)3, (uint)4, (uint)5, (uint)6, (uint)7, (uint)8);
uint2x4 uint2x4_2 = uint2x4((uint)111, (uint)222, (uint)333, (uint)444, (uint)555, (uint)666, (uint)777, (uint)888);
uint2x4 uint2x4_r = (mask2x4 ? uint2x4_1 : uint2x4_2);
float2x2 f2x2_r = mul((float2x4)uint2x4_r, float4x2((float2)float4_r.xy, (float2)float4_r.zw, (float2)float2(int1x3_r._m00, int1x3_r._m01), (float2)float2(int1x3_r._m02, 1.0)));
float2x2 f2x2_r = mul((float2x4)uint2x4_r, float4x2((float2)float4_r.xy, (float2)float4_r.zw, (float2)float2(int1x3_r._m00, int1x3_r._m01), (float2)float2(int1x3_r._m02, asfloat(1065353216U))));
return float4(f2x2_r._m00, f2x2_r._m01, f2x2_r._m10, f2x2_r._m11);
}
""", shaderInfo.HlslSource);
Expand Down Expand Up @@ -179,4 +180,32 @@ public float4 Execute()
mask2x3_r_or.M11 ? 1 : 0);
}
}

// See https://github.com/Sergio0694/ComputeSharp/issues/780
[TestMethod]
public void ProblematicFloatLiteralValue_IsRewrittenCorrectly()
{
D2D1TestRunner.RunAndCompareShader(
new ProblematicFloatLiteralValueShader(131072.65f),
32,
32,
"Green32x32.png");
}

[D2DInputCount(0)]
[D2DShaderProfile(D2D1ShaderProfile.PixelShader50)]
[D2DOutputBuffer(D2D1BufferPrecision.Float32)]
[D2DGeneratedPixelShaderDescriptor]
internal readonly partial struct ProblematicFloatLiteralValueShader(float x) : ID2D1PixelShader
{
public float4 Execute()
{
if (x != 131072.65f)
{
return 0;
}

return new float4(0.0f, 1.0f, 0.0f, 1.0f);
}
}
}
2 changes: 1 addition & 1 deletion tests/ComputeSharp.D2D1.Tests/ShadersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public unsafe void ContouredLayers()
[TestMethod]
public void TerracedHills()
{
RunTest<TerracedHills>(0.000026f);
RunTest<TerracedHills>(0.000027f);
}

private static void RunTest<T>(float threshold = 0.00001f)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<Link>Assets\Shaders\ProteanClouds.png</Link>
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="..\ComputeSharp.Tests\Assets\TerracedHills.png">
<Content Include="..\ComputeSharp.D2D1.Tests\Assets\TerracedHills.png">
<Link>Assets\Shaders\TerracedHills.png</Link>
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
Expand Down
2 changes: 1 addition & 1 deletion tests/ComputeSharp.D2D1.WinUI.Tests/Tests/ShadersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public async Task ProteanClouds(WrapperType wrapperType)
[DataRow(WrapperType.CanvasEffect)]
public async Task TerracedHills(WrapperType wrapperType)
{
await RunTestAsync<TerracedHills>(wrapperType, 0.000026f);
await RunTestAsync<TerracedHills>(wrapperType, 0.000027f);
}

private static async Task RunTestAsync<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>(WrapperType wrapperType, float threshold = 0.00001f)
Expand Down

0 comments on commit 908d691

Please sign in to comment.