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 12168fae..45efbd35 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 1a76bf62..6e1a3d85 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 edde81e2..53a6b9c2 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 a1f681c7..0eeac23a 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 1bb4000a..1465ce17 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 72e82b48..a8b14911 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 736248eb..f70be817 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 108fd5e2..afe83e48 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 da81fe98..9a7c9b57 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 dd18c13a..05107e78 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 d087d6dd..2127cae8 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);