o3de / o3de-azslc

Amazon Shader Language (AZSL) Compiler
Other
23 stars 14 forks source link

[Info][Patch] Current O3DE is incompatible with current azslc #54

Open martinwinter-huawei opened 2 years ago

martinwinter-huawei commented 2 years ago

The --use-spaces change is a breaking change within current O3DE, as it is no longer a valid keyword. To be able to use the current version of azslc with the current state of development of o3de, small changes are necessary. The following patch (for o3de) introduces these fixes

diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp
index 38cd7af751..7663c90558 100644
--- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp
+++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp
@@ -448,8 +448,8 @@ namespace AZ
                 // since the register Id of the resource will not change even if the pipeline layout changes.
                 // We can pass in a default ShaderCompilerArguments because all we care about is whether the shaderPlatformInterface
                 // appends the "--use-spaces" flag.
-                const auto& azslcArguments = buildArgsManager.GetCurrentArguments().m_azslcArguments;
-                const bool platformUsesRegisterSpaces = RHI::ShaderBuildArguments::HasArgument(azslcArguments, "--use-spaces");
+                // const auto& azslcArguments = buildArgsManager.GetCurrentArguments().m_azslcArguments;
+                const bool platformUsesRegisterSpaces = true; // RHI::ShaderBuildArguments::HasArgument(azslcArguments, "--use-spaces");

                 uint32_t supervariantIndex = 0;
                 for (const auto& supervariantInfo : supervariantList)
diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp
index 41ef6dc7af..0d0b520f26 100644
--- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp
+++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp
@@ -866,9 +866,10 @@ namespace AZ
                     RHI::Ptr<RHI::PipelineLayoutDescriptor> pipelineLayoutDescriptor;
                     if (shaderPlatformInterface->VariantCompilationRequiresSrgLayoutData())
                     {
-                        const auto& azslcArguments = buildArgsManager.GetCurrentArguments().m_azslcArguments;
-                        const bool platformUsesRegisterSpaces = RHI::ShaderBuildArguments::HasArgument(azslcArguments, "--use-spaces");
-                    
+                        // const auto& azslcArguments = buildArgsManager.GetCurrentArguments().m_azslcArguments;
+                        const bool platformUsesRegisterSpaces =
+                            true; //= RHI::ShaderBuildArguments::HasArgument(azslcArguments, "--use-spaces");
+
                         RPI::ShaderResourceGroupLayoutList srgLayoutList;
                         RootConstantData rootConstantData;
                         if (!LoadSrgLayoutListFromShaderAssetBuilder(
diff --git a/Gems/Atom/Asset/Shader/Config/shader_build_options.json b/Gems/Atom/Asset/Shader/Config/shader_build_options.json
index df69bd17fd..f14ae106ee 100644
--- a/Gems/Atom/Asset/Shader/Config/shader_build_options.json
+++ b/Gems/Atom/Asset/Shader/Config/shader_build_options.json
@@ -5,7 +5,8 @@
               , "-+" // C++ mode
         ],
         "azslc": ["--full" // Always generate the *.json files with SRG and reflection info.
-                , "--use-spaces"
+               // ,
+               // "--use-spaces"
                 , "--Zpr" // Row major matrices.
                 , "--W1" // Warning Level 1
                 , "--strip-unused-srgs" // Remove unreferenced SRGs.
santorac commented 2 years ago

I made similar (but more comprehensive) changes here, it's currently sitting on a feature branch for bindless support. https://github.com/aws-lumberyard-dev/o3de/commit/3ffbe0c46dad991458eaae081f08d3f6a76bd438

The bindless support is likely to be done soon, and a new AZSLc package released by @siliconvoodoo , and that will clear everything up.

Sorry about the mixup.