Skip to content

Commit

Permalink
Allow customization of DXC executable (#17150)
Browse files Browse the repository at this point in the history
* Allow customization of DXC executable

Resolves #17148 (DXC should be customizable via Settings Registry)
Which enables working around this bug:
o3de/o3de-extras#625

Developers can now pick a custom version of the DXC executable
via the Settings Registry Key:
`/O3DE/Atom/DxcOverridePath`.

Also, introduced the macro `PRE_HLSL_2021`, along with
an explanation in the following file:
/o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md
TL;DR: If a game project uses a DXC executable that doesn't conform
to the HLSL 2021 standard, then they should globally define the macro
PRE_HLSL_2021  when compiling shaders.

The O3DE default version of DXC is 1.7.2308 which conforms to HLSL 2021.
The problem is that some shaders get compiled to versions of SPIRV
not supported by Adreno GPU drivers. This commit gives the option
to use a different DXC and avoid this issue.

Removed redundant LinearToSRGB conversion functions.

* Removed "real*" Linear<->SRGB functions
in favor of both "float*" and "half*" functions.

---------

Signed-off-by: galibzon <[email protected]>
  • Loading branch information
galibzon authored Dec 2, 2023
1 parent 5c46d85 commit ac2a6bb
Show file tree
Hide file tree
Showing 15 changed files with 166 additions and 57 deletions.
2 changes: 1 addition & 1 deletion Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ namespace AZ
AZStd::string overridePath;
if (setReg->Get(overridePath, AzslCompilerOverridePath))
{
AZ_TraceOnce("AzslCompiler", "AZSLc executable override specified, using %s", azslcPath.c_str());
AZ_TraceOnce("AzslCompiler", "AZSLc executable override specified, using %s", overridePath.c_str());
azslcPath = AZStd::move(overridePath);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,24 @@

#pragma once

real3 LinearSrgb_To_Srgb(real3 color)
float3 LinearSrgb_To_Srgb(float3 color)
{
// Copied from CryFx's ToAccurateSRGB()
return select((color.xyz < 0.0031308), 12.92 * color.xyz, 1.055 * pow(color.xyz, 1.0 / 2.4) - real3(0.055, 0.055, 0.055));
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
return (color.xyz < 0.0031308) ? 12.92 * color.xyz : 1.055 * pow(color.xyz, 1.0 / 2.4) - float3(0.055, 0.055, 0.055);
#else
return select((color.xyz < 0.0031308), 12.92 * color.xyz, 1.055 * pow(color.xyz, 1.0 / 2.4) - float3(0.055, 0.055, 0.055));
#endif
}

half3 LinearSrgb_To_Srgb(half3 color)
{
// Copied from CryFx's ToAccurateSRGB()
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
return (color.xyz < 0.0031308h) ? 12.92h * color.xyz : 1.055h * pow(color.xyz, 1.0h / 2.4h) - half3(0.055, 0.055, 0.055);
#else
return select((color.xyz < 0.0031308h), 12.92h * color.xyz, 1.055h * pow(color.xyz, 1.0h / 2.4h) - half3(0.055, 0.055, 0.055));
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,27 @@

#pragma once

real3 Srgb_To_LinearSrgb(real3 color)
float3 Srgb_To_LinearSrgb(float3 color)
{
// Copied from CryFx's ToAccurateLinear()
return select((color.xyz < 0.04045), color.xyz / 12.92h, pow((color.xyz + real3(0.055, 0.055, 0.055)) / real3(1.055, 1.055, 1.055), 2.4));
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
return (color.xyz < 0.04045) ? color.xyz / 12.92 : pow((color.xyz + float3(0.055, 0.055, 0.055)) / float3(1.055, 1.055, 1.055), 2.4);
#else
return select((color.xyz < 0.04045), color.xyz / 12.92, pow((color.xyz + float3(0.055, 0.055, 0.055)) / float3(1.055, 1.055, 1.055), 2.4));
#endif
}

// Some shaders don't use "real" variables. So on mobile devices they'd be penalized
// when calling Srgb_To_LinearSrgb because of the data conversion from "float" to "half" and viceversa.
// This is why this alternative version is available.
half3 Srgb_To_LinearSrgb(half3 color)
{
// Copied from CryFx's ToAccurateLinear()
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
return (color.xyz < 0.04045h) ? color.xyz / 12.92h : pow((color.xyz + half3(0.055, 0.055, 0.055)) / half3(1.055, 1.055, 1.055), 2.4);
#else
return select((color.xyz < 0.04045h), color.xyz / 12.92h, pow((color.xyz + half3(0.055, 0.055, 0.055)) / half3(1.055, 1.055, 1.055), 2.4));
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,12 @@ float3 OutputDeviceTransform(const float3 oces, OutputTransformParameters otPara
// LDR mode, clamp 0/1 and encode with given gamma value for the OETF
linearCV = clamp(linearCV, 0., 1.);

#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
outputCV = outputCV > 0.0f ? pow(linearCV, 1.0f / otParams.gamma) : 0.0f;
#else
outputCV = select(outputCV > 0.0f, pow(linearCV, 1.0f / otParams.gamma), 0.0f);
#endif
}

return outputCV;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,9 @@

#pragma once

/*
Linear to sRGB and back again
The following 4 functions are helpful for accurately converting from
Linear colorspace to sRGB colorspace and back again. With both floats and halfs.

More information, including the transformations used in this file, can be found here:
https://en.wikipedia.org/wiki/SRGB
*/

float3 LinearToSRGB(float3 col)
{
return select((col.xyz < 0.0031308), 12.92 * col.xyz, 1.055 * pow(abs(col.xyz), 1.0 / 2.4) - float3( 0.055, 0.055, 0.055 )) ;
}

float3 SRGBToLinear(float3 col)
{
return select((col.xyz < 0.04045), col.xyz / 12.92, pow (abs((col.xyz + float3(0.055, 0.055, 0.055)) / float3(1.055, 1.055, 1.055)), 2.4));
}

/* AZSL doesn't support function overload for the moment.
half3 LinearToSRGB(half3 col)
{
return select((col < 0.0031308h), 12.92h * col, 1.055h * pow(abs(col), 1.0h / 2.4h) - half3(0.055h, 0.055h, 0.055h));
}

half3 SRGBToLinear(half3 col)
{
return select((col.xyz < 0.04045h), col.xyz / 12.92h, pow (abs((col.xyz + half3(0.055h, 0.055h, 0.055h)) / half3(1.055h, 1.055h, 1.055h)), 2.4h));
}
*/

// Lifted from Legacy LY Renderer
// Input color must be in linear space
float GetLuminance( float3 color )
{
return dot( color, float3( 0.2126f, 0.7152f, 0.0722f ) );
}

75 changes: 75 additions & 0 deletions Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# About the macro PRE_HLSL_2021

The macro PRE_HLSL_2021 was introduced since this bug: https://github.com/o3de/o3de-extras/issues/625.

The workaround to the bug mentioned above was to use an older version of DXC 1.6.2112, instead of DXC 1.7.2308. DXC 1.7.2308 conforms to HLSL 2021 which doesn't support the ternary conditional operator (`(cond) ? true_statement : false_statement`) for vectors. HLSL 2021 only supports ternary conditional operator for scalars. Starting HLSL 2021, to achieve an equivalent behavior for ternary conditional operator for vectors, the `select()` intrinsic was introduced. See more details here:
https://devblogs.microsoft.com/directx/announcing-hlsl-2021/

This is why you'll find now code like this in some *.azsli or *.azsl files:
```hlsl
#ifdef PRE_HLSL_2021
return (color.xyz < 0.04045) ? color.xyz / 12.92h : pow((color.xyz + real3(0.055, 0.055, 0.055)) / real3(1.055, 1.055, 1.055), 2.4);
#else
return select((color.xyz < 0.04045), color.xyz / 12.92h, pow((color.xyz + real3(0.055, 0.055, 0.055)) / real3(1.055, 1.055, 1.055), 2.4));
#endif
```

By default the macro `PRE_HLSL_2021` is not defined, and the shaders will use the `and`, `or` and `select` intrinsics.

## When should the developer define the PRE_HLSL_2021 macro?
Defining the PRE_HLSL_2021 is required if the developer customizes the DXC executable to version pre-HLSL 2021 like DXC 1.6.2112.

The DXC executable can be customized via the Settings Registry Key:
`/O3DE/Atom/DxcOverridePath`.
Here is a customization example in a *.setreg file:
```json
{
"O3DE": {
"Atom": {
"DxcOverridePath": "C:\\Users\\galib\\.o3de\\3rdParty\\packages\\DirectXShaderCompilerDxc-1.6.2112-o3de-rev1-windows\\DirectXShaderCompilerDxc\\bin\\Release\\dxc.exe"
}
}
}
```

## How to globally define the PRE_HLSL_2021 macro when compiling all shaders?
Customization of command line arguments for shader compilation tools is described here:
https://github.com/o3de/o3de/wiki/%5BAtom%5D-Developer-Guide:-Shader-Build-Arguments-Customization

In particular if you don't want to mess with engine files, in your game project:
1- Define an alternate location for `shader_build_options.json` using the registry key: `/O3DE/Atom/Shaders/Build/ConfigPath`.
2- Your new `shader_build_options.json` should be a copy of engine original located at `C:\GIT\o3de\Gems\Atom\Asset\Shader\Config\shader_build_options.json` with the addition of:
```json
"Definitions": [ "PRE_HLSL_2021" ], // Needed when using DXC 1.6.2112
```
Complete example of customized shader_build_options.json:
```json
{
"Definitions": [ "PRE_HLSL_2021" ], // Needed when using DXC 1.6.2112
"AddBuildArguments": {
"debug": false,
"preprocessor": ["-C" // Preserve comments
, "-+" // C++ mode
],
"azslc": ["--full" // Always generate the *.json files with SRG and reflection info.
, "--Zpr" // Row major matrices.
, "--W1" // Warning Level 1
, "--strip-unused-srgs" // Remove unreferenced SRGs.
, "--root-const=128"
],
"dxc": ["-Zpr" // Row major matrices.
],
// The following apply for all Metal based platforms.
"spirv-cross": ["--msl"
, "--msl-version"
, "20100"
, "--msl-invariant-float-math"
, "--msl-argument-buffers"
, "--msl-decoration-binding"
, "--msl-texture-buffer-native"
]
}
}
```

Finally, if not done already you'd have to restart the AssetProcessor so it picks the changes from the Settings Registry.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ float4 SampleAverageValidColors(float2 tex0, float2 tex1, float2 tex2, float2 te
float4 dofFactors = float4(color0.a, color1.a, color2.a, color3.a);
float4 blends = ConvertBlendsFloat4(dofFactors , PassSrg::m_blendFactor);
// 0.25 is 1 / pixel number.
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
float4 masks = blends > 0.0f ? 0.25f : 0.0f;
#else
float4 masks = select(blends > 0.0f, 0.25f, 0.0f);
#endif
float weight = dot(masks, (float4)1.0f);

float4 color = (float4)0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ bool TraceRayScreenSpace(float3 rayStartVS, float3 rayDirectionVS, uint2 dimensi
float3 Q1 = rayEndVS * k1;

// if the line is degenerate, make it cover at least one pixel
#ifdef PRE_HLSL_2021
// See /o3de/Gems/Atom/Feature/Common/Assets/ShaderLib/PRE_HLSL_2021.md for details.
P1 += DistanceSqr(P0, P1) < EPSILON ? float2(0.01f, 0.01f) : 0.0f;
#else
P1 += select(DistanceSqr(P0, P1) < EPSILON, float2(0.01f, 0.01f), 0.0f);
#endif

// store all of the start variables in a single float4
float4 PQK = float4(P0, Q0.z, k0);
Expand Down
6 changes: 6 additions & 0 deletions Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ namespace AZ::RHI
return result;
}

//! Returns the path to the DirectXShaderCompiler (also known as DXC) executable.
//! If the user has not provided a customized executable path
//! in the Settings Registry, then @defaultPathToDxc is returned.
//! @param defaultPathToDxc Default path for the current platform. Can not be nullptr.
AZStd::string GetDirectXShaderCompilerPath(const char* defaultPathToDxc);

//! Runs a shader compiler executable with specific parameters.
//! Returns true it compiled the shader without errors.
//! Returns false otherwise and with compilation errors messages.
Expand Down
20 changes: 20 additions & 0 deletions Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <AzCore/Math/Sha1.h>
#include <AzCore/Platform.h>
#include <AzCore/std/time.h>
#include <AzCore/Settings/SettingsRegistry.h>

#include <AzFramework/StringFunc/StringFunc.h>

Expand Down Expand Up @@ -499,6 +500,25 @@ namespace AZ::RHI
return outputFile;
}

AZStd::string GetDirectXShaderCompilerPath(const char* defaultPathToDxc)
{
// To quickly test changes to the DXC compiler or as a workaround to the default.
static constexpr char DxcOverridePathKey[] = "/O3DE/Atom/DxcOverridePath";

if (auto setReg = AZ::Interface<SettingsRegistryInterface>::Get())
{
AZStd::string overridePath;
if (setReg->Get(overridePath, DxcOverridePathKey))
{
AZ_TraceOnce("CustomDxc", "DXC executable override specified, using %s.\n", overridePath.c_str());
return overridePath;
}
}

AZ_Assert(defaultPathToDxc != nullptr, "Invalid default path to DXC.");
return AZStd::string(defaultPathToDxc);
}

namespace CommandLineArgumentUtils
{
AZStd::vector<AZStd::string> GetListOfArgumentNames(AZStd::string_view commandLineString)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <AzCore/IO/SystemFile.h>
#include <AzFramework/StringFunc/StringFunc.h>


namespace AZ
{
namespace DX12
Expand Down Expand Up @@ -202,7 +201,7 @@ namespace AZ
ByProducts& byProducts) const
{
// Shader compiler executable
static const char* dxcRelativePath = "Builders/DirectXShaderCompiler/dxc.exe";
const auto dxcRelativePath = RHI::GetDirectXShaderCompilerPath("Builders/DirectXShaderCompiler/dxc.exe");

// NOTE:
// Running DX12 on PC with DXIL shaders requires modern GPUs and at least Windows 10 Build 1803 or later for Shader Model 6.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <Atom/RHI.Reflect/Metal/ShaderStageFunction.h>
#include <AzFramework/StringFunc/StringFunc.h>

#include <ShaderBuilder_Traits_Platform.h>

namespace AZ
{
namespace Metal
Expand Down Expand Up @@ -230,7 +232,7 @@ namespace AZ
ByProducts& byProducts) const
{
// Shader compiler executable
static const char* dxcRelativePath = "Builders/DirectXShaderCompiler/bin/dxc";
const auto dxcRelativePath = RHI::GetDirectXShaderCompilerPath("Builders/DirectXShaderCompiler/bin/dxc");

// Output file
AZStd::string shaderMSLOutputFile = RHI::BuildFileNameWithExtension(shaderSourceFile, tempFolder, "metal");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ namespace AZ
ByProducts& byProducts) const
{
// Shader compiler executable
static const char* dxcRelativePath = AZ_TRAIT_ATOM_SHADERBUILDER_DXC;
const auto dxcRelativePath = RHI::GetDirectXShaderCompilerPath(AZ_TRAIT_ATOM_SHADERBUILDER_DXC);

// -Fo "Output file"
AZStd::string shaderOutputFile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

#include <Atom/Features/SrgSemantics.azsli>
#include <Atom/Features/ColorManagement/TransformColor.azsli>

struct VertexInput
{
Expand Down Expand Up @@ -40,22 +41,11 @@ ShaderResourceGroup ObjectSrg : SRG_PerObject
column_major float4x4 m_projectionMatrix;
}

float3 SRGBToLinear( float3 x )
{
return select(x < 0.04045f, x / 12.92f, pow( (abs(x) + 0.055f) / 1.055f, 2.4f ));

// NOTE: abs on x quiets FXC warning "X3571: pow(f, e) will not work for negative f,
// use abs(f) or conditionally handle negative values if you expect them".
// Ternary operator ?: in shaders can handle vectors in the condition, but it doesn't
// count as 'conditionally handle' since branching is not involved. When we start using
// DXC instead of FXC we can remove abs since DXC handles it correctly with no warnings.
}

VertexOutput MainVS(in VertexInput input)
{
VertexOutput output;
output.Position = mul(float4(input.Position, 0.0f, 1.0f), ObjectSrg::m_projectionMatrix);
output.Color = float4(SRGBToLinear(input.Color.rgb), input.Color.a);
output.Color = float4(Srgb_To_LinearSrgb(input.Color.rgb), input.Color.a);
output.UV = input.UV;
return output;
}
Expand Down
7 changes: 3 additions & 4 deletions Gems/LyShine/Assets/LyShine/Shaders/LyShineUI.azsl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#include <Atom/Features/SrgSemantics.azsli>

#include <Atom/Features/PostProcessing/PostProcessUtil.azsli>
#include <Atom/Features/ColorManagement/TransformColor.azsli>

// If true pixels with an alpha value of less than 0.5 are clipped
option bool o_alphaTest;
Expand Down Expand Up @@ -114,7 +114,7 @@ PSOutput MainPS(VSOutput IN)
// Should use srgb anytime after tonemapping
if (o_srgbWrite)
{
resColor.xyz = LinearToSRGB(resColor.xyz);
resColor.xyz = LinearSrgb_To_Srgb(resColor.xyz);
}

// If the o_modulate option is not None it means that the verts have two texture indicies. The second texture is used to
Expand Down Expand Up @@ -145,5 +145,4 @@ PSOutput MainPS(VSOutput IN)
OUT.m_color = resColor;

return OUT;
};

};

0 comments on commit ac2a6bb

Please sign in to comment.