From a88ff24e94bba0efaa44efe6263bd1b698e58849 Mon Sep 17 00:00:00 2001 From: assiduous Date: Mon, 14 Sep 2020 17:45:59 -0700 Subject: Completed shader compilation refactoring (fixed https://github.com/DiligentGraphics/DiligentCore/issues/160) --- .../GraphicsEngineD3DBase/src/ShaderD3DBase.cpp | 57 ++++++++-------------- .../GraphicsEngineOpenGL/include/ShaderGLImpl.hpp | 2 +- Graphics/GraphicsEngineOpenGL/src/ShaderGLImpl.cpp | 18 +++---- Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp | 25 ++++++---- Graphics/ShaderTools/include/HLSLUtils.hpp | 53 ++++++++++++++++++++ Graphics/ShaderTools/src/DXCompiler.cpp | 42 ++++------------ 6 files changed, 107 insertions(+), 90 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GraphicsEngineD3DBase/src/ShaderD3DBase.cpp b/Graphics/GraphicsEngineD3DBase/src/ShaderD3DBase.cpp index 39684476..4ac72599 100644 --- a/Graphics/GraphicsEngineD3DBase/src/ShaderD3DBase.cpp +++ b/Graphics/GraphicsEngineD3DBase/src/ShaderD3DBase.cpp @@ -63,7 +63,7 @@ public: return E_FAIL; } - RefCntAutoPtr pFileData(MakeNewRCObj()(0)); + RefCntAutoPtr pFileData(MakeNewRCObj{}(0)); pSourceStream->ReadBlob(pFileData); *ppData = pFileData->GetDataPtr(); *pBytes = static_cast(pFileData->GetSize()); @@ -115,17 +115,27 @@ ShaderD3DBase::ShaderD3DBase(const ShaderCreateInfo& ShaderCI, const ShaderVersi { DEV_CHECK_ERR(ShaderCI.ByteCode == nullptr, "'ByteCode' must be null when shader is created from the source code or a file"); DEV_CHECK_ERR(ShaderCI.ByteCodeSize == 0, "'ByteCodeSize' must be 0 when shader is created from the source code or a file"); + DEV_CHECK_ERR(ShaderCI.EntryPoint != nullptr, "Entry point must not be null"); bool UseDXC = false; // validate compiler type switch (ShaderCI.ShaderCompiler) { - // clang-format off - case SHADER_COMPILER_DEFAULT: UseDXC = false; break; - case SHADER_COMPILER_DXC: UseDXC = DxCompiler != nullptr && DxCompiler->IsLoaded(); break; - case SHADER_COMPILER_FXC: UseDXC = false; break; - // clang-format on + case SHADER_COMPILER_DEFAULT: + UseDXC = false; + break; + + case SHADER_COMPILER_DXC: + UseDXC = DxCompiler != nullptr && DxCompiler->IsLoaded(); + if (!UseDXC) + LOG_WARNING_MESSAGE("DXC compiler is not available. Using default shader compiler"); + break; + + case SHADER_COMPILER_FXC: + UseDXC = false; + break; + default: UNEXPECTED("Unsupported shader compiler"); } @@ -140,37 +150,10 @@ ShaderD3DBase::ShaderD3DBase(const ShaderCreateInfo& ShaderCI, const ShaderVersi String ShaderSource = BuildHLSLSourceString(ShaderCI); - DEV_CHECK_ERR(ShaderCI.EntryPoint != nullptr, "Entry point must not be null"); - - CComPtr errors; - - auto hr = CompileShader(ShaderSource.c_str(), ShaderSource.length(), ShaderCI, strShaderProfile.c_str(), &m_pShaderByteCode, &errors); - - const size_t CompilerMsgLen = errors ? errors->GetBufferSize() : 0; - const char* CompilerMsg = CompilerMsgLen > 0 ? static_cast(errors->GetBufferPointer()) : nullptr; - - if (CompilerMsg != nullptr && ShaderCI.ppCompilerOutput != nullptr) - { - auto* pOutputDataBlob = MakeNewRCObj()(CompilerMsgLen + 1 + ShaderSource.length() + 1); - char* DataPtr = static_cast(pOutputDataBlob->GetDataPtr()); - memcpy(DataPtr, CompilerMsg, CompilerMsgLen); - DataPtr[CompilerMsgLen] = 0; // Set null terminator - memcpy(DataPtr + CompilerMsgLen + 1, ShaderSource.data(), ShaderSource.length() + 1); - pOutputDataBlob->QueryInterface(IID_DataBlob, reinterpret_cast(ShaderCI.ppCompilerOutput)); - } - - if (FAILED(hr)) - { - ComErrorDesc ErrDesc(hr); - if (ShaderCI.ppCompilerOutput != nullptr) - { - LOG_ERROR_AND_THROW("Failed to compile D3D shader \"", (ShaderCI.Desc.Name != nullptr ? ShaderCI.Desc.Name : ""), "\" (", ErrDesc.Get(), ")."); - } - else - { - LOG_ERROR_AND_THROW("Failed to compile D3D shader \"", (ShaderCI.Desc.Name != nullptr ? ShaderCI.Desc.Name : ""), "\" (", ErrDesc.Get(), "):\n", (CompilerMsg != nullptr ? CompilerMsg : "")); - } - } + CComPtr CompilerOutput; + + auto hr = CompileShader(ShaderSource.c_str(), ShaderSource.length(), ShaderCI, strShaderProfile.c_str(), &m_pShaderByteCode, &CompilerOutput); + HandleHLSLCompilerResult(SUCCEEDED(hr), CompilerOutput.p, ShaderSource, ShaderCI.Desc.Name, ShaderCI.ppCompilerOutput); } } else if (ShaderCI.ByteCode) diff --git a/Graphics/GraphicsEngineOpenGL/include/ShaderGLImpl.hpp b/Graphics/GraphicsEngineOpenGL/include/ShaderGLImpl.hpp index 374fe0c2..bc0f32e6 100644 --- a/Graphics/GraphicsEngineOpenGL/include/ShaderGLImpl.hpp +++ b/Graphics/GraphicsEngineOpenGL/include/ShaderGLImpl.hpp @@ -80,7 +80,7 @@ public: ShaderGLImpl(IReferenceCounters* pRefCounters, RenderDeviceGLImpl* pDeviceGL, - const ShaderCreateInfo& ShaderCreateInfo, + const ShaderCreateInfo& ShaderCI, bool bIsDeviceInternal = false); ~ShaderGLImpl(); diff --git a/Graphics/GraphicsEngineOpenGL/src/ShaderGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/ShaderGLImpl.cpp index c83905f8..f2707fd4 100644 --- a/Graphics/GraphicsEngineOpenGL/src/ShaderGLImpl.cpp +++ b/Graphics/GraphicsEngineOpenGL/src/ShaderGLImpl.cpp @@ -40,26 +40,26 @@ namespace Diligent ShaderGLImpl::ShaderGLImpl(IReferenceCounters* pRefCounters, RenderDeviceGLImpl* pDeviceGL, - const ShaderCreateInfo& CreationAttribs, + const ShaderCreateInfo& ShaderCI, bool bIsDeviceInternal) : // clang-format off TShaderBase { pRefCounters, pDeviceGL, - CreationAttribs.Desc, + ShaderCI.Desc, bIsDeviceInternal }, m_GLShaderObj{true, GLObjectWrappers::GLShaderObjCreateReleaseHelper{GetGLShaderType(m_Desc.ShaderType)}} // clang-format on { - DEV_CHECK_ERR(CreationAttribs.ByteCode == nullptr, "'ByteCode' must be null when shader is created from the source code or a file"); - DEV_CHECK_ERR(CreationAttribs.ByteCodeSize == 0, "'ByteCodeSize' must be 0 when shader is created from the source code or a file"); - DEV_CHECK_ERR(CreationAttribs.ShaderCompiler == SHADER_COMPILER_DEFAULT, "only default compiler is supported in OpenGL"); + DEV_CHECK_ERR(ShaderCI.ByteCode == nullptr, "'ByteCode' must be null when shader is created from the source code or a file"); + DEV_CHECK_ERR(ShaderCI.ByteCodeSize == 0, "'ByteCodeSize' must be 0 when shader is created from the source code or a file"); + DEV_CHECK_ERR(ShaderCI.ShaderCompiler == SHADER_COMPILER_DEFAULT, "only default compiler is supported in OpenGL"); const auto& deviceCaps = pDeviceGL->GetDeviceCaps(); - auto GLSLSource = BuildGLSLSourceString(CreationAttribs, deviceCaps, TargetGLSLCompiler::driver); + auto GLSLSource = BuildGLSLSourceString(ShaderCI, deviceCaps, TargetGLSLCompiler::driver); // Note: there is a simpler way to create the program: //m_uiShaderSeparateProg = glCreateShaderProgramv(GL_VERTEX_SHADER, _countof(ShaderStrings), ShaderStrings); @@ -92,7 +92,7 @@ ShaderGLImpl::ShaderGLImpl(IReferenceCounters* pRefCounters, FullSource.append(str); std::stringstream ErrorMsgSS; - ErrorMsgSS << "Failed to compile shader file '" << (CreationAttribs.Desc.Name != nullptr ? CreationAttribs.Desc.Name : "") << '\'' << std::endl; + ErrorMsgSS << "Failed to compile shader file '" << (ShaderCI.Desc.Name != nullptr ? ShaderCI.Desc.Name : "") << '\'' << std::endl; int infoLogLen = 0; // The function glGetShaderiv() tells how many bytes to allocate; the length includes the NULL terminator. glGetShaderiv(m_GLShaderObj, GL_INFO_LOG_LENGTH, &infoLogLen); @@ -110,7 +110,7 @@ ShaderGLImpl::ShaderGLImpl(IReferenceCounters* pRefCounters, << infoLog.data() << std::endl; } - if (CreationAttribs.ppCompilerOutput != nullptr) + if (ShaderCI.ppCompilerOutput != nullptr) { // infoLogLen accounts for null terminator auto* pOutputDataBlob = MakeNewRCObj()(infoLogLen + FullSource.length() + 1); @@ -118,7 +118,7 @@ ShaderGLImpl::ShaderGLImpl(IReferenceCounters* pRefCounters, if (infoLogLen > 0) memcpy(DataPtr, infoLog.data(), infoLogLen); memcpy(DataPtr + infoLogLen, FullSource.data(), FullSource.length() + 1); - pOutputDataBlob->QueryInterface(IID_DataBlob, reinterpret_cast(CreationAttribs.ppCompilerOutput)); + pOutputDataBlob->QueryInterface(IID_DataBlob, reinterpret_cast(ShaderCI.ppCompilerOutput)); } else { diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp index 033f711a..c16010d8 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp @@ -64,19 +64,24 @@ ShaderVkImpl::ShaderVkImpl(IReferenceCounters* pRefCounters, "# define VULKAN 1\n" "#endif\n"; - switch (CreationAttribs.ShaderCompiler) + auto ShaderCompiler = CreationAttribs.ShaderCompiler; + if (ShaderCompiler == SHADER_COMPILER_DXC) + { + auto* pDXComiler = pRenderDeviceVk->GetDxCompiler(); + if (pDXComiler == nullptr || !pDXComiler->IsLoaded()) + { + LOG_WARNING_MESSAGE("DX Compiler is not loaded. Using default shader compiler"); + ShaderCompiler = SHADER_COMPILER_DEFAULT; + } + } + + switch (ShaderCompiler) { case SHADER_COMPILER_DXC: { auto* pDXComiler = pRenderDeviceVk->GetDxCompiler(); - if (pDXComiler != nullptr && pDXComiler->IsLoaded()) - { - pDXComiler->Compile(CreationAttribs, VulkanDefine, nullptr, &m_SPIRV, CreationAttribs.ppCompilerOutput); - } - else - { - LOG_ERROR_AND_THROW("DX Compiler is not loaded"); - } + VERIFY_EXPR(pDXComiler != nullptr && pDXComiler->IsLoaded()); + pDXComiler->Compile(CreationAttribs, VulkanDefine, nullptr, &m_SPIRV, CreationAttribs.ppCompilerOutput); } break; @@ -84,7 +89,7 @@ ShaderVkImpl::ShaderVkImpl(IReferenceCounters* pRefCounters, case SHADER_COMPILER_GLSLANG: { #if DILIGENT_NO_GLSLANG - LOG_ERROR_AND_THROW("Diligent engine was not linked with glslang, use DXIL compiler or precompiled SPIRV bytecode."); + LOG_ERROR_AND_THROW("Diligent engine was not linked with glslang, use DXC or precompiled SPIRV bytecode."); #else if (CreationAttribs.SourceLanguage == SHADER_SOURCE_LANGUAGE_HLSL) { diff --git a/Graphics/ShaderTools/include/HLSLUtils.hpp b/Graphics/ShaderTools/include/HLSLUtils.hpp index 9abd1e7d..0e36c7b4 100644 --- a/Graphics/ShaderTools/include/HLSLUtils.hpp +++ b/Graphics/ShaderTools/include/HLSLUtils.hpp @@ -27,6 +27,8 @@ #pragma once +#include + #include "BasicTypes.h" #include "GraphicsTypes.h" #include "Shader.h" @@ -39,4 +41,55 @@ String BuildHLSLSourceString(const ShaderCreateInfo& ShaderCI, String GetHLSLProfileString(SHADER_TYPE ShaderType, ShaderVersion ShaderModel); +template +void HandleHLSLCompilerResult(bool CompilationSucceeded, + BlobType* pCompilerMsgBlob, + const std::string& ShaderSource, + const char* ShaderName, + IDataBlob** ppOutputLog) noexcept(false) +{ + const char* CompilerMsg = pCompilerMsgBlob ? static_cast(pCompilerMsgBlob->GetBufferPointer()) : nullptr; + const size_t CompilerMsgLen = CompilerMsg ? pCompilerMsgBlob->GetBufferSize() : 0; + + if (ppOutputLog != nullptr) + { + const auto ShaderSourceLen = ShaderSource.length(); + auto* pOutputLogBlob = MakeNewRCObj{}(ShaderSourceLen + 1 + CompilerMsgLen + 1); + + auto* log = static_cast(pOutputLogBlob->GetDataPtr()); + + if (CompilerMsg != nullptr) + memcpy(log, CompilerMsg, CompilerMsgLen); + log[CompilerMsgLen] = 0; // Explicitly set null terminator + log += CompilerMsgLen + 1; + + memcpy(log, ShaderSource.data(), ShaderSourceLen); + log[ShaderSourceLen] = 0; + + pOutputLogBlob->QueryInterface(IID_DataBlob, reinterpret_cast(ppOutputLog)); + } + + if (!CompilationSucceeded || CompilerMsgLen != 0) + { + std::stringstream ss; + ss << (CompilationSucceeded ? "Compiler output for shader '" : "Failed to compile shader '") + << (ShaderName != nullptr ? ShaderName : "") + << "'"; + if (CompilerMsg != nullptr && CompilerMsgLen != 0) + { + ss << ":" << std::endl + << CompilerMsg; + } + else if (!CompilationSucceeded) + { + ss << " (no shader log available)."; + } + + if (CompilationSucceeded) + LOG_INFO_MESSAGE(ss.str()); + else + LOG_ERROR_AND_THROW(ss.str()); + } +} + } // namespace Diligent diff --git a/Graphics/ShaderTools/src/DXCompiler.cpp b/Graphics/ShaderTools/src/DXCompiler.cpp index 3ce6066f..ad7a0023 100644 --- a/Graphics/ShaderTools/src/DXCompiler.cpp +++ b/Graphics/ShaderTools/src/DXCompiler.cpp @@ -459,8 +459,8 @@ void DXCompilerImpl::Compile(const ShaderCreateInfo& ShaderCI, #endif }; - CComPtr compiled; - CComPtr errors; + CComPtr pCompiledShader; + CComPtr pDxcLog; IDXCompiler::CompileAttribs CA; @@ -475,44 +475,20 @@ void DXCompilerImpl::Compile(const ShaderCreateInfo& ShaderCI, CA.pArgs = m_Target == DXCompilerTarget::Direct3D12 ? pDxbcArgs : pSpirvArgs; CA.ArgsCount = m_Target == DXCompilerTarget::Direct3D12 ? _countof(pDxbcArgs) : _countof(pSpirvArgs); CA.pShaderSourceStreamFactory = ShaderCI.pShaderSourceStreamFactory; - CA.ppBlobOut = &compiled; - CA.ppCompilerOutput = &errors; + CA.ppBlobOut = &pCompiledShader; + CA.ppCompilerOutput = &pDxcLog; auto result = Compile(CA); + HandleHLSLCompilerResult(result, pDxcLog.p, Source, ShaderCI.Desc.Name, ppCompilerOutput); - const size_t CompilerMsgLen = errors ? errors->GetBufferSize() : 0; - const char* CompilerMsg = CompilerMsgLen > 0 ? static_cast(errors->GetBufferPointer()) : nullptr; - - if (CompilerMsg != nullptr && ppCompilerOutput != nullptr) - { - auto* pOutputDataBlob = MakeNewRCObj()(Source.length() + 1 + CompilerMsgLen + 1); - char* DataPtr = static_cast(pOutputDataBlob->GetDataPtr()); - memcpy(DataPtr, CompilerMsg, CompilerMsgLen); - DataPtr[CompilerMsgLen] = 0; // Set null terminator as CompilerMsgLen may not account for it - memcpy(DataPtr + CompilerMsgLen + 1, Source.data(), Source.length() + 1); - pOutputDataBlob->QueryInterface(IID_DataBlob, reinterpret_cast(ppCompilerOutput)); - } - - if (!result) - { - if (ppCompilerOutput != nullptr) - { - LOG_ERROR_AND_THROW("Failed to compile Vulkan shader \"", (ShaderCI.Desc.Name != nullptr ? ShaderCI.Desc.Name : ""), "\"."); - } - else - { - LOG_ERROR_AND_THROW("Failed to compile Vukan shader \"", (ShaderCI.Desc.Name != nullptr ? ShaderCI.Desc.Name : ""), "\":\n", (CompilerMsg != nullptr ? std::string(CompilerMsg, CompilerMsgLen) : "")); - } - } - - if (result && compiled && compiled->GetBufferSize() > 0) + if (result && pCompiledShader && pCompiledShader->GetBufferSize() > 0) { if (pByteCode != nullptr) - pByteCode->assign(static_cast(compiled->GetBufferPointer()), - static_cast(compiled->GetBufferPointer()) + compiled->GetBufferSize() / sizeof(uint32_t)); + pByteCode->assign(static_cast(pCompiledShader->GetBufferPointer()), + static_cast(pCompiledShader->GetBufferPointer()) + pCompiledShader->GetBufferSize() / sizeof(uint32_t)); if (ppByteCodeBlob != nullptr) - *ppByteCodeBlob = compiled.Detach(); + *ppByteCodeBlob = pCompiledShader.Detach(); } } -- cgit v1.2.3