From 4cac87558901899265faec9f1bb79e7b9d42c171 Mon Sep 17 00:00:00 2001 From: galibzon <66021303+galibzon@users.noreply.github.com> Date: Tue, 17 Aug 2021 08:21:39 -0500 Subject: [PATCH] [ATOM-15058] Remove Automatic Entry Point Detection (#3150) .shader files must declare at least one entry function. Signed-off-by: garrieta --- .../AzslShaderBuilderSystemComponent.cpp | 2 +- .../Code/Source/Editor/ShaderAssetBuilder.cpp | 25 +++--- .../Source/Editor/ShaderBuilderUtility.cpp | 85 ------------------- .../Code/Source/Editor/ShaderBuilderUtility.h | 8 -- .../Editor/ShaderVariantAssetBuilder.cpp | 15 ++-- .../Materials/Special/ShadowCatcher.shader | 15 ++++ .../Assets/Shaders/Depth/DepthPass.shader | 11 +++ .../Depth/DepthPassTransparentMax.shader | 11 +++ .../Depth/DepthPassTransparentMin.shader | 11 +++ 9 files changed, 65 insertions(+), 118 deletions(-) diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslShaderBuilderSystemComponent.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslShaderBuilderSystemComponent.cpp index 29e6fb7a6f..16cebef6ac 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslShaderBuilderSystemComponent.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslShaderBuilderSystemComponent.cpp @@ -81,7 +81,7 @@ namespace AZ // Register Shader Asset Builder AssetBuilderSDK::AssetBuilderDesc shaderAssetBuilderDescriptor; shaderAssetBuilderDescriptor.m_name = "Shader Asset Builder"; - shaderAssetBuilderDescriptor.m_version = 102; // ATOM-15472 + shaderAssetBuilderDescriptor.m_version = 103; // ATOM-15058 // .shader file changes trigger rebuilds shaderAssetBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern( AZStd::string::format("*.%s", RPI::ShaderSourceData::Extension), AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); shaderAssetBuilderDescriptor.m_busId = azrtti_typeid(); diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp index 606502fb22..2332f4522b 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp @@ -226,11 +226,9 @@ namespace AZ if (!hasRasterProgram && !hasComputeProgram && !hasRayTracingProgram) { - AZStd::string entryPointNames = ShaderBuilderUtility::GetAcceptableDefaultEntryPointNames(azslData); return AZ::Failure( - AZStd::string::format( "Shader asset descriptor has a program variant that does not define any entry points. Either declare entry " - "points in the .shader file, or use one of the available default names (not case-sensitive): [%s]", - entryPointNames.c_str())); + AZStd::string( "Shader asset descriptor has a program variant that does not define any entry points." + " Please declare entry points in the .shader file.")); } return AZ::Success(attributeMaps); @@ -478,21 +476,18 @@ namespace AZ } } - // Discover entry points & type of programs. - MapOfStringToStageType shaderEntryPoints; if (shaderSourceData.m_programSettings.m_entryPoints.empty()) { - AZ_TracePrintf( - ShaderAssetBuilderName, - "ProgramSettings do not specify entry points, will use GetDefaultEntryPointsFromShader()\n"); - ShaderBuilderUtility::GetDefaultEntryPointsFromFunctionDataList(azslData.m_functions, shaderEntryPoints); + AZ_Error( ShaderAssetBuilderName, false, "ProgramSettings must specify entry points."); + response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Failed; + return; } - else + + // Discover entry points & type of programs. + MapOfStringToStageType shaderEntryPoints; + for (const auto& entryPoint : shaderSourceData.m_programSettings.m_entryPoints) { - for (const auto& entryPoint : shaderSourceData.m_programSettings.m_entryPoints) - { - shaderEntryPoints[entryPoint.m_name] = entryPoint.m_type; - } + shaderEntryPoints[entryPoint.m_name] = entryPoint.m_type; } bool hasRasterProgram = false; diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp index d7c3de48c0..0018f2ead8 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp @@ -809,91 +809,6 @@ namespace AZ return success; } - - //! Returns a list of acceptable default entry point names - static void GetAcceptableDefaultEntryPoints( - const AZStd::vector& azslFunctionDataList, - AZStd::unordered_map& defaultEntryPoints) - { - for (const auto& func : azslFunctionDataList) - { - if (!func.m_hasShaderStageVaryings) - { - // Not declaring any semantics for a shader entry is valid, but unusual. - // A shader entry with no semantics must be explicitly listed and won't be selected by default. - continue; - } - - if (func.m_name.starts_with("VS") || func.m_name.ends_with("VS")) - { - defaultEntryPoints[func.m_name] = RPI::ShaderStageType::Vertex; - AZ_TracePrintf( - ShaderBuilderUtilityName, "Assuming \"%s\" is a valid Vertex shader entry point.\n", func.m_name.c_str()); - } - else if (func.m_name.starts_with("PS") || func.m_name.ends_with("PS")) - { - defaultEntryPoints[func.m_name] = RPI::ShaderStageType::Fragment; - AZ_TracePrintf( - ShaderBuilderUtilityName, "Assuming \"%s\" is a valid Fragment shader entry point.\n", - func.m_name.c_str()); - } - else if (func.m_name.starts_with("CS") || func.m_name.ends_with("CS")) - { - defaultEntryPoints[func.m_name] = RPI::ShaderStageType::Compute; - AZ_TracePrintf( - ShaderBuilderUtilityName, "Assuming \"%s\" is a valid Compute shader entry point.\n", func.m_name.c_str()); - } - } - } - - - // DEPRECATED [ATOM-15472 - //! Returns a list of acceptable default entry point names - //! This function - static void GetAcceptableDefaultEntryPoints( - const AzslData& azslData, AZStd::unordered_map& defaultEntryPoints) - { - return GetAcceptableDefaultEntryPoints(azslData.m_functions, defaultEntryPoints); - } - - - void GetDefaultEntryPointsFromFunctionDataList( - const AZStd::vector azslFunctionDataList, - AZStd::unordered_map& shaderEntryPoints) - { - AZStd::unordered_map defaultEntryPoints; - GetAcceptableDefaultEntryPoints(azslFunctionDataList, defaultEntryPoints); - - for (const auto& functionData : azslFunctionDataList) - { - for (const auto& defaultEntryPoint : defaultEntryPoints) - { - // Equal defaults to case insensitive compares... - if (AzFramework::StringFunc::Equal(defaultEntryPoint.first.c_str(), functionData.m_name.c_str())) - { - shaderEntryPoints[defaultEntryPoint.first] = defaultEntryPoint.second; - break; // stop looping default entry points and go to the next shader function - } - } - } - } - - AZStd::string GetAcceptableDefaultEntryPointNames(const AzslData& azslData) - { - AZStd::unordered_map defaultEntryPointList; - GetAcceptableDefaultEntryPoints(azslData, defaultEntryPointList); - - AZStd::vector defaultEntryPointNamesList; - for (const auto& shaderEntryPoint : defaultEntryPointList) - { - defaultEntryPointNamesList.push_back(shaderEntryPoint.first); - } - AZStd::string shaderEntryPoints; - AzFramework::StringFunc::Join( - shaderEntryPoints, defaultEntryPointNamesList.begin(), defaultEntryPointNamesList.end(), ", "); - return AZStd::move(shaderEntryPoints); - } - } // namespace ShaderBuilderUtility } // namespace ShaderBuilder } // AZ diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.h b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.h index 9310bf2e2f..c000ba9df6 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.h +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.h @@ -94,10 +94,6 @@ namespace AZ RPI::ShaderOutputContract& shaderOutputContract, size_t& colorAttachmentCount); - //! Returns a list of acceptable default entry point names as a single string for debug messages. - AZStd::string GetAcceptableDefaultEntryPointNames(const AzslData& shaderData); - - //! Create a file from a string's content. //! That file will be named filename.api.azslin //! This is meant to be used at this stage: @@ -138,10 +134,6 @@ namespace AZ AZStd::vector GetSupervariantListFromShaderSourceData( const RPI::ShaderSourceData& shaderSourceData); - void GetDefaultEntryPointsFromFunctionDataList( - const AZStd::vector azslFunctionDataList, - AZStd::unordered_map& shaderEntryPoints); - void LogProfilingData(const char* builderName, AZStd::string_view shaderPath); //! Returns the asset path of a product artifact produced by ShaderAssetBuilder. diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp index 1da4623774..59660440e4 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp @@ -843,17 +843,14 @@ namespace AZ MapOfStringToStageType shaderEntryPoints; if (shaderSourceDescriptor.m_programSettings.m_entryPoints.empty()) { - AZ_TracePrintf( - ShaderVariantAssetBuilderName, - "ProgramSettings do not specify entry points, will use GetDefaultEntryPointsFromShader()\n"); - ShaderBuilderUtility::GetDefaultEntryPointsFromFunctionDataList(azslFunctions, shaderEntryPoints); + AZ_Error(ShaderVariantAssetBuilderName, false, "ProgramSettings must specify entry points."); + response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Failed; + return; } - else + + for (const auto& entryPoint : shaderSourceDescriptor.m_programSettings.m_entryPoints) { - for (const auto& entryPoint : shaderSourceDescriptor.m_programSettings.m_entryPoints) - { - shaderEntryPoints[entryPoint.m_name] = entryPoint.m_type; - } + shaderEntryPoints[entryPoint.m_name] = entryPoint.m_type; } // 3- hlslCode diff --git a/Gems/Atom/Feature/Common/Assets/Materials/Special/ShadowCatcher.shader b/Gems/Atom/Feature/Common/Assets/Materials/Special/ShadowCatcher.shader index f4784440b1..7df4169498 100644 --- a/Gems/Atom/Feature/Common/Assets/Materials/Special/ShadowCatcher.shader +++ b/Gems/Atom/Feature/Common/Assets/Materials/Special/ShadowCatcher.shader @@ -18,5 +18,20 @@ "BlendOp": "Add" }, + "ProgramSettings" : + { + "EntryPoints": + [ + { + "name": "ShadowCatcherVS", + "type" : "Vertex" + }, + { + "name": "ShadowCatcherPS", + "type" : "Fragment" + } + ] + }, + "DrawList": "transparent" } diff --git a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPass.shader b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPass.shader index fe76eb06cb..463db025e7 100644 --- a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPass.shader +++ b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPass.shader @@ -9,5 +9,16 @@ "DisableOptimizations" : false }, + "ProgramSettings" : + { + "EntryPoints": + [ + { + "name": "DepthPassVS", + "type" : "Vertex" + } + ] + }, + "DrawList" : "depth" } diff --git a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMax.shader b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMax.shader index a56959e357..5bfdc8bcc1 100644 --- a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMax.shader +++ b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMax.shader @@ -13,5 +13,16 @@ "CompilerHints" : { }, + "ProgramSettings" : + { + "EntryPoints": + [ + { + "name": "DepthPassVS", + "type" : "Vertex" + } + ] + }, + "DrawList" : "depthTransparentMax" } diff --git a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMin.shader b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMin.shader index 709e467479..5cd8ea7c33 100644 --- a/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMin.shader +++ b/Gems/Atom/Feature/Common/Assets/Shaders/Depth/DepthPassTransparentMin.shader @@ -11,5 +11,16 @@ "DisableOptimizations" : false }, + "ProgramSettings" : + { + "EntryPoints": + [ + { + "name": "DepthPassVS", + "type" : "Vertex" + } + ] + }, + "DrawList" : "depthTransparentMin" }