From 4d9262b271dbba19859d7df3ae6eea80902adfaf Mon Sep 17 00:00:00 2001 From: hzqst <113660872@qq.com> Date: Fri, 26 Jun 2026 15:36:35 +0800 Subject: [PATCH 1/5] ShaderTools: resolve nested local includes relative to parent Preserve local and system include semantics when processing and unrolling shader includes. Support both path separators for memory-backed source factories and add regression tests. --- .../ShaderTools/src/ShaderToolsCommon.cpp | 91 +++++++++++-- .../IncludeNestedParentRelativeTest.hlsl | 3 + .../ShaderPreprocessor/Nested/Config.hlsl | 3 + .../ShaderPreprocessor/Nested/Types.hlsl | 3 + .../src/ShaderTools/ShaderPreprocessTest.cpp | 122 +++++++++++++++++- 5 files changed, 212 insertions(+), 10 deletions(-) create mode 100644 Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/IncludeNestedParentRelativeTest.hlsl create mode 100644 Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Config.hlsl create mode 100644 Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Types.hlsl diff --git a/Graphics/ShaderTools/src/ShaderToolsCommon.cpp b/Graphics/ShaderTools/src/ShaderToolsCommon.cpp index dd6c8bebe4..5a0bda0286 100644 --- a/Graphics/ShaderTools/src/ShaderToolsCommon.cpp +++ b/Graphics/ShaderTools/src/ShaderToolsCommon.cpp @@ -374,7 +374,8 @@ bool FindIncludes(const char* pBuffer, size_t BufferSize, HandlerType&& IncludeH if (pCurrPos < pBufferEnd && *pCurrPos != '<' && *pCurrPos != '"') throw ErrorType{pCurrPos, "\'<\' or \'\"\' is expected"}; - const char ClosingChar = *pCurrPos == '<' ? '>' : '"'; + const bool IsLocalInclude = *pCurrPos == '"'; + const char ClosingChar = IsLocalInclude ? '"' : '>'; ++pCurrPos; while (pCurrPos < pBufferEnd && *pCurrPos != ClosingChar) ++pCurrPos; @@ -385,7 +386,7 @@ bool FindIncludes(const char* pBuffer, size_t BufferSize, HandlerType&& IncludeH if (pCurrPos >= pLineEnd) throw ErrorType{pLineEnd, "New line in the file name."}; - IncludeHandler(std::string{pOpenQuoteOrAngleBracket + 1, pCurrPos}, pIncludeStart - pBuffer, pCurrPos - pBuffer + 1); + IncludeHandler(std::string{pOpenQuoteOrAngleBracket + 1, pCurrPos}, IsLocalInclude, pIncludeStart - pBuffer, pCurrPos - pBuffer + 1); ++pCurrPos; } @@ -424,6 +425,74 @@ static void ProcessIncludeErrorHandler(const ShaderCreateInfo& ShaderCI, const s throw std::pair{std::move(FileInfo), Error}; } +static Char GetFirstSlash(const char* Path) +{ + if (Path == nullptr) + return 0; + + for (const char* c = Path; *c != '\0'; ++c) + { + if (BasicFileSystem::IsSlash(*c)) + return *c; + } + + return 0; +} + +static Char GetPreferredIncludePathSlash(const ShaderCreateInfo& ShaderCI, const std::string& IncludeName) +{ + if (const Char IncludeSlash = GetFirstSlash(IncludeName.c_str())) + return IncludeSlash; + + if (const Char SourceSlash = GetFirstSlash(ShaderCI.FilePath)) + return SourceSlash; + + return BasicFileSystem::SlashSymbol; +} + +static std::string MakeParentRelativeIncludePath(const String& ParentDir, const std::string& IncludeName, Char Slash) +{ + return BasicFileSystem::SimplifyPath( + (ParentDir + Slash + IncludeName).c_str(), + Slash); +} + +static bool TryOpenShaderSource(IShaderSourceInputStreamFactory* pFactory, const std::string& FilePath) +{ + RefCntAutoPtr pSourceStream; + pFactory->CreateInputStream2(FilePath.c_str(), CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, &pSourceStream); + return pSourceStream != nullptr; +} + +static std::string ResolveIncludePathForPreprocess(const ShaderCreateInfo& ShaderCI, const std::string& IncludeName, bool IsLocalInclude) +{ + if (!IsLocalInclude || ShaderCI.FilePath == nullptr || BasicFileSystem::IsPathAbsolute(IncludeName.c_str())) + return IncludeName; + + String ParentDir; + BasicFileSystem::GetPathComponents(ShaderCI.FilePath, &ParentDir, nullptr); + if (ParentDir.empty()) + return IncludeName; + + const Char PreferredSlash = GetPreferredIncludePathSlash(ShaderCI, IncludeName); + const std::string RelativePath = MakeParentRelativeIncludePath(ParentDir, IncludeName, PreferredSlash); + + if (ShaderCI.pShaderSourceStreamFactory != nullptr) + { + if (TryOpenShaderSource(ShaderCI.pShaderSourceStreamFactory, RelativePath)) + return RelativePath; + + const Char AlternateSlash = PreferredSlash == BasicFileSystem::UnixSlash ? BasicFileSystem::WinSlash : BasicFileSystem::UnixSlash; + const std::string AlternatePath = MakeParentRelativeIncludePath(ParentDir, IncludeName, AlternateSlash); + if (AlternatePath != RelativePath && TryOpenShaderSource(ShaderCI.pShaderSourceStreamFactory, AlternatePath)) + return AlternatePath; + + return IncludeName; + } + + return RelativePath; +} + template void ProcessShaderIncludesImpl(const ShaderCreateInfo& ShaderCI, std::unordered_set& Includes, IncludeHandlerType&& IncludeHandler) noexcept(false) { @@ -436,13 +505,14 @@ void ProcessShaderIncludesImpl(const ShaderCreateInfo& ShaderCI, std::unordered_ FindIncludes( FileInfo.Source, FileInfo.SourceLength, - [&](const std::string& FilePath, size_t Start, size_t End) // + [&](const std::string& IncludeName, bool IsLocalInclude, size_t /*Start*/, size_t /*End*/) // { - if (!Includes.insert(FilePath).second) + const std::string ResolvedPath = ResolveIncludePathForPreprocess(ShaderCI, IncludeName, IsLocalInclude); + if (!Includes.insert(ResolvedPath).second) return; ShaderCreateInfo IncludeCI{ShaderCI}; - IncludeCI.FilePath = FilePath.c_str(); + IncludeCI.FilePath = ResolvedPath.c_str(); IncludeCI.Source = nullptr; IncludeCI.SourceLength = 0; ProcessShaderIncludesImpl(IncludeCI, Includes, IncludeHandler); @@ -477,6 +547,8 @@ static std::string UnrollShaderIncludesImpl(ShaderCreateInfo ShaderCI, std::unor { const ShaderSourceFileData SourceData = ReadShaderSourceFile(ShaderCI); + const ShaderCreateInfo IncludeContextCI{ShaderCI}; + ShaderCI.Source = SourceData.Source; ShaderCI.SourceLength = SourceData.SourceLength; ShaderCI.FilePath = nullptr; @@ -485,24 +557,25 @@ static std::string UnrollShaderIncludesImpl(ShaderCreateInfo ShaderCI, std::unor size_t PrevIncludeEnd = 0; FindIncludes( - ShaderCI.Source, ShaderCI.SourceLength, [&](const std::string& Path, size_t IncludeStart, size_t IncludeEnd) { + ShaderCI.Source, ShaderCI.SourceLength, [&](const std::string& Path, bool IsLocalInclude, size_t IncludeStart, size_t IncludeEnd) { // Insert text before the include start Stream.write(ShaderCI.Source + PrevIncludeEnd, IncludeStart - PrevIncludeEnd); - if (AllIncludes.insert(Path).second) + const std::string ResolvedPath = ResolveIncludePathForPreprocess(IncludeContextCI, Path, IsLocalInclude); + if (AllIncludes.insert(ResolvedPath).second) { // Process the #include directive ShaderCreateInfo IncludeCI{ShaderCI}; IncludeCI.Source = nullptr; IncludeCI.SourceLength = 0; - IncludeCI.FilePath = Path.c_str(); + IncludeCI.FilePath = ResolvedPath.c_str(); std::string UnrolledInclude = UnrollShaderIncludesImpl(IncludeCI, AllIncludes); Stream << UnrolledInclude; } PrevIncludeEnd = IncludeEnd; }, - std::bind(ProcessIncludeErrorHandler, ShaderCI, std::placeholders::_1)); + std::bind(ProcessIncludeErrorHandler, IncludeContextCI, std::placeholders::_1)); // Insert text after the last include Stream.write(ShaderCI.Source + PrevIncludeEnd, ShaderCI.SourceLength - PrevIncludeEnd); diff --git a/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/IncludeNestedParentRelativeTest.hlsl b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/IncludeNestedParentRelativeTest.hlsl new file mode 100644 index 0000000000..4688a6a230 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/IncludeNestedParentRelativeTest.hlsl @@ -0,0 +1,3 @@ +// Start IncludeNestedParentRelativeTest.hlsl +#include "Nested/Types.hlsl" +// End IncludeNestedParentRelativeTest.hlsl diff --git a/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Config.hlsl b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Config.hlsl new file mode 100644 index 0000000000..18ca53212c --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Config.hlsl @@ -0,0 +1,3 @@ +// Start Nested/Config.hlsl +#define NESTED_CONFIG_VALUE 1 +// End Nested/Config.hlsl diff --git a/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Types.hlsl b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Types.hlsl new file mode 100644 index 0000000000..2ed4739f7c --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/ShaderPreprocessor/Nested/Types.hlsl @@ -0,0 +1,3 @@ +// Start Nested/Types.hlsl +#include "Config.hlsl" +// End Nested/Types.hlsl diff --git a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp index dda824eafa..91ec77ba6c 100644 --- a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp +++ b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2019-2022 Diligent Graphics LLC + * Copyright 2019-2026 Diligent Graphics LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ #include "ShaderToolsCommon.hpp" #include "DefaultShaderSourceStreamFactory.h" #include "RenderDevice.h" +#include "ShaderSourceFactoryUtils.hpp" #include "TestingEnvironment.hpp" #include "gtest/gtest.h" @@ -135,6 +136,104 @@ TEST(ShaderPreprocessTest, Include) } } +TEST(ShaderPreprocessTest, IncludeNestedParentRelative) +{ + RefCntAutoPtr pShaderSourceFactory; + CreateDefaultShaderSourceStreamFactory("shaders/ShaderPreprocessor", &pShaderSourceFactory); + ASSERT_NE(pShaderSourceFactory, nullptr); + + std::deque Includes{ + "Nested/Config.hlsl", + "Nested/Types.hlsl", + "IncludeNestedParentRelativeTest.hlsl"}; + + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "IncludeNestedParentRelativeTest.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + const auto Result = ProcessShaderIncludes(ShaderCI, [&](const ShaderIncludePreprocessInfo& ProcessInfo) { + ASSERT_FALSE(Includes.empty()); + EXPECT_EQ(ProcessInfo.FilePath, Includes.front()); + Includes.pop_front(); + }); + + EXPECT_EQ(Result, true); + EXPECT_TRUE(Includes.empty()); +} + +TEST(ShaderPreprocessTest, IncludeNestedLocalAndSystemFromMemory) +{ + auto pShaderSourceFactory = CreateMemoryShaderSourceFactory( + { + {"Main.hlsl", + "// Start Main.hlsl\n" + "#include \"Nested/LocalTypes.hlsl\"\n" + "#include \"Nested/SystemTypes.hlsl\"\n" + "// End Main.hlsl\n"}, + {"Nested/LocalTypes.hlsl", + "// Start Nested/LocalTypes.hlsl\n" + "#include \"Config.hlsl\"\n" + "// End Nested/LocalTypes.hlsl\n"}, + {"Nested/SystemTypes.hlsl", + "// Start Nested/SystemTypes.hlsl\n" + "#include \n" + "// End Nested/SystemTypes.hlsl\n"}, + {"Nested/Config.hlsl", + "// Start Nested/Config.hlsl\n" + "#define CONFIG_SOURCE 2\n" + "// End Nested/Config.hlsl\n"}, + {"Config.hlsl", + "// Start Config.hlsl\n" + "#define CONFIG_SOURCE 1\n" + "// End Config.hlsl\n"}, + }, + false); + ASSERT_NE(pShaderSourceFactory, nullptr); + + std::deque Includes{ + "Nested/Config.hlsl", + "Nested/LocalTypes.hlsl", + "Config.hlsl", + "Nested/SystemTypes.hlsl", + "Main.hlsl"}; + + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "Main.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + const auto Result = ProcessShaderIncludes(ShaderCI, [&](const ShaderIncludePreprocessInfo& ProcessInfo) { + ASSERT_FALSE(Includes.empty()); + EXPECT_EQ(ProcessInfo.FilePath, Includes.front()); + Includes.pop_front(); + }); + + EXPECT_EQ(Result, true); + EXPECT_TRUE(Includes.empty()); + + constexpr char RefString[] = + "// Start Main.hlsl\n" + "// Start Nested/LocalTypes.hlsl\n" + "// Start Nested/Config.hlsl\n" + "#define CONFIG_SOURCE 2\n" + "// End Nested/Config.hlsl\n" + "\n" + "// End Nested/LocalTypes.hlsl\n" + "\n" + "// Start Nested/SystemTypes.hlsl\n" + "// Start Config.hlsl\n" + "#define CONFIG_SOURCE 1\n" + "// End Config.hlsl\n" + "\n" + "// End Nested/SystemTypes.hlsl\n" + "\n" + "// End Main.hlsl\n"; + + auto UnrolledStr = UnrollShaderIncludes(ShaderCI); + ASSERT_EQ(RefString, UnrolledStr); +} + TEST(ShaderPreprocessTest, InvalidInclude) { RefCntAutoPtr pShaderSourceFactory; @@ -192,6 +291,27 @@ TEST(ShaderPreprocessTest, UnrollIncludes) auto UnrolledStr = UnrollShaderIncludes(ShaderCI); ASSERT_EQ(RefString, UnrolledStr); } + + { + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "IncludeNestedParentRelativeTest.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + constexpr char RefString[] = + "// Start IncludeNestedParentRelativeTest.hlsl\n" + "// Start Nested/Types.hlsl\n" + "// Start Nested/Config.hlsl\n" + "#define NESTED_CONFIG_VALUE 1\n" + "// End Nested/Config.hlsl\n" + "\n" + "// End Nested/Types.hlsl\n" + "\n" + "// End IncludeNestedParentRelativeTest.hlsl\n"; + + auto UnrolledStr = UnrollShaderIncludes(ShaderCI); + ASSERT_EQ(RefString, UnrolledStr); + } } TEST(ShaderPreprocessTest, ShaderSourceLanguageDefiniton) From dd45c6787abd7149e9460d3ab5bf7e6dcc606a29 Mon Sep 17 00:00:00 2001 From: assiduous Date: Sat, 18 Jul 2026 20:26:00 -0700 Subject: [PATCH 2/5] BasicFileSystem: add platform-independent path root detection Add PathRootType and GetPathRootType() to distinguish Unix, Windows drive, and UNC roots regardless of the host platform. --- Graphics/GraphicsEngine/interface/APIInfo.h | 2 +- Graphics/GraphicsEngine/interface/Shader.h | 16 ++++-- .../src/DefaultShaderSourceStreamFactory.cpp | 54 +++++++++--------- .../src/ShaderSourceFactoryUtils.cpp | 57 +++++++++++-------- .../ShaderTools/src/ShaderToolsCommon.cpp | 10 ++-- Platforms/Basic/interface/BasicFileSystem.hpp | 11 ++++ Platforms/Basic/src/BasicFileSystem.cpp | 17 ++++++ ReleaseHistory.md | 1 + .../src/ShaderCreationTest.cpp | 27 +++++---- .../src/Platforms/FileSystemTest.cpp | 24 ++++++++ .../src/ShaderTools/ShaderPreprocessTest.cpp | 43 ++++++++++++++ 11 files changed, 191 insertions(+), 71 deletions(-) diff --git a/Graphics/GraphicsEngine/interface/APIInfo.h b/Graphics/GraphicsEngine/interface/APIInfo.h index 0cf7b5d037..65963ea0bd 100644 --- a/Graphics/GraphicsEngine/interface/APIInfo.h +++ b/Graphics/GraphicsEngine/interface/APIInfo.h @@ -30,7 +30,7 @@ /// \file /// Diligent API information -#define DILIGENT_API_VERSION 256019 +#define DILIGENT_API_VERSION 256020 #include "../../../Primitives/interface/BasicTypes.h" diff --git a/Graphics/GraphicsEngine/interface/Shader.h b/Graphics/GraphicsEngine/interface/Shader.h index f035c0d2a9..7c430bc3e3 100644 --- a/Graphics/GraphicsEngine/interface/Shader.h +++ b/Graphics/GraphicsEngine/interface/Shader.h @@ -241,8 +241,12 @@ DILIGENT_BEGIN_INTERFACE(IShaderSourceInputStreamFactory, IObject) /// The stream is used to load the shader source code. /// \param [in] Name - The name of the file to load. - /// \param [out] ppStream - Pointer to the shader source input stream. - VIRTUAL void METHOD(CreateInputStream)(THIS_ + /// \param [out] ppStream - Pointer to the shader source input stream. May be null to only check + /// if the source exists. The check never reports an error if the source + /// is not found. + /// \return True if the source exists and, when \p ppStream is not null, the stream was + /// created successfully. False otherwise. + VIRTUAL Bool METHOD(CreateInputStream)(THIS_ const Char* Name, IFileStream** ppStream) PURE; @@ -252,8 +256,12 @@ DILIGENT_BEGIN_INTERFACE(IShaderSourceInputStreamFactory, IObject) /// \param [in] Name - The name of the file to load. /// \param [in] Flags - Flags that control the stream creation, /// see Diligent::CREATE_SHADER_SOURCE_INPUT_STREAM_FLAGS. - /// \param [out] ppStream - Pointer to the shader source input stream. - VIRTUAL void METHOD(CreateInputStream2)(THIS_ + /// \param [out] ppStream - Pointer to the shader source input stream. May be null to only check + /// if the source exists. The check never reports an error if the source + /// is not found, regardless of \p Flags. + /// \return True if the source exists and, when \p ppStream is not null, the stream was + /// created successfully. False otherwise. + VIRTUAL Bool METHOD(CreateInputStream2)(THIS_ const Char* Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAGS Flags, IFileStream** ppStream) PURE; diff --git a/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp b/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp index 90e1d726d3..ee72d952ea 100644 --- a/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp +++ b/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp @@ -40,9 +40,9 @@ class DefaultShaderSourceStreamFactory final : public ObjectBase pFileStream; - if (FileSystem::FileExists(Path)) + Bool SourceFound = FileSystem::FileExists(Path); + if (SourceFound && ppStream != nullptr) { - pFileStream = BasicFileStream::Create(Path, EFileAccessMode::Read); - if (!pFileStream->IsValid()) - pFileStream.Release(); + RefCntAutoPtr pFileStream = BasicFileStream::Create(Path, EFileAccessMode::Read); + + SourceFound = pFileStream->IsValid(); + if (SourceFound) + { + pFileStream->QueryInterface(IID_FileStream, ppStream); + SourceFound = *ppStream != nullptr; + } } - return pFileStream; + return SourceFound; }; - RefCntAutoPtr pFileStream; + DEV_CHECK_ERR(ppStream == nullptr || *ppStream == nullptr, "Output stream pointer must be null. Overwriting a non-null output pointer may result in memory leaks."); + + Bool SourceFound = False; if (FileSystem::IsPathAbsolute(Name)) { - pFileStream = CreateFileStream(Name); + SourceFound = TryCreateFileStream(Name, ppStream); } else { for (const std::string& SearchDir : m_SearchDirectories) { const std::string FullPath = SearchDir + ((Name[0] == '\\' || Name[0] == '/') ? Name + 1 : Name); - pFileStream = CreateFileStream(FullPath.c_str()); - if (pFileStream) + + SourceFound = TryCreateFileStream(FullPath.c_str(), ppStream); + if (SourceFound) break; } } - if (pFileStream) - { - pFileStream->QueryInterface(IID_FileStream, ppStream); - } - else + if (!SourceFound && ppStream != nullptr && (Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) { - *ppStream = nullptr; - if ((Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) - { - LOG_ERROR("Failed to create input stream for source file ", Name); - } + LOG_ERROR("Failed to create input stream for source file ", Name); } + + return SourceFound; } void CreateDefaultShaderSourceStreamFactory(const Char* SearchDirectories, diff --git a/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp b/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp index b01e1e1e1c..04dd1c3d8e 100644 --- a/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp +++ b/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2023-2025 Diligent Graphics LLC + * Copyright 2023-2026 Diligent Graphics LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -73,17 +73,17 @@ class CompoundShaderSourceFactory : public ObjectBasesecond.c_str(); } - for (size_t i = 0; i < m_pFactories.size() && *ppStream == nullptr; ++i) + Bool SourceFound = False; + for (size_t i = 0; i < m_pFactories.size() && !SourceFound; ++i) { if (m_pFactories[i]) - m_pFactories[i]->CreateInputStream2(Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, ppStream); + SourceFound = m_pFactories[i]->CreateInputStream2(Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, ppStream); } - if (*ppStream == nullptr && (Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) != 0) + if (!SourceFound && ppStream != nullptr && (Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) { - LOG_ERROR("Failed to create input stream for source file ", Name); + LOG_ERROR_MESSAGE("Failed to create input stream for source file ", Name); } + + return SourceFound; } private: @@ -158,31 +161,39 @@ class MemoryShaderSourceFactory final : public ObjectBase pDataBlob{MakeNewRCObj()(SourceIt->second)}; - RefCntAutoPtr pMemStream{MakeNewRCObj()(pDataBlob)}; + DEV_CHECK_ERR(*ppStream == nullptr, "Output stream pointer must be null. Overwriting a non-null output pointer may result in memory leaks."); + if (SourceFound) + { + RefCntAutoPtr pDataBlob{MakeNewRCObj()(SourceIt->second)}; + RefCntAutoPtr pMemStream{MakeNewRCObj()(pDataBlob)}; - pMemStream->QueryInterface(IID_FileStream, ppStream); - } - else - { - *ppStream = nullptr; - if ((Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) + pMemStream->QueryInterface(IID_FileStream, ppStream); + SourceFound = *ppStream != nullptr; + } + else { - LOG_ERROR("Failed to create input stream for source file ", Name); + *ppStream = nullptr; + if ((Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) + { + LOG_ERROR_MESSAGE("Failed to create input stream for source file ", Name); + } } } + + return SourceFound; } IMPLEMENT_QUERY_INTERFACE_IN_PLACE(IID_IShaderSourceInputStreamFactory, TBase) diff --git a/Graphics/ShaderTools/src/ShaderToolsCommon.cpp b/Graphics/ShaderTools/src/ShaderToolsCommon.cpp index 5a0bda0286..325b85c372 100644 --- a/Graphics/ShaderTools/src/ShaderToolsCommon.cpp +++ b/Graphics/ShaderTools/src/ShaderToolsCommon.cpp @@ -457,11 +457,9 @@ static std::string MakeParentRelativeIncludePath(const String& ParentDir, const Slash); } -static bool TryOpenShaderSource(IShaderSourceInputStreamFactory* pFactory, const std::string& FilePath) +static bool ShaderSourceExists(IShaderSourceInputStreamFactory* pFactory, const std::string& FilePath) { - RefCntAutoPtr pSourceStream; - pFactory->CreateInputStream2(FilePath.c_str(), CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, &pSourceStream); - return pSourceStream != nullptr; + return pFactory->CreateInputStream(FilePath.c_str(), nullptr); } static std::string ResolveIncludePathForPreprocess(const ShaderCreateInfo& ShaderCI, const std::string& IncludeName, bool IsLocalInclude) @@ -479,12 +477,12 @@ static std::string ResolveIncludePathForPreprocess(const ShaderCreateInfo& Shade if (ShaderCI.pShaderSourceStreamFactory != nullptr) { - if (TryOpenShaderSource(ShaderCI.pShaderSourceStreamFactory, RelativePath)) + if (ShaderSourceExists(ShaderCI.pShaderSourceStreamFactory, RelativePath)) return RelativePath; const Char AlternateSlash = PreferredSlash == BasicFileSystem::UnixSlash ? BasicFileSystem::WinSlash : BasicFileSystem::UnixSlash; const std::string AlternatePath = MakeParentRelativeIncludePath(ParentDir, IncludeName, AlternateSlash); - if (AlternatePath != RelativePath && TryOpenShaderSource(ShaderCI.pShaderSourceStreamFactory, AlternatePath)) + if (AlternatePath != RelativePath && ShaderSourceExists(ShaderCI.pShaderSourceStreamFactory, AlternatePath)) return AlternatePath; return IncludeName; diff --git a/Platforms/Basic/interface/BasicFileSystem.hpp b/Platforms/Basic/interface/BasicFileSystem.hpp index bd59690ea4..290e178fd8 100644 --- a/Platforms/Basic/interface/BasicFileSystem.hpp +++ b/Platforms/Basic/interface/BasicFileSystem.hpp @@ -51,6 +51,14 @@ enum class FilePosOrigin End }; +enum class PathRootType +{ + None, + Unix, + WindowsDrive, + WindowsUNC +}; + struct FileOpenAttribs { const Char* strFilePath; @@ -172,6 +180,9 @@ struct BasicFileSystem String* Directory, String* FileName); + /// Returns the type of the path root independently of the current platform. + static PathRootType GetPathRootType(const Char* Path); + static bool IsPathAbsolute(const Char* Path); /// Splits path into individual components optionally simplifying it. diff --git a/Platforms/Basic/src/BasicFileSystem.cpp b/Platforms/Basic/src/BasicFileSystem.cpp index 3d6173bd1d..39a226f777 100644 --- a/Platforms/Basic/src/BasicFileSystem.cpp +++ b/Platforms/Basic/src/BasicFileSystem.cpp @@ -125,6 +125,23 @@ void BasicFileSystem::GetPathComponents(const String& Path, } } +PathRootType BasicFileSystem::GetPathRootType(const Char* Path) +{ + if (Path == nullptr || Path[0] == '\0') + return PathRootType::None; + + if (IsSlash(Path[0]) && IsSlash(Path[1])) + return PathRootType::WindowsUNC; + + if (Path[1] == ':' && IsSlash(Path[2])) + return PathRootType::WindowsDrive; + + if (IsSlash(Path[0])) + return PathRootType::Unix; + + return PathRootType::None; +} + bool BasicFileSystem::IsPathAbsolute(const Char* strPath) { if (strPath == nullptr || strPath[0] == 0) diff --git a/ReleaseHistory.md b/ReleaseHistory.md index 1ebeb117b8..2e44f3aa74 100644 --- a/ReleaseHistory.md +++ b/ReleaseHistory.md @@ -2,6 +2,7 @@ ## Current progress +* Added success results and probe mode to `IShaderSourceInputStreamFactory::CreateInputStream()` and `CreateInputStream2()` (API256020) * Added `SHADER_OPTIMIZATION_LEVEL` enum and `ShaderCreateInfo::ShaderOptimizationLevel` member (API256019) * Added `ShaderFloat64` and `ShaderBarycentrics` members to `DeviceFeatures` struct (API256018) * Added `DrawMeshIndirectAttribs::pMtlAttribs` member (API256017) diff --git a/Tests/DiligentCoreAPITest/src/ShaderCreationTest.cpp b/Tests/DiligentCoreAPITest/src/ShaderCreationTest.cpp index d019c7be02..a70f1cc799 100644 --- a/Tests/DiligentCoreAPITest/src/ShaderCreationTest.cpp +++ b/Tests/DiligentCoreAPITest/src/ShaderCreationTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2025 Diligent Graphics LLC + * Copyright 2025-2026 Diligent Graphics LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -144,22 +144,27 @@ TEST(ShaderCreationTest, FromBytecode) { } - virtual void DILIGENT_CALL_TYPE CreateInputStream(const Char* Name, IFileStream** ppStream) override final + virtual Bool DILIGENT_CALL_TYPE CreateInputStream(const Char* Name, IFileStream** ppStream) override final { - CreateInputStream2(Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, ppStream); + return CreateInputStream2(Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, ppStream); } - virtual void DILIGENT_CALL_TYPE CreateInputStream2(const Char* Name, + virtual Bool DILIGENT_CALL_TYPE CreateInputStream2(const Char* Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAGS Flags, IFileStream** ppStream) override final { - if (strcmp(Name, "ps.bc") != 0) - return; - - RefCntAutoPtr pDataBlob = ProxyDataBlob::Create(m_Bytecode.data(), m_Bytecode.size()); - RefCntAutoPtr pMemStream{MakeNewRCObj()(pDataBlob)}; - - pMemStream->QueryInterface(IID_FileStream, reinterpret_cast(ppStream)); + if (ppStream != nullptr) + *ppStream = nullptr; + Bool SourceFound = strcmp(Name, "ps.bc") == 0; + if (SourceFound && ppStream != nullptr) + { + RefCntAutoPtr pDataBlob = ProxyDataBlob::Create(m_Bytecode.data(), m_Bytecode.size()); + RefCntAutoPtr pMemStream{MakeNewRCObj()(pDataBlob)}; + + pMemStream->QueryInterface(IID_FileStream, reinterpret_cast(ppStream)); + SourceFound = *ppStream != nullptr; + } + return SourceFound; } private: diff --git a/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp b/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp index 99a4322f5b..b4e5ac3d65 100644 --- a/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp +++ b/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp @@ -160,6 +160,30 @@ TEST(Platforms_FileSystem, SplitPath) } +TEST(Platforms_FileSystem, GetPathRootType) +{ + EXPECT_EQ(FileSystem::GetPathRootType(nullptr), PathRootType::None); + EXPECT_EQ(FileSystem::GetPathRootType(""), PathRootType::None); + EXPECT_EQ(FileSystem::GetPathRootType("File"), PathRootType::None); + EXPECT_EQ(FileSystem::GetPathRootType("Directory/File"), PathRootType::None); + EXPECT_EQ(FileSystem::GetPathRootType("C:"), PathRootType::None); + EXPECT_EQ(FileSystem::GetPathRootType("C:File"), PathRootType::None); + + EXPECT_EQ(FileSystem::GetPathRootType("/"), PathRootType::Unix); + EXPECT_EQ(FileSystem::GetPathRootType("/File"), PathRootType::Unix); + EXPECT_EQ(FileSystem::GetPathRootType("\\File"), PathRootType::Unix); + + EXPECT_EQ(FileSystem::GetPathRootType("C:/"), PathRootType::WindowsDrive); + EXPECT_EQ(FileSystem::GetPathRootType("C:/File"), PathRootType::WindowsDrive); + EXPECT_EQ(FileSystem::GetPathRootType("C:\\File"), PathRootType::WindowsDrive); + + EXPECT_EQ(FileSystem::GetPathRootType("//Server/Share"), PathRootType::WindowsUNC); + EXPECT_EQ(FileSystem::GetPathRootType("\\\\Server\\Share"), PathRootType::WindowsUNC); + EXPECT_EQ(FileSystem::GetPathRootType("/\\Server/Share"), PathRootType::WindowsUNC); + EXPECT_EQ(FileSystem::GetPathRootType("\\/Server\\Share"), PathRootType::WindowsUNC); +} + + TEST(Platforms_FileSystem, SimplifyPath) { EXPECT_STREQ(FileSystem::SimplifyPath("", '/').c_str(), ""); diff --git a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp index 91ec77ba6c..1211edc17b 100644 --- a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp +++ b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp @@ -136,6 +136,49 @@ TEST(ShaderPreprocessTest, Include) } } +TEST(ShaderPreprocessTest, ShaderSourceFactoryProbe) +{ + RefCntAutoPtr pDefaultFactory; + CreateDefaultShaderSourceStreamFactory("shaders/ShaderPreprocessor", &pDefaultFactory); + ASSERT_NE(pDefaultFactory, nullptr); + + EXPECT_TRUE(pDefaultFactory->CreateInputStream("IncludeBasicTest.hlsl", nullptr)); + EXPECT_FALSE(pDefaultFactory->CreateInputStream("Missing.hlsl", nullptr)); + EXPECT_TRUE(pDefaultFactory->CreateInputStream2("IncludeBasicTest.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + EXPECT_FALSE(pDefaultFactory->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + + auto pMemoryFactory0 = CreateMemoryShaderSourceFactory( + { + {"Memory0.hlsl", "// Memory0.hlsl\n"}, + }, + false); + auto pMemoryFactory1 = CreateMemoryShaderSourceFactory( + { + {"Memory1.hlsl", "// Memory1.hlsl\n"}, + }, + false); + ASSERT_NE(pMemoryFactory0, nullptr); + ASSERT_NE(pMemoryFactory1, nullptr); + + EXPECT_TRUE(pMemoryFactory0->CreateInputStream("Memory0.hlsl", nullptr)); + EXPECT_FALSE(pMemoryFactory0->CreateInputStream("Missing.hlsl", nullptr)); + EXPECT_TRUE(pMemoryFactory0->CreateInputStream2("Memory0.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + EXPECT_FALSE(pMemoryFactory0->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + + auto pCompoundFactory = CreateCompoundShaderSourceFactory({pMemoryFactory0.RawPtr(), pMemoryFactory1.RawPtr()}); + ASSERT_NE(pCompoundFactory, nullptr); + + EXPECT_TRUE(pCompoundFactory->CreateInputStream("Memory1.hlsl", nullptr)); + EXPECT_FALSE(pCompoundFactory->CreateInputStream("Missing.hlsl", nullptr)); + EXPECT_TRUE(pCompoundFactory->CreateInputStream2("Memory0.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + EXPECT_TRUE(pCompoundFactory->CreateInputStream2("Memory1.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + EXPECT_FALSE(pCompoundFactory->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + + RefCntAutoPtr pStream; + EXPECT_TRUE(pCompoundFactory->CreateInputStream2("Memory1.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, &pStream)); + EXPECT_NE(pStream, nullptr); +} + TEST(ShaderPreprocessTest, IncludeNestedParentRelative) { RefCntAutoPtr pShaderSourceFactory; From f1a2f7a31076185f29b7a65c55a21bac9ebbf9b3 Mon Sep 17 00:00:00 2001 From: assiduous Date: Sat, 18 Jul 2026 22:32:31 -0700 Subject: [PATCH 3/5] BasicFileSystem: add JoinPath helper Add JoinPath() to combine a base path with a relative path fragment, normalize boundary separators, and preserve Unix, drive, and UNC roots. --- Platforms/Basic/interface/BasicFileSystem.hpp | 6 +++ Platforms/Basic/src/BasicFileSystem.cpp | 42 +++++++++++++++++++ .../src/Platforms/FileSystemTest.cpp | 33 +++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/Platforms/Basic/interface/BasicFileSystem.hpp b/Platforms/Basic/interface/BasicFileSystem.hpp index 290e178fd8..342fd40f1c 100644 --- a/Platforms/Basic/interface/BasicFileSystem.hpp +++ b/Platforms/Basic/interface/BasicFileSystem.hpp @@ -27,6 +27,7 @@ #pragma once +#include #include #include "../../../Primitives/interface/BasicTypes.h" #include "../../../Primitives/interface/FlagEnum.h" @@ -196,6 +197,11 @@ struct BasicFileSystem /// Builds a path from the given components. static std::string BuildPathFromComponents(const std::vector& Components, Char Slash = 0); + /// Joins a base path and a relative path using the given slash symbol. + /// Redundant separators at the join boundary are removed, while a root separator in BasePath is preserved. + /// RelativePath is treated as a path fragment, so its leading separators do not make the result absolute. + static std::string JoinPath(std::string_view BasePath, std::string_view RelativePath, Char Slash = 0); + /// Simplifies the path. /// The function performs the following path simplifications: diff --git a/Platforms/Basic/src/BasicFileSystem.cpp b/Platforms/Basic/src/BasicFileSystem.cpp index 39a226f777..0a79d1b951 100644 --- a/Platforms/Basic/src/BasicFileSystem.cpp +++ b/Platforms/Basic/src/BasicFileSystem.cpp @@ -242,6 +242,48 @@ std::string BasicFileSystem::BuildPathFromComponents(const std::vector& return Path; } +std::string BasicFileSystem::JoinPath(std::string_view BasePath, std::string_view RelativePath, Char Slash) +{ + if (Slash != 0) + DEV_CHECK_ERR(IsSlash(Slash), "Incorrect slash symbol"); + else + Slash = SlashSymbol; + + size_t RootLength = 0; + if (BasePath.length() >= 2 && IsSlash(BasePath[0]) && IsSlash(BasePath[1])) + RootLength = 2; + else if (!BasePath.empty() && IsSlash(BasePath[0])) + RootLength = 1; + else if (BasePath.length() >= 3 && BasePath[1] == ':' && IsSlash(BasePath[2])) + RootLength = 3; + + size_t BaseEnd = BasePath.length(); + while (BaseEnd > RootLength && IsSlash(BasePath[BaseEnd - 1])) + --BaseEnd; + + size_t RelativeStart = 0; + while (RelativeStart < RelativePath.length() && IsSlash(RelativePath[RelativeStart])) + ++RelativeStart; + + std::string Path; + Path.reserve(BaseEnd + (BaseEnd != 0 && RelativeStart < RelativePath.length() ? 1 : 0) + RelativePath.length() - RelativeStart); + if (BaseEnd != 0) + Path.append(BasePath.data(), BaseEnd); + + // A path consisting only of root separators keeps its root, but uses the requested slash. + for (size_t i = Path.length(); i > 0 && IsSlash(Path[i - 1]); --i) + Path[i - 1] = Slash; + + if (RelativeStart < RelativePath.length()) + { + if (!Path.empty() && !IsSlash(Path.back())) + Path.push_back(Slash); + Path.append(RelativePath.data() + RelativeStart, RelativePath.length() - RelativeStart); + } + + return Path; +} + std::string BasicFileSystem::SimplifyPath(const Char* Path, Char Slash) { if (Path == nullptr || Path[0] == '\0') diff --git a/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp b/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp index b4e5ac3d65..dc4f664edb 100644 --- a/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp +++ b/Tests/DiligentCoreTest/src/Platforms/FileSystemTest.cpp @@ -459,6 +459,39 @@ TEST(Platforms_FileSystem, BuildPathFromComponents) EXPECT_STREQ(FileSystem::BuildPathFromComponents({{"a"}, {"b"}, {"c"}}, '/').c_str(), "a/b/c"); } +TEST(Platforms_FileSystem, JoinPath) +{ + EXPECT_EQ(FileSystem::JoinPath("", "", '/'), ""); + EXPECT_EQ(FileSystem::JoinPath("", "a", '/'), "a"); + EXPECT_EQ(FileSystem::JoinPath("", "/a", '/'), "a"); + EXPECT_EQ(FileSystem::JoinPath("a", "", '/'), "a"); + EXPECT_EQ(FileSystem::JoinPath("a/", "", '/'), "a"); + + EXPECT_EQ(FileSystem::JoinPath("a", "b", '/'), "a/b"); + EXPECT_EQ(FileSystem::JoinPath("a/", "/b", '/'), "a/b"); + EXPECT_EQ(FileSystem::JoinPath("a\\", "\\b", '/'), "a/b"); + EXPECT_EQ(FileSystem::JoinPath("a/", "/b", '\\'), "a\\b"); + EXPECT_EQ(FileSystem::JoinPath("a", "b"), std::string{"a"} + FileSystem::SlashSymbol + "b"); + + EXPECT_EQ(FileSystem::JoinPath("/", "a", '/'), "/a"); + EXPECT_EQ(FileSystem::JoinPath("\\", "a", '/'), "/a"); + EXPECT_EQ(FileSystem::JoinPath("//", "Server", '/'), "//Server"); + EXPECT_EQ(FileSystem::JoinPath("C:/", "/File", '/'), "C:/File"); + EXPECT_EQ(FileSystem::JoinPath("C:\\", "\\File", '\\'), "C:\\File"); + EXPECT_EQ(FileSystem::JoinPath("\\\\Server\\Share\\", "\\File", '\\'), "\\\\Server\\Share\\File"); + EXPECT_EQ(FileSystem::JoinPath("/", "", '\\'), "\\"); + EXPECT_EQ(FileSystem::JoinPath("//", "", '\\'), "\\\\"); + EXPECT_EQ(FileSystem::JoinPath("C:/", "", '\\'), "C:\\"); + + EXPECT_EQ(FileSystem::JoinPath("a\\b", "c/d", '/'), "a\\b/c/d"); + EXPECT_EQ(FileSystem::JoinPath("a/.", "../b", '/'), "a/./../b"); + + const std::string Paths = "[Base/][Relative]"; + EXPECT_EQ(FileSystem::JoinPath(std::string_view{Paths}.substr(1, 5), + std::string_view{Paths}.substr(8, 8), '/'), + "Base/Relative"); +} + TEST(Platforms_FileSystem, GetRelativePath) { EXPECT_STREQ(FileSystem::GetRelativePath("", true, "", true).c_str(), ""); From 6aa1b0e52453f5bfdf94cef4a0e5efff139f26c1 Mon Sep 17 00:00:00 2001 From: assiduous Date: Sun, 19 Jul 2026 09:05:04 -0700 Subject: [PATCH 4/5] ShaderTools: normalize shader source paths Normalize shader source paths across filesystem, memory, and compound stream factories. Resolve local includes relative to the including source while preserving Unix, drive, and UNC roots. --- Graphics/GraphicsEngine/CMakeLists.txt | 1 + .../include/ShaderSourcePath.hpp | 57 ++++++ Graphics/GraphicsEngine/interface/Shader.h | 5 +- .../src/DefaultShaderSourceStreamFactory.cpp | 19 +- .../src/ShaderSourceFactoryUtils.cpp | 23 ++- .../ShaderTools/src/ShaderToolsCommon.cpp | 137 +++++++------ ReleaseHistory.md | 2 +- .../src/ShaderTools/ShaderPreprocessTest.cpp | 192 ++++++++++++++++++ 8 files changed, 354 insertions(+), 82 deletions(-) create mode 100644 Graphics/GraphicsEngine/include/ShaderSourcePath.hpp diff --git a/Graphics/GraphicsEngine/CMakeLists.txt b/Graphics/GraphicsEngine/CMakeLists.txt index a75b4fc65a..67b8f30b9e 100644 --- a/Graphics/GraphicsEngine/CMakeLists.txt +++ b/Graphics/GraphicsEngine/CMakeLists.txt @@ -29,6 +29,7 @@ set(INCLUDE include/ResourceMappingImpl.hpp include/SamplerBase.hpp include/ShaderBase.hpp + include/ShaderSourcePath.hpp include/ShaderResourceBindingBase.hpp include/ShaderResourceCacheCommon.hpp include/ShaderResourceVariableBase.hpp diff --git a/Graphics/GraphicsEngine/include/ShaderSourcePath.hpp b/Graphics/GraphicsEngine/include/ShaderSourcePath.hpp new file mode 100644 index 0000000000..15400c144c --- /dev/null +++ b/Graphics/GraphicsEngine/include/ShaderSourcePath.hpp @@ -0,0 +1,57 @@ +/* + * Copyright 2026 Diligent Graphics LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * In no event and under no legal theory, whether in tort (including negligence), + * contract, or otherwise, unless required by applicable law (such as deliberate + * and grossly negligent acts) or agreed to in writing, shall any Contributor be + * liable for any damages, including any direct, indirect, special, incidental, + * or consequential damages of any character arising as a result of this License or + * out of the use or inability to use the software (including but not limited to damages + * for loss of goodwill, work stoppage, computer failure or malfunction, or any and + * all other commercial damages or losses), even if such Contributor has been advised + * of the possibility of such damages. + */ + +#pragma once + +#include "BasicFileSystem.hpp" + +namespace Diligent +{ + +inline String NormalizeShaderSourcePath(const Char* Path, Char Slash = BasicFileSystem::UnixSlash) +{ + if (Path == nullptr || Path[0] == '\0') + return {}; + + if (Slash == 0) + Slash = BasicFileSystem::SlashSymbol; + + // Simplify drive and UNC paths using Windows semantics to preserve their roots. + // By default, shader source names use '/' as the canonical separator. + const PathRootType RootType = BasicFileSystem::GetPathRootType(Path); + + const Char SimplifySlash = + RootType == PathRootType::WindowsDrive || RootType == PathRootType::WindowsUNC ? + BasicFileSystem::WinSlash : + BasicFileSystem::UnixSlash; + + String NormalizedPath = BasicFileSystem::SimplifyPath(Path, SimplifySlash); + if (SimplifySlash != Slash) + BasicFileSystem::CorrectSlashes(NormalizedPath, Slash); + return NormalizedPath; +} + +} // namespace Diligent diff --git a/Graphics/GraphicsEngine/interface/Shader.h b/Graphics/GraphicsEngine/interface/Shader.h index 7c430bc3e3..ab4f9aa917 100644 --- a/Graphics/GraphicsEngine/interface/Shader.h +++ b/Graphics/GraphicsEngine/interface/Shader.h @@ -234,7 +234,10 @@ static DILIGENT_CONSTEXPR INTERFACE_ID IID_IShaderSourceInputStreamFactory = IObjectInclusiveMethods; \ IShaderSourceInputStreamFactoryMethods ShaderSourceInputStreamFactory -/// Shader source stream factory interface +/// Shader source stream factory interface. +/// +/// File names may use either '/' or '\\' as a path separator. Implementations must +/// treat both separators as equivalent. DILIGENT_BEGIN_INTERFACE(IShaderSourceInputStreamFactory, IObject) { /// Creates a shader source input stream for the specified file name. diff --git a/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp b/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp index ee72d952ea..88296418ec 100644 --- a/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp +++ b/Graphics/GraphicsEngine/src/DefaultShaderSourceStreamFactory.cpp @@ -31,6 +31,7 @@ #include "RefCntAutoPtr.hpp" #include "EngineMemory.h" #include "BasicFileStream.hpp" +#include "ShaderSourcePath.hpp" namespace Diligent { @@ -78,12 +79,12 @@ Bool DefaultShaderSourceStreamFactory::CreateInputStream2(const Char* CREATE_SHADER_SOURCE_INPUT_STREAM_FLAGS Flags, IFileStream** ppStream) { - auto TryCreateFileStream = [](const char* Path, IFileStream** ppStream) // + auto TryCreateFileStream = [](const std::string& Path, IFileStream** ppStream) // { - Bool SourceFound = FileSystem::FileExists(Path); + Bool SourceFound = FileSystem::FileExists(Path.c_str()); if (SourceFound && ppStream != nullptr) { - RefCntAutoPtr pFileStream = BasicFileStream::Create(Path, EFileAccessMode::Read); + RefCntAutoPtr pFileStream = BasicFileStream::Create(Path.c_str(), EFileAccessMode::Read); SourceFound = pFileStream->IsValid(); if (SourceFound) @@ -97,18 +98,20 @@ Bool DefaultShaderSourceStreamFactory::CreateInputStream2(const Char* DEV_CHECK_ERR(ppStream == nullptr || *ppStream == nullptr, "Output stream pointer must be null. Overwriting a non-null output pointer may result in memory leaks."); + const String FileName = NormalizeShaderSourcePath(Name, FileSystem::SlashSymbol); + Bool SourceFound = False; - if (FileSystem::IsPathAbsolute(Name)) + if (FileSystem::IsPathAbsolute(FileName.c_str())) { - SourceFound = TryCreateFileStream(Name, ppStream); + SourceFound = TryCreateFileStream(FileName, ppStream); } - else + else if (!FileName.empty()) { for (const std::string& SearchDir : m_SearchDirectories) { - const std::string FullPath = SearchDir + ((Name[0] == '\\' || Name[0] == '/') ? Name + 1 : Name); + const std::string FullPath = FileSystem::JoinPath(SearchDir, FileName); - SourceFound = TryCreateFileStream(FullPath.c_str(), ppStream); + SourceFound = TryCreateFileStream(FullPath, ppStream); if (SourceFound) break; } diff --git a/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp b/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp index 04dd1c3d8e..fdbded98b5 100644 --- a/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp +++ b/Graphics/GraphicsTools/src/ShaderSourceFactoryUtils.cpp @@ -29,12 +29,14 @@ #include #include #include +#include #include "ObjectBase.hpp" #include "HashUtils.hpp" #include "RefCntAutoPtr.hpp" #include "StringDataBlobImpl.hpp" #include "MemoryFileStream.hpp" +#include "ShaderSourcePath.hpp" namespace Diligent { @@ -66,7 +68,9 @@ class CompoundShaderSourceFactory : public ObjectBasesecond.c_str(); + SourceName = it->second.c_str(); } Bool SourceFound = False; for (size_t i = 0; i < m_pFactories.size() && !SourceFound; ++i) { if (m_pFactories[i]) - SourceFound = m_pFactories[i]->CreateInputStream2(Name, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, ppStream); + SourceFound = m_pFactories[i]->CreateInputStream2(SourceName, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT, ppStream); } if (!SourceFound && ppStream != nullptr && (Flags & CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT) == 0) { - LOG_ERROR_MESSAGE("Failed to create input stream for source file ", Name); + LOG_ERROR_MESSAGE("Failed to create input stream for source file ", SourceName); } return SourceFound; @@ -157,7 +163,7 @@ class MemoryShaderSourceFactory final : public ObjectBase{std::move(FileInfo), Error}; } -static Char GetFirstSlash(const char* Path) -{ - if (Path == nullptr) - return 0; - - for (const char* c = Path; *c != '\0'; ++c) - { - if (BasicFileSystem::IsSlash(*c)) - return *c; - } - - return 0; -} - -static Char GetPreferredIncludePathSlash(const ShaderCreateInfo& ShaderCI, const std::string& IncludeName) -{ - if (const Char IncludeSlash = GetFirstSlash(IncludeName.c_str())) - return IncludeSlash; - - if (const Char SourceSlash = GetFirstSlash(ShaderCI.FilePath)) - return SourceSlash; - - return BasicFileSystem::SlashSymbol; -} - -static std::string MakeParentRelativeIncludePath(const String& ParentDir, const std::string& IncludeName, Char Slash) -{ - return BasicFileSystem::SimplifyPath( - (ParentDir + Slash + IncludeName).c_str(), - Slash); -} - -static bool ShaderSourceExists(IShaderSourceInputStreamFactory* pFactory, const std::string& FilePath) -{ - return pFactory->CreateInputStream(FilePath.c_str(), nullptr); -} - +// Resolves a shader include path relative to the including source when applicable. static std::string ResolveIncludePathForPreprocess(const ShaderCreateInfo& ShaderCI, const std::string& IncludeName, bool IsLocalInclude) { - if (!IsLocalInclude || ShaderCI.FilePath == nullptr || BasicFileSystem::IsPathAbsolute(IncludeName.c_str())) - return IncludeName; + const std::string NormalizedIncludeName = NormalizeShaderSourcePath(IncludeName.c_str()); + // Examples: + // -> Common.hlsl + // "/Shaders/Common.hlsl" (absolute) -> /Shaders/Common.hlsl + if (!IsLocalInclude || ShaderCI.FilePath == nullptr || BasicFileSystem::GetPathRootType(NormalizedIncludeName.c_str()) != PathRootType::None) + return NormalizedIncludeName; + + const std::string SourcePath = NormalizeShaderSourcePath(ShaderCI.FilePath); String ParentDir; - BasicFileSystem::GetPathComponents(ShaderCI.FilePath, &ParentDir, nullptr); + BasicFileSystem::GetPathComponents(SourcePath, &ParentDir, nullptr); + // GetPathComponents() returns an empty directory for a file in the root + // (e.g. "/Main.hlsl"). Restore the root so local includes remain absolute. + if (ParentDir.empty() && !SourcePath.empty() && SourcePath.front() == BasicFileSystem::UnixSlash) + ParentDir.push_back(BasicFileSystem::UnixSlash); + // "Main.hlsl" + "Common.hlsl" -> "Common.hlsl" if (ParentDir.empty()) - return IncludeName; - - const Char PreferredSlash = GetPreferredIncludePathSlash(ShaderCI, IncludeName); - const std::string RelativePath = MakeParentRelativeIncludePath(ParentDir, IncludeName, PreferredSlash); - - if (ShaderCI.pShaderSourceStreamFactory != nullptr) - { - if (ShaderSourceExists(ShaderCI.pShaderSourceStreamFactory, RelativePath)) - return RelativePath; - - const Char AlternateSlash = PreferredSlash == BasicFileSystem::UnixSlash ? BasicFileSystem::WinSlash : BasicFileSystem::UnixSlash; - const std::string AlternatePath = MakeParentRelativeIncludePath(ParentDir, IncludeName, AlternateSlash); - if (AlternatePath != RelativePath && ShaderSourceExists(ShaderCI.pShaderSourceStreamFactory, AlternatePath)) - return AlternatePath; - - return IncludeName; - } - - return RelativePath; + return NormalizedIncludeName; + + // "Shaders/Nested/Main.hlsl" + "../Common.hlsl" + // -> "Shaders/Common.hlsl" + const std::string ParentRelativePath = BasicFileSystem::JoinPath(ParentDir, NormalizedIncludeName, BasicFileSystem::UnixSlash); + const std::string RelativePath = NormalizeShaderSourcePath(ParentRelativePath.c_str()); + // With no factory, return "Shaders/Common.hlsl" without probing it. + if (ShaderCI.pShaderSourceStreamFactory == nullptr) + return RelativePath; + + // Return "Shaders/Common.hlsl" when it exists; otherwise return + // "Common.hlsl" so the factory can use its search paths. + return ShaderCI.pShaderSourceStreamFactory->CreateInputStream(RelativePath.c_str(), nullptr) ? + RelativePath : + NormalizedIncludeName; } template @@ -525,8 +530,11 @@ bool ProcessShaderIncludes(const ShaderCreateInfo& ShaderCI, std::function Includes; - ProcessShaderIncludesImpl(ShaderCI, Includes, IncludeHandler); + if (const Char* FilePath = NormalizedShaderCI.GetFilePath()) + Includes.emplace(FilePath); + ProcessShaderIncludesImpl(NormalizedShaderCI, Includes, IncludeHandler); return true; } catch (const std::pair& ErrInfo) @@ -583,13 +591,14 @@ static std::string UnrollShaderIncludesImpl(ShaderCreateInfo ShaderCI, std::unor std::string UnrollShaderIncludes(const ShaderCreateInfo& ShaderCI) noexcept(false) { + NormalizedShaderCreateInfo NormalizedShaderCI{ShaderCI}; std::unordered_set Includes; - if (ShaderCI.FilePath != nullptr) - Includes.emplace(ShaderCI.FilePath); + if (const Char* FilePath = NormalizedShaderCI.GetFilePath()) + Includes.emplace(FilePath); try { - return UnrollShaderIncludesImpl(ShaderCI, Includes); + return UnrollShaderIncludesImpl(NormalizedShaderCI, Includes); } catch (const std::pair& ErrInfo) { diff --git a/ReleaseHistory.md b/ReleaseHistory.md index 2e44f3aa74..60c21283ae 100644 --- a/ReleaseHistory.md +++ b/ReleaseHistory.md @@ -2,7 +2,7 @@ ## Current progress -* Added success results and probe mode to `IShaderSourceInputStreamFactory::CreateInputStream()` and `CreateInputStream2()` (API256020) +* Added success results, probe mode, and path separator normalization to `IShaderSourceInputStreamFactory::CreateInputStream()` and `CreateInputStream2()` (API256020) * Added `SHADER_OPTIMIZATION_LEVEL` enum and `ShaderCreateInfo::ShaderOptimizationLevel` member (API256019) * Added `ShaderFloat64` and `ShaderBarycentrics` members to `DeviceFeatures` struct (API256018) * Added `DrawMeshIndirectAttribs::pMtlAttribs` member (API256017) diff --git a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp index 1211edc17b..105bd922f3 100644 --- a/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp +++ b/Tests/DiligentCoreTest/src/ShaderTools/ShaderPreprocessTest.cpp @@ -27,6 +27,7 @@ #include #include "ShaderToolsCommon.hpp" +#include "ShaderSourcePath.hpp" #include "DefaultShaderSourceStreamFactory.h" #include "RenderDevice.h" #include "ShaderSourceFactoryUtils.hpp" @@ -40,6 +41,43 @@ using namespace Diligent::Testing; namespace { +TEST(ShaderPreprocessTest, NormalizeShaderSourcePath) +{ + const struct + { + const Char* Path; + const Char* ExpectedPath; + } TestCases[] = { + {nullptr, ""}, + {"", ""}, + {"Shader.hlsl", "Shader.hlsl"}, + {"./Shader.hlsl", "Shader.hlsl"}, + {"Directory//./Subdir/../Shader.hlsl", "Directory/Shader.hlsl"}, + {"Directory\\Subdir\\Shader.hlsl", "Directory/Subdir/Shader.hlsl"}, + {"/Directory\\Subdir/../Shader.hlsl", "/Directory/Shader.hlsl"}, + {"C:\\Directory\\Subdir\\..\\Shader.hlsl", "C:/Directory/Shader.hlsl"}, + {"C:/../Shader.hlsl", "C:/Shader.hlsl"}, + {"\\\\Server\\Share\\Directory\\..\\Shader.hlsl", "//Server/Share/Shader.hlsl"}, + {"//Server\\Share/Directory/./Shader.hlsl", "//Server/Share/Directory/Shader.hlsl"}, + }; + + for (const auto& TestCase : TestCases) + { + EXPECT_EQ(NormalizeShaderSourcePath(TestCase.Path), TestCase.ExpectedPath) + << "Path: " << (TestCase.Path != nullptr ? TestCase.Path : ""); + } + + EXPECT_EQ(NormalizeShaderSourcePath("Directory/Subdir/../Shader.hlsl", BasicFileSystem::WinSlash), "Directory\\Shader.hlsl"); + EXPECT_EQ(NormalizeShaderSourcePath("/Directory/Subdir/../Shader.hlsl", BasicFileSystem::WinSlash), "\\Directory\\Shader.hlsl"); + EXPECT_EQ(NormalizeShaderSourcePath("C:/Directory/Subdir/../Shader.hlsl", BasicFileSystem::WinSlash), "C:\\Directory\\Shader.hlsl"); + EXPECT_EQ(NormalizeShaderSourcePath("//Server/Share/Directory/../Shader.hlsl", BasicFileSystem::WinSlash), "\\\\Server\\Share\\Shader.hlsl"); + + String PlatformPath = "Directory"; + PlatformPath += BasicFileSystem::SlashSymbol; + PlatformPath += "Shader.hlsl"; + EXPECT_EQ(NormalizeShaderSourcePath("Directory/Subdir/../Shader.hlsl", 0), PlatformPath); +} + TEST(ShaderPreprocessTest, Include) { RefCntAutoPtr pShaderSourceFactory; @@ -144,23 +182,32 @@ TEST(ShaderPreprocessTest, ShaderSourceFactoryProbe) EXPECT_TRUE(pDefaultFactory->CreateInputStream("IncludeBasicTest.hlsl", nullptr)); EXPECT_FALSE(pDefaultFactory->CreateInputStream("Missing.hlsl", nullptr)); + EXPECT_TRUE(pDefaultFactory->CreateInputStream("Nested\\Config.hlsl", nullptr)); EXPECT_TRUE(pDefaultFactory->CreateInputStream2("IncludeBasicTest.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); EXPECT_FALSE(pDefaultFactory->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); auto pMemoryFactory0 = CreateMemoryShaderSourceFactory( { {"Memory0.hlsl", "// Memory0.hlsl\n"}, + {"Directory/Memory0.hlsl", "// Directory/Memory0.hlsl\n"}, + {"C:\\Directory\\DriveMemory.hlsl", "// DriveMemory.hlsl\n"}, + {"\\\\server\\share\\NetworkMemory.hlsl", "// NetworkMemory.hlsl\n"}, }, false); auto pMemoryFactory1 = CreateMemoryShaderSourceFactory( { {"Memory1.hlsl", "// Memory1.hlsl\n"}, + {"Directory\\Memory1.hlsl", "// Directory/Memory1.hlsl\n"}, }, false); ASSERT_NE(pMemoryFactory0, nullptr); ASSERT_NE(pMemoryFactory1, nullptr); EXPECT_TRUE(pMemoryFactory0->CreateInputStream("Memory0.hlsl", nullptr)); + EXPECT_TRUE(pMemoryFactory0->CreateInputStream("Directory\\Memory0.hlsl", nullptr)); + EXPECT_TRUE(pMemoryFactory1->CreateInputStream("Directory/Memory1.hlsl", nullptr)); + EXPECT_TRUE(pMemoryFactory0->CreateInputStream("C:/Directory/DriveMemory.hlsl", nullptr)); + EXPECT_TRUE(pMemoryFactory0->CreateInputStream("//server/share/NetworkMemory.hlsl", nullptr)); EXPECT_FALSE(pMemoryFactory0->CreateInputStream("Missing.hlsl", nullptr)); EXPECT_TRUE(pMemoryFactory0->CreateInputStream2("Memory0.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); EXPECT_FALSE(pMemoryFactory0->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); @@ -174,6 +221,12 @@ TEST(ShaderPreprocessTest, ShaderSourceFactoryProbe) EXPECT_TRUE(pCompoundFactory->CreateInputStream2("Memory1.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); EXPECT_FALSE(pCompoundFactory->CreateInputStream2("Missing.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, nullptr)); + auto pSubstituteFactory = CreateCompoundShaderSourceFactory( + {pMemoryFactory0.RawPtr(), pMemoryFactory1.RawPtr()}, + {{"Alias\\Memory.hlsl", "Directory/Memory1.hlsl"}}); + ASSERT_NE(pSubstituteFactory, nullptr); + EXPECT_TRUE(pSubstituteFactory->CreateInputStream("Alias/Memory.hlsl", nullptr)); + RefCntAutoPtr pStream; EXPECT_TRUE(pCompoundFactory->CreateInputStream2("Memory1.hlsl", CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE, &pStream)); EXPECT_NE(pStream, nullptr); @@ -277,6 +330,145 @@ TEST(ShaderPreprocessTest, IncludeNestedLocalAndSystemFromMemory) ASSERT_EQ(RefString, UnrolledStr); } +TEST(ShaderPreprocessTest, IncludeCanonicalPathsFromMemory) +{ + auto pShaderSourceFactory = CreateMemoryShaderSourceFactory( + { + {"/pkg/Main.hlsl", + "// Start Main.hlsl\n" + "#include \"Nested\\Types.hlsl\"\n" + "// End Main.hlsl\n"}, + {"/pkg\\Nested/Types.hlsl", + "// Start Types.hlsl\n" + "#include \"Config.hlsl\"\n" + "// End Types.hlsl\n"}, + {"/pkg/Nested\\Config.hlsl", + "// Start Correct Config.hlsl\n" + "#define CONFIG_SOURCE 2\n" + "// End Correct Config.hlsl\n"}, + {"pkg/Nested/Config.hlsl", + "// Start Decoy Config.hlsl\n" + "#define CONFIG_SOURCE 1\n" + "// End Decoy Config.hlsl\n"}, + {"/Main.hlsl", + "// Start Root Main.hlsl\n" + "#include \"Config.hlsl\"\n" + "// End Root Main.hlsl\n"}, + {"/Config.hlsl", + "// Start Root Config.hlsl\n" + "#define ROOT_CONFIG_SOURCE 2\n" + "// End Root Config.hlsl\n"}, + {"Config.hlsl", + "// Start Decoy Root Config.hlsl\n" + "#define ROOT_CONFIG_SOURCE 1\n" + "// End Decoy Root Config.hlsl\n"}, + {"C:\\pkg\\Main.hlsl", + "// Start Drive Main.hlsl\n" + "#include \"../Config.hlsl\"\n" + "// End Drive Main.hlsl\n"}, + {"C:/Config.hlsl", + "// Start Drive Config.hlsl\n" + "#define DRIVE_CONFIG_SOURCE 2\n" + "// End Drive Config.hlsl\n"}, + }, + false); + ASSERT_NE(pShaderSourceFactory, nullptr); + + { + std::deque Includes{ + "/pkg/Nested/Config.hlsl", + "/pkg/Nested/Types.hlsl", + "/pkg/Main.hlsl"}; + + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "/pkg/Main.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + const auto Result = ProcessShaderIncludes(ShaderCI, [&](const ShaderIncludePreprocessInfo& ProcessInfo) { + ASSERT_FALSE(Includes.empty()); + EXPECT_EQ(ProcessInfo.FilePath, Includes.front()); + Includes.pop_front(); + }); + + EXPECT_EQ(Result, true); + EXPECT_TRUE(Includes.empty()); + + constexpr char RefString[] = + "// Start Main.hlsl\n" + "// Start Types.hlsl\n" + "// Start Correct Config.hlsl\n" + "#define CONFIG_SOURCE 2\n" + "// End Correct Config.hlsl\n" + "\n" + "// End Types.hlsl\n" + "\n" + "// End Main.hlsl\n"; + + EXPECT_EQ(UnrollShaderIncludes(ShaderCI), RefString); + } + + { + std::deque Includes{ + "/Config.hlsl", + "/Main.hlsl"}; + + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "/Main.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + const auto Result = ProcessShaderIncludes(ShaderCI, [&](const ShaderIncludePreprocessInfo& ProcessInfo) { + ASSERT_FALSE(Includes.empty()); + EXPECT_EQ(ProcessInfo.FilePath, Includes.front()); + Includes.pop_front(); + }); + + EXPECT_EQ(Result, true); + EXPECT_TRUE(Includes.empty()); + + constexpr char RefString[] = + "// Start Root Main.hlsl\n" + "// Start Root Config.hlsl\n" + "#define ROOT_CONFIG_SOURCE 2\n" + "// End Root Config.hlsl\n" + "\n" + "// End Root Main.hlsl\n"; + + EXPECT_EQ(UnrollShaderIncludes(ShaderCI), RefString); + } + + { + std::deque Includes{ + "C:/Config.hlsl", + "C:/pkg/Main.hlsl"}; + + ShaderCreateInfo ShaderCI{}; + ShaderCI.Desc.Name = "TestShader"; + ShaderCI.FilePath = "C:\\pkg\\Main.hlsl"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + + const auto Result = ProcessShaderIncludes(ShaderCI, [&](const ShaderIncludePreprocessInfo& ProcessInfo) { + ASSERT_FALSE(Includes.empty()); + EXPECT_EQ(ProcessInfo.FilePath, Includes.front()); + Includes.pop_front(); + }); + + EXPECT_EQ(Result, true); + EXPECT_TRUE(Includes.empty()); + + constexpr char RefString[] = + "// Start Drive Main.hlsl\n" + "// Start Drive Config.hlsl\n" + "#define DRIVE_CONFIG_SOURCE 2\n" + "// End Drive Config.hlsl\n" + "\n" + "// End Drive Main.hlsl\n"; + + EXPECT_EQ(UnrollShaderIncludes(ShaderCI), RefString); + } +} + TEST(ShaderPreprocessTest, InvalidInclude) { RefCntAutoPtr pShaderSourceFactory; From 671bce3fc151b515d65450d0ed75dc831fd8e0bb Mon Sep 17 00:00:00 2001 From: hzqst <113660872@qq.com> Date: Sun, 19 Jul 2026 09:48:16 -0700 Subject: [PATCH 5/5] GLSLangUtils: support parent-relative nested includes Pass source names to glslang and resolve local includes relative to the including file. Normalize resolved paths before querying shader source factories while preserving root lookup for system includes. --- .../GraphicsEngineVulkan/src/ShaderVkImpl.cpp | 1 + .../src/ShaderWebGPUImpl.cpp | 1 + Graphics/ShaderTools/include/GLSLangUtils.hpp | 1 + Graphics/ShaderTools/src/GLSLangUtils.cpp | 93 +++++++++++---- .../Headers/Config.glsl | 2 + .../Headers/Config.hlsli | 2 + .../Headers/Types.glsl | 6 + .../Headers/Types.hlsli | 6 + .../IncludeNestedParentRelative/Main.glsl | 11 ++ .../IncludeNestedParentRelative/Main.psh | 6 + .../ShaderTools/SPIRVShaderResourcesTest.cpp | 106 ++++++++++++++++++ 11 files changed, 215 insertions(+), 20 deletions(-) create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.glsl create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.hlsli create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.glsl create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.hlsli create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.glsl create mode 100644 Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.psh diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp index 6973a8df73..6c15097c1f 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp @@ -139,6 +139,7 @@ std::vector CompileShaderGLSLang(const ShaderCreateInfo& Shade GLSLangUtils::GLSLtoSPIRVAttribs Attribs; Attribs.ShaderType = ShaderCI.Desc.ShaderType; Attribs.ShaderSource = SourceData.Source; + Attribs.SourceName = ShaderCI.FilePath; Attribs.SourceCodeLen = static_cast(SourceData.SourceLength); Attribs.Version = GLSLangUtils::SpirvVersion::Vk100; Attribs.Macros = Macros; diff --git a/Graphics/GraphicsEngineWebGPU/src/ShaderWebGPUImpl.cpp b/Graphics/GraphicsEngineWebGPU/src/ShaderWebGPUImpl.cpp index bb756b685b..eca349ba04 100644 --- a/Graphics/GraphicsEngineWebGPU/src/ShaderWebGPUImpl.cpp +++ b/Graphics/GraphicsEngineWebGPU/src/ShaderWebGPUImpl.cpp @@ -140,6 +140,7 @@ std::vector CompileShaderToSPIRV(const ShaderCreateInfo& S GLSLangUtils::GLSLtoSPIRVAttribs Attribs; Attribs.ShaderType = ShaderCI.Desc.ShaderType; Attribs.ShaderSource = SourceData.Source; + Attribs.SourceName = ShaderCI.FilePath; Attribs.SourceCodeLen = static_cast(SourceData.SourceLength); Attribs.Version = GLSLangUtils::SpirvVersion::Vk100; Attribs.Macros = Macros; diff --git a/Graphics/ShaderTools/include/GLSLangUtils.hpp b/Graphics/ShaderTools/include/GLSLangUtils.hpp index 4f5afc00be..f23d7ae8ec 100644 --- a/Graphics/ShaderTools/include/GLSLangUtils.hpp +++ b/Graphics/ShaderTools/include/GLSLangUtils.hpp @@ -57,6 +57,7 @@ struct GLSLtoSPIRVAttribs { SHADER_TYPE ShaderType = SHADER_TYPE_UNKNOWN; const char* ShaderSource = nullptr; + const char* SourceName = nullptr; int SourceCodeLen = 0; ShaderMacroArray Macros; IShaderSourceInputStreamFactory* pShaderSourceStreamFactory = nullptr; diff --git a/Graphics/ShaderTools/src/GLSLangUtils.cpp b/Graphics/ShaderTools/src/GLSLangUtils.cpp index f52082ffd0..672c318a86 100644 --- a/Graphics/ShaderTools/src/GLSLangUtils.cpp +++ b/Graphics/ShaderTools/src/GLSLangUtils.cpp @@ -47,6 +47,8 @@ #include "DataBlobImpl.hpp" #include "RefCntAutoPtr.hpp" #include "ShaderToolsCommon.hpp" +#include "ShaderSourcePath.hpp" +#include "BasicFileSystem.hpp" #ifdef USE_SPIRV_TOOLS # include "SPIRVTools.hpp" # include "spirv-tools/libspirv.h" @@ -295,7 +297,6 @@ std::vector CompileShaderInternal(::glslang::TShader& Sh return spirv; } - class IncluderImpl : public ::glslang::TShader::Includer { public: @@ -308,27 +309,14 @@ class IncluderImpl : public ::glslang::TShader::Includer const char* /*includerName*/, size_t /*inclusionDepth*/) { - DEV_CHECK_ERR(m_pInputStreamFactory != nullptr, "The shader source contains #include directives, but no input stream factory was provided"); - RefCntAutoPtr pSourceStream; - m_pInputStreamFactory->CreateInputStream(headerName, &pSourceStream); - if (pSourceStream == nullptr) + IncludeResult* pInclude = ReadIncludeFile(headerName, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_NONE); + if (pInclude == nullptr) { LOG_ERROR("Failed to open shader include file '", headerName, "'. Check that the file exists"); return nullptr; } - RefCntAutoPtr pFileData = DataBlobImpl::Create(); - pSourceStream->ReadBlob(pFileData); - IncludeResult* pNewInclude = - new IncludeResult{ - headerName, - pFileData->GetConstDataPtr(), - pFileData->GetSize(), - nullptr}; - - m_IncludeRes.emplace(pNewInclude); - m_DataBlobs.emplace(pNewInclude, std::move(pFileData)); - return pNewInclude; + return pInclude; } // For the "local"-only aspect of a "" include. Should not search in the @@ -336,9 +324,39 @@ class IncluderImpl : public ::glslang::TShader::Includer // call includeSystem() to look in the "system" locations. virtual IncludeResult* includeLocal(const char* headerName, const char* includerName, - size_t inclusionDepth) + size_t /*inclusionDepth*/) { - return nullptr; + if (m_pInputStreamFactory == nullptr) + return nullptr; + + const String HeaderPath = NormalizeShaderSourcePath(headerName); + if (HeaderPath.empty()) + return nullptr; + + IncludeResult* pInclude = nullptr; + if (BasicFileSystem::GetPathRootType(HeaderPath.c_str()) != PathRootType::None) + { + pInclude = ReadIncludeFile(HeaderPath.c_str(), CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT); + } + else if (includerName != nullptr && *includerName != '\0') + { + const String IncluderPath = NormalizeShaderSourcePath(includerName); + + String ParentDir; + BasicFileSystem::GetPathComponents(IncluderPath, &ParentDir, nullptr); + // GetPathComponents() returns an empty directory for a file in the root + // (e.g. "/Main.glsl"). Restore the root so local includes remain absolute. + if (ParentDir.empty() && !IncluderPath.empty() && IncluderPath.front() == BasicFileSystem::UnixSlash) + ParentDir.push_back(BasicFileSystem::UnixSlash); + + if (!ParentDir.empty()) + { + const std::string ParentRelativePath = BasicFileSystem::JoinPath(ParentDir, HeaderPath, BasicFileSystem::UnixSlash); + const String LocalPath = NormalizeShaderSourcePath(ParentRelativePath.c_str()); + pInclude = ReadIncludeFile(LocalPath.c_str(), CREATE_SHADER_SOURCE_INPUT_STREAM_FLAG_SILENT); + } + } + return pInclude; } // Signals that the parser will no longer use the contents of the @@ -349,6 +367,33 @@ class IncluderImpl : public ::glslang::TShader::Includer } private: + IncludeResult* ReadIncludeFile(const char* IncludePath, CREATE_SHADER_SOURCE_INPUT_STREAM_FLAGS Flags) + { + DEV_CHECK_ERR(m_pInputStreamFactory != nullptr, "The shader source contains #include directives, but no input stream factory was provided"); + + IncludeResult* pNewInclude = nullptr; + const String NormalizedIncludePath = NormalizeShaderSourcePath(IncludePath); + if (!NormalizedIncludePath.empty()) + { + RefCntAutoPtr pSourceStream; + m_pInputStreamFactory->CreateInputStream2(NormalizedIncludePath.c_str(), Flags, &pSourceStream); + if (pSourceStream != nullptr) + { + RefCntAutoPtr pFileData = DataBlobImpl::Create(); + pSourceStream->ReadBlob(pFileData); + pNewInclude = new IncludeResult{ + NormalizedIncludePath, + pFileData->GetConstDataPtr(), + pFileData->GetSize(), + nullptr}; + + m_IncludeRes.emplace(pNewInclude); + m_DataBlobs.emplace(pNewInclude, std::move(pFileData)); + } + } + return pNewInclude; + } + IShaderSourceInputStreamFactory* const m_pInputStreamFactory; std::unordered_set> m_IncludeRes; std::unordered_map> m_DataBlobs; @@ -531,7 +576,15 @@ std::vector GLSLtoSPIRV(const GLSLtoSPIRVAttribs& Attribs) const char* ShaderStrings[] = {Attribs.ShaderSource}; int Lengths[] = {Attribs.SourceCodeLen}; - Shader.setStringsWithLengths(ShaderStrings, Lengths, 1); + const char* Names[] = {Attribs.SourceName}; + if (Attribs.SourceName != nullptr) + { + Shader.setStringsWithLengthsAndNames(ShaderStrings, Lengths, Names, 1); + } + else + { + Shader.setStringsWithLengths(ShaderStrings, Lengths, 1); + } std::string Preamble; if (Attribs.UseRowMajorMatrices) diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.glsl b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.glsl new file mode 100644 index 0000000000..e097a9a8e3 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.glsl @@ -0,0 +1,2 @@ +#define NESTED_INCLUDE_R 0.25 +#define NESTED_INCLUDE_G 0.75 diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.hlsli b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.hlsli new file mode 100644 index 0000000000..e097a9a8e3 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Config.hlsli @@ -0,0 +1,2 @@ +#define NESTED_INCLUDE_R 0.25 +#define NESTED_INCLUDE_G 0.75 diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.glsl b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.glsl new file mode 100644 index 0000000000..a3a33ea15d --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.glsl @@ -0,0 +1,6 @@ +#include "Config.glsl" + +vec4 GetNestedIncludeColor() +{ + return vec4(NESTED_INCLUDE_R, NESTED_INCLUDE_G, 0.0, 1.0); +} diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.hlsli b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.hlsli new file mode 100644 index 0000000000..0de0a08657 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Headers/Types.hlsli @@ -0,0 +1,6 @@ +#include "Config.hlsli" + +float4 GetNestedIncludeColor() +{ + return float4(NESTED_INCLUDE_R, NESTED_INCLUDE_G, 0.0, 1.0); +} diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.glsl b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.glsl new file mode 100644 index 0000000000..0abcb50c74 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.glsl @@ -0,0 +1,11 @@ +#version 450 +#extension GL_GOOGLE_include_directive : enable + +#include "IncludeNestedParentRelative/Headers/Types.glsl" + +layout(location = 0) out vec4 FragColor; + +void main() +{ + FragColor = GetNestedIncludeColor(); +} diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.psh new file mode 100644 index 0000000000..0f2eadba20 --- /dev/null +++ b/Tests/DiligentCoreTest/assets/shaders/SPIRV/IncludeNestedParentRelative/Main.psh @@ -0,0 +1,6 @@ +#include "IncludeNestedParentRelative/Headers/Types.hlsli" + +float4 main() : SV_Target +{ + return GetNestedIncludeColor(); +} diff --git a/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp b/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp index e675ba28c1..7796bdb970 100644 --- a/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp +++ b/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp @@ -28,6 +28,7 @@ #include "GLSLangUtils.hpp" #include "DXCompiler.hpp" #include "DefaultShaderSourceStreamFactory.h" +#include "ShaderSourceFactoryUtils.hpp" #include "RefCntAutoPtr.hpp" #include "EngineMemory.h" #include "BasicFileSystem.hpp" @@ -160,6 +161,7 @@ std::vector LoadSPIRVFromGLSL(const char* FilePath, SHADER_TYPE Sh GLSLangUtils::GLSLtoSPIRVAttribs Attribs; Attribs.ShaderType = ShaderType; Attribs.ShaderSource = ShaderSource.data(); + Attribs.SourceName = FilePath; Attribs.SourceCodeLen = static_cast(ShaderSourceSize); Attribs.pShaderSourceStreamFactory = pShaderSourceStreamFactory; Attribs.Version = Version; @@ -646,4 +648,108 @@ TEST_F(SPIRVShaderResourcesTest, SpecializationConstants_DXC) TestSpecializationConstants(SHADER_COMPILER_DXC); } +TEST_F(SPIRVShaderResourcesTest, NestedParentRelativeIncludes_HLSL_GLSLang) +{ + auto SPIRV = LoadSPIRVFromHLSL("IncludeNestedParentRelative/Main.psh", SHADER_TYPE_PIXEL, SHADER_COMPILER_GLSLANG); + EXPECT_FALSE(SPIRV.empty()); +} + +TEST_F(SPIRVShaderResourcesTest, NestedParentRelativeIncludes_GLSL_GLSLang) +{ + auto SPIRV = LoadSPIRVFromGLSL("IncludeNestedParentRelative/Main.glsl"); + EXPECT_FALSE(SPIRV.empty()); +} + +TEST_F(SPIRVShaderResourcesTest, NestedLocalAndSystemIncludesFromMemory_GLSL_GLSLang) +{ + constexpr char MainGLSL[] = + "#version 450\n" + "#extension GL_GOOGLE_include_directive : enable\n" + "\n" + "#include \"Nested/LocalTypes.glsl\"\n" + "#include \"Nested/SystemTypes.glsl\"\n" + "layout(location = 0) out vec4 OutColor;\n" + "void main()\n" + "{\n" + " OutColor = GetLocalColor() + GetSystemColor();\n" + "}\n"; + + auto pShaderSourceFactory = CreateMemoryShaderSourceFactory( + { + {"Nested/LocalTypes.glsl", + "#include \"Config.glsl\"\n" + "vec4 GetLocalColor()\n" + "{\n" + " return vec4(float(LOCAL_CONFIG_VALUE), 0.0, 0.0, 1.0);\n" + "}\n"}, + {"Nested/SystemTypes.glsl", + "#include \n" + "vec4 GetSystemColor()\n" + "{\n" + " return vec4(0.0, float(ROOT_CONFIG_VALUE), 0.0, 1.0);\n" + "}\n"}, + {"Nested/Config.glsl", + "#define LOCAL_CONFIG_VALUE 1\n"}, + {"Config.glsl", + "#define ROOT_CONFIG_VALUE 1\n"}, + }, + false); + ASSERT_NE(pShaderSourceFactory, nullptr); + + GLSLangUtils::GLSLtoSPIRVAttribs Attribs; + Attribs.ShaderType = SHADER_TYPE_PIXEL; + Attribs.ShaderSource = MainGLSL; + Attribs.SourceName = "Main.glsl"; + Attribs.SourceCodeLen = static_cast(sizeof(MainGLSL) - 1); + Attribs.pShaderSourceStreamFactory = pShaderSourceFactory; + Attribs.Version = GLSLangUtils::SpirvVersion::Vk100; + Attribs.AssignBindings = true; + + auto SPIRV = GLSLangUtils::GLSLtoSPIRV(Attribs); + EXPECT_FALSE(SPIRV.empty()); +} + +TEST_F(SPIRVShaderResourcesTest, NestedLocalAndSystemIncludesFromMemory_HLSL_GLSLang) +{ + auto pShaderSourceFactory = CreateMemoryShaderSourceFactory( + { + {"Main.hlsl", + "#include \"Nested/LocalTypes.hlsli\"\n" + "#include \"Nested/SystemTypes.hlsli\"\n" + "float4 main() : SV_Target\n" + "{\n" + " return GetLocalColor() + GetSystemColor();\n" + "}\n"}, + {"Nested/LocalTypes.hlsli", + "#include \"Config.hlsli\"\n" + "float4 GetLocalColor()\n" + "{\n" + " return float4(LOCAL_CONFIG_VALUE, 0.0, 0.0, 1.0);\n" + "}\n"}, + {"Nested/SystemTypes.hlsli", + "#include \n" + "float4 GetSystemColor()\n" + "{\n" + " return float4(0.0, ROOT_CONFIG_VALUE, 0.0, 1.0);\n" + "}\n"}, + {"Nested/Config.hlsli", + "#define LOCAL_CONFIG_VALUE 1.0\n"}, + {"Config.hlsli", + "#define ROOT_CONFIG_VALUE 1.0\n"}, + }, + false); + ASSERT_NE(pShaderSourceFactory, nullptr); + + ShaderCreateInfo ShaderCI; + ShaderCI.SourceLanguage = SHADER_SOURCE_LANGUAGE_HLSL; + ShaderCI.FilePath = "Main.hlsl"; + ShaderCI.Desc = {"SPIRV memory include test shader", SHADER_TYPE_PIXEL}; + ShaderCI.EntryPoint = "main"; + ShaderCI.pShaderSourceStreamFactory = pShaderSourceFactory; + ShaderCI.ShaderOptimizationLevel = SHADER_OPTIMIZATION_LEVEL_DISABLED; + + auto SPIRV = GLSLangUtils::HLSLtoSPIRV(ShaderCI, GLSLangUtils::SpirvVersion::Vk100, nullptr, nullptr); + EXPECT_FALSE(SPIRV.empty()); +} + } // namespace