From 90c5214344bc9ec0e7c5cc09e86dad82b8b7abc9 Mon Sep 17 00:00:00 2001 From: Greg Fischer Date: Fri, 28 Oct 2022 17:27:18 -0600 Subject: [PATCH] Improve ResourceLimits interface to be more forward compatible New interface allows users to generate ResourceLimits for interface so that additions to TBuiltInResource do not break the ABI. Users should use the glslang-default-resource-limits library and the Public/ResourceLimits.h header. Similar changes have been made to the C interface. Use Public/resource_limits_c.h. Fixes #2822 --- BUILD.bazel | 2 +- StandAlone/ResourceLimits.cpp | 14 +++++++++++--- StandAlone/StandAlone.cpp | 15 +++++++-------- StandAlone/resource_limits_c.cpp | 15 ++++++++++----- glslang/CInterface/glslang_c_interface.cpp | 2 +- glslang/CMakeLists.txt | 2 ++ {StandAlone => glslang/Public}/ResourceLimits.h | 10 +++++----- .../Public}/resource_limits_c.h | 3 +++ gtests/BuiltInResource.FromFile.cpp | 4 ++-- gtests/Config.FromFile.cpp | 4 ++-- gtests/TestFixture.h | 6 +++--- 11 files changed, 47 insertions(+), 30 deletions(-) rename {StandAlone => glslang/Public}/ResourceLimits.h (90%) rename {StandAlone => glslang/Public}/resource_limits_c.h (95%) diff --git a/BUILD.bazel b/BUILD.bazel index 12168fae1..45efbd353 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -210,7 +210,7 @@ cc_library( cc_library( name = "glslang-default-resource-limits", srcs = ["StandAlone/ResourceLimits.cpp"], - hdrs = ["StandAlone/ResourceLimits.h"], + hdrs = ["glslang/Public/ResourceLimits.h"], copts = COMMON_COPTS, linkstatic = 1, deps = [":glslang"], diff --git a/StandAlone/ResourceLimits.cpp b/StandAlone/ResourceLimits.cpp index 1a76bf62d..6e1a3d85c 100644 --- a/StandAlone/ResourceLimits.cpp +++ b/StandAlone/ResourceLimits.cpp @@ -37,9 +37,9 @@ #include #include -#include "ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" -namespace glslang { +TBuiltInResource Resources; const TBuiltInResource DefaultTBuiltInResource = { /* .MaxLights = */ 32, @@ -529,4 +529,12 @@ void DecodeResourceLimits(TBuiltInResource* resources, char* config) } } -} // end namespace glslang +TBuiltInResource* GetResources() +{ + return &Resources; +} + +const TBuiltInResource* GetDefaultResources() +{ + return &DefaultTBuiltInResource; +} diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index edde81e28..53a6b9c2f 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -41,7 +41,7 @@ #define _CRT_SECURE_NO_WARNINGS #endif -#include "ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" #include "Worklist.h" #include "DirStackFileIncluder.h" #include "./../glslang/Include/ShHandle.h" @@ -149,7 +149,6 @@ bool LinkFailed = false; // array of unique places to leave the shader names and infologs for the asynchronous compiles std::vector> WorkItems; -TBuiltInResource Resources; std::string ConfigFile; // @@ -158,11 +157,11 @@ std::string ConfigFile; void ProcessConfigFile() { if (ConfigFile.size() == 0) - Resources = glslang::DefaultTBuiltInResource; + *GetResources() = *GetDefaultResources(); #ifndef GLSLANG_WEB else { char* configString = ReadFileData(ConfigFile.c_str()); - glslang::DecodeResourceLimits(&Resources, configString); + DecodeResourceLimits(GetResources(), configString); FreeFileData(configString); } #endif @@ -1417,7 +1416,7 @@ void CompileAndLinkShaderUnits(std::vector compUnits) #ifndef GLSLANG_WEB if (Options & EOptionOutputPreprocessed) { std::string str; - if (shader->preprocess(&Resources, defaultVersion, ENoProfile, false, false, messages, &str, includer)) { + if (shader->preprocess(GetResources(), defaultVersion, ENoProfile, false, false, messages, &str, includer)) { PutsIfNonEmpty(str.c_str()); } else { CompileFailed = true; @@ -1428,7 +1427,7 @@ void CompileAndLinkShaderUnits(std::vector compUnits) } #endif - if (! shader->parse(&Resources, defaultVersion, false, messages, includer)) + if (! shader->parse(GetResources(), defaultVersion, false, messages, includer)) CompileFailed = true; program.addShader(shader); @@ -1612,7 +1611,7 @@ int singleMain() #ifndef GLSLANG_WEB if (Options & EOptionDumpConfig) { - printf("%s", glslang::GetDefaultTBuiltInResourceString().c_str()); + printf("%s", GetDefaultTBuiltInResourceString().c_str()); if (workList.empty()) return ESuccess; } @@ -1838,7 +1837,7 @@ void CompileFile(const char* fileName, ShHandle compiler) for (int i = 0; i < ((Options & EOptionMemoryLeakMode) ? 100 : 1); ++i) { for (int j = 0; j < ((Options & EOptionMemoryLeakMode) ? 100 : 1); ++j) { // ret = ShCompile(compiler, shaderStrings, NumShaderStrings, lengths, EShOptNone, &Resources, Options, (Options & EOptionDefaultDesktop) ? 110 : 100, false, messages); - ret = ShCompile(compiler, &shaderString, 1, nullptr, EShOptNone, &Resources, Options, (Options & EOptionDefaultDesktop) ? 110 : 100, false, messages); + ret = ShCompile(compiler, &shaderString, 1, nullptr, EShOptNone, GetResources(), Options, (Options & EOptionDefaultDesktop) ? 110 : 100, false, messages); // const char* multi[12] = { "# ve", "rsion", " 300 e", "s", "\n#err", // "or should be l", "ine 1", "string 5\n", "float glo", "bal", // ";\n#error should be line 2\n void main() {", "global = 2.3;}" }; diff --git a/StandAlone/resource_limits_c.cpp b/StandAlone/resource_limits_c.cpp index a1f681c7b..0eeac23a5 100644 --- a/StandAlone/resource_limits_c.cpp +++ b/StandAlone/resource_limits_c.cpp @@ -26,15 +26,20 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. **/ -#include "resource_limits_c.h" -#include "ResourceLimits.h" +#include "glslang/Public/resource_limits_c.h" +#include "glslang/Public/ResourceLimits.h" #include #include #include +glslang_resource_t* glslang_resource(void) +{ + return reinterpret_cast(GetResources()); +} + const glslang_resource_t* glslang_default_resource(void) { - return reinterpret_cast(&glslang::DefaultTBuiltInResource); + return reinterpret_cast(GetDefaultResources()); } #if defined(__clang__) || defined(__GNUC__) @@ -47,7 +52,7 @@ const glslang_resource_t* glslang_default_resource(void) const char* glslang_default_resource_string() { - std::string cpp_str = glslang::GetDefaultTBuiltInResourceString(); + std::string cpp_str = GetDefaultTBuiltInResourceString(); char* c_str = (char*)malloc(cpp_str.length() + 1); strcpy(c_str, cpp_str.c_str()); return c_str; @@ -61,5 +66,5 @@ const char* glslang_default_resource_string() void glslang_decode_resource_limits(glslang_resource_t* resources, char* config) { - glslang::DecodeResourceLimits(reinterpret_cast(resources), config); + DecodeResourceLimits(reinterpret_cast(resources), config); } diff --git a/glslang/CInterface/glslang_c_interface.cpp b/glslang/CInterface/glslang_c_interface.cpp index 1bb4000ac..1465ce17a 100644 --- a/glslang/CInterface/glslang_c_interface.cpp +++ b/glslang/CInterface/glslang_c_interface.cpp @@ -33,7 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "glslang/Include/glslang_c_interface.h" #include "StandAlone/DirStackFileIncluder.h" -#include "StandAlone/ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" #include "glslang/Include/ShHandle.h" #include "glslang/Include/ResourceLimits.h" diff --git a/glslang/CMakeLists.txt b/glslang/CMakeLists.txt index 72e82b482..a8b14911f 100644 --- a/glslang/CMakeLists.txt +++ b/glslang/CMakeLists.txt @@ -149,6 +149,8 @@ set(GLSLANG_SOURCES set(GLSLANG_HEADERS Public/ShaderLang.h + Public/ResourceLimits.h + Public/resource_limits_c.h Include/arrays.h Include/BaseTypes.h Include/Common.h diff --git a/StandAlone/ResourceLimits.h b/glslang/Public/ResourceLimits.h similarity index 90% rename from StandAlone/ResourceLimits.h rename to glslang/Public/ResourceLimits.h index 736248eb3..f70be8172 100644 --- a/StandAlone/ResourceLimits.h +++ b/glslang/Public/ResourceLimits.h @@ -37,14 +37,16 @@ #include -#include "../glslang/Include/ResourceLimits.h" +#include "../Include/ResourceLimits.h" -namespace glslang { +// Return pointer to user-writable Resource to pass through API in +// future-proof way. +extern TBuiltInResource* GetResources(); // These are the default resources for TBuiltInResources, used for both // - parsing this string for the case where the user didn't supply one, // - dumping out a template for user construction of a config file. -extern const TBuiltInResource DefaultTBuiltInResource; +extern const TBuiltInResource* GetDefaultResources(); // Returns the DefaultTBuiltInResource as a human-readable string. std::string GetDefaultTBuiltInResourceString(); @@ -52,6 +54,4 @@ std::string GetDefaultTBuiltInResourceString(); // Decodes the resource limits from |config| to |resources|. void DecodeResourceLimits(TBuiltInResource* resources, char* config); -} // end namespace glslang - #endif // _STAND_ALONE_RESOURCE_LIMITS_INCLUDED_ diff --git a/StandAlone/resource_limits_c.h b/glslang/Public/resource_limits_c.h similarity index 95% rename from StandAlone/resource_limits_c.h rename to glslang/Public/resource_limits_c.h index 108fd5e21..afe83e485 100644 --- a/StandAlone/resource_limits_c.h +++ b/glslang/Public/resource_limits_c.h @@ -35,6 +35,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. extern "C" { #endif +// Returns a struct that can be use to create custom resource values. +glslang_resource_t* glslang_resource(void); + // These are the default resources for TBuiltInResources, used for both // - parsing this string for the case where the user didn't supply one, // - dumping out a template for user construction of a config file. diff --git a/gtests/BuiltInResource.FromFile.cpp b/gtests/BuiltInResource.FromFile.cpp index da81fe98c..9a7c9b575 100644 --- a/gtests/BuiltInResource.FromFile.cpp +++ b/gtests/BuiltInResource.FromFile.cpp @@ -36,7 +36,7 @@ #include -#include "StandAlone/ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" #include "TestFixture.h" namespace glslangtest { @@ -49,7 +49,7 @@ TEST_F(DefaultResourceTest, FromFile) const std::string path = GlobalTestSettings.testRoot + "/baseResults/test.conf"; std::string expectedConfig; tryLoadFile(path, "expected resource limit", &expectedConfig); - const std::string realConfig = glslang::GetDefaultTBuiltInResourceString(); + const std::string realConfig = GetDefaultTBuiltInResourceString(); ASSERT_EQ(expectedConfig, realConfig); } diff --git a/gtests/Config.FromFile.cpp b/gtests/Config.FromFile.cpp index dd18c13a9..05107e787 100644 --- a/gtests/Config.FromFile.cpp +++ b/gtests/Config.FromFile.cpp @@ -32,7 +32,7 @@ // ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. -#include "StandAlone/ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" #include "TestFixture.h" namespace glslangtest { @@ -65,7 +65,7 @@ TEST_P(ConfigTest, FromFile) char* configChars = new char[len + 1]; memcpy(configChars, configContents.data(), len); configChars[len] = 0; - glslang::DecodeResourceLimits(&resources, configChars); + DecodeResourceLimits(&resources, configChars); delete[] configChars; } diff --git a/gtests/TestFixture.h b/gtests/TestFixture.h index d087d6ddc..2127cae8d 100644 --- a/gtests/TestFixture.h +++ b/gtests/TestFixture.h @@ -48,7 +48,7 @@ #include "SPIRV/disassemble.h" #include "SPIRV/doc.h" #include "SPIRV/SPVRemapper.h" -#include "StandAlone/ResourceLimits.h" +#include "glslang/Public/ResourceLimits.h" #include "glslang/Public/ShaderLang.h" #include "Initializer.h" @@ -199,7 +199,7 @@ public: shader->setStringsWithLengths(&shaderStrings, &shaderLengths, 1); if (!entryPointName.empty()) shader->setEntryPoint(entryPointName.c_str()); return shader->parse( - (resources ? resources : &glslang::DefaultTBuiltInResource), + (resources ? resources : GetDefaultResources()), defaultVersion, isForwardCompatible, controls); } @@ -640,7 +640,7 @@ public: std::string ppShader; glslang::TShader::ForbidIncluder includer; const bool success = shader.preprocess( - &glslang::DefaultTBuiltInResource, defaultVersion, defaultProfile, + GetDefaultResources(), defaultVersion, defaultProfile, forceVersionProfile, isForwardCompatible, (EShMessages)(EShMsgOnlyPreprocessor | EShMsgCascadingErrors), &ppShader, includer);