diff --git a/src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp b/src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp index 9d98f3e78..907952a99 100644 --- a/src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp +++ b/src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp @@ -25,6 +25,7 @@ #include "AnyConverter.h" +#include #include #include #include @@ -39,6 +40,9 @@ namespace Magnum { namespace ShaderTools { struct AnyConverter::State { Format inputFormat, outputFormat; Containers::String inputVersion, outputVersion; + + Containers::Array> definitions; + Containers::Array> definitionViews; }; AnyConverter::AnyConverter(PluginManager::Manager& manager): AbstractConverter{manager} {} @@ -48,9 +52,9 @@ AnyConverter::AnyConverter(PluginManager::AbstractManager& manager, const std::s AnyConverter::~AnyConverter() = default; ConverterFeatures AnyConverter::doFeatures() const { - /** @todo Preprocess, Optimize, DebugInfo, those also need checks that the - plugin actually supports them */ - return ConverterFeature::ValidateFile|ConverterFeature::ConvertFile; + /** @todo Optimize, DebugInfo, those also need checks that the plugin + actually supports them */ + return ConverterFeature::ValidateFile|ConverterFeature::ConvertFile|ConverterFeature::Preprocess; } void AnyConverter::doSetInputFormat(const Format format, const Containers::StringView version) { @@ -63,6 +67,28 @@ void AnyConverter::doSetOutputFormat(Format format, Containers::StringView versi _state->outputVersion = Containers::String::nullTerminatedGlobalView(version); } +void AnyConverter::doSetDefinitions(const Containers::ArrayView> definitions) { + /* We have to make a local copy, unfortunately, and then a view on that + local copy */ + _state->definitions = Containers::Array>{definitions.size()}; + _state->definitionViews = Containers::Array>{definitions.size()}; + for(std::size_t i = 0; i != definitions.size(); ++i) { + /* Avoid a copy if the input is a global string literal */ + _state->definitions[i] = { + Containers::String::nullTerminatedGlobalView(definitions[i].first), + Containers::String::nullTerminatedGlobalView(definitions[i].second) + }; + /* Preserve the distinction between empty defines ("") and undefines + (nullptr or default constructor) */ + _state->definitionViews[i] = { + _state->definitions[i].first, + definitions[i].second.data() ? + Containers::StringView{_state->definitions[i].second} : + Containers::StringView{} + }; + } +} + namespace { using namespace Containers::Literals; @@ -141,18 +167,30 @@ std::pair AnyConverter::doValidateFile(const Stage sta d << "(provided by" << metadata->name() << Debug::nospace << ")"; } - /* Instantiate the plugin, check that it can actually validate */ + /* Instantiate the plugin */ Containers::Pointer converter = static_cast*>(manager())->instantiate(plugin); + + /* Check that it can actually validate */ if(!(converter->features() & ConverterFeature::ValidateFile)) { Error{} << "ShaderTools::AnyConverter::validateFile():" << metadata->name() << "does not support validation"; return {}; } + /* Check that it can preprocess, in case we were asked to preprocess */ + if((!_state->definitionViews.empty() || (flags() & ConverterFlag::PreprocessOnly)) && !(converter->features() & ConverterFeature::Preprocess)) { + Error{} << "ShaderTools::AnyConverter::validateFile():" << metadata->name() << "does not support preprocessing"; + return {}; + } + /* Propagate input/output version and flags */ converter->setFlags(flags()); converter->setInputFormat(_state->inputFormat, _state->inputVersion); converter->setOutputFormat(_state->outputFormat, _state->outputVersion); + /* Propagate definitions, if any */ + if(!_state->definitionViews.empty()) + converter->setDefinitions(_state->definitionViews); + /* Try to validate the file (error output should be printed by the plugin itself) */ return converter->validateFile(stage, filename); @@ -185,18 +223,30 @@ bool AnyConverter::doConvertFileToFile(const Stage stage, const Containers::Stri d << "(provided by" << metadata->name() << Debug::nospace << ")"; } - /* Instantiate the plugin, check that it can actually validate */ + /* Instantiate the plugin */ Containers::Pointer converter = static_cast*>(manager())->instantiate(plugin); + + /* Check that it can actually convert */ if(!(converter->features() & ConverterFeature::ConvertFile)) { Error{} << "ShaderTools::AnyConverter::convertFileToFile():" << metadata->name() << "does not support conversion"; return {}; } + /* Check that it can preprocess, in case we were asked to preprocess */ + if((!_state->definitionViews.empty() || (flags() & ConverterFlag::PreprocessOnly)) && !(converter->features() & ConverterFeature::Preprocess)) { + Error{} << "ShaderTools::AnyConverter::convertFileToFile():" << metadata->name() << "does not support preprocessing"; + return {}; + } + /* Propagate input/output version and flags */ converter->setFlags(flags()); converter->setInputFormat(_state->inputFormat, _state->inputVersion); converter->setOutputFormat(_state->outputFormat, _state->outputVersion); + /* Propagate definitions, if any */ + if(!_state->definitionViews.empty()) + converter->setDefinitions(_state->definitionViews); + /* Try to convert the file (error output should be printed by the plugin itself) */ return converter->convertFileToFile(stage, from, to); diff --git a/src/MagnumPlugins/AnyShaderConverter/AnyConverter.h b/src/MagnumPlugins/AnyShaderConverter/AnyConverter.h index fa8809e88..1bd0797be 100644 --- a/src/MagnumPlugins/AnyShaderConverter/AnyConverter.h +++ b/src/MagnumPlugins/AnyShaderConverter/AnyConverter.h @@ -132,6 +132,7 @@ class MAGNUM_ANYSHADERCONVERTER_EXPORT AnyConverter: public AbstractConverter { MAGNUM_ANYSHADERCONVERTER_LOCAL ConverterFeatures doFeatures() const override; MAGNUM_ANYSHADERCONVERTER_LOCAL void doSetInputFormat(Format, Containers::StringView version) override; MAGNUM_ANYSHADERCONVERTER_LOCAL void doSetOutputFormat(Format, Containers::StringView version) override; + MAGNUM_ANYSHADERCONVERTER_LOCAL void doSetDefinitions(Containers::ArrayView> definitions) override; MAGNUM_ANYSHADERCONVERTER_LOCAL std::pair doValidateFile(Stage stage, Containers::StringView filename) override; MAGNUM_ANYSHADERCONVERTER_LOCAL bool doConvertFileToFile(Stage stage, Containers::StringView from, Containers::StringView to) override; diff --git a/src/MagnumPlugins/AnyShaderConverter/Test/.gitattributes b/src/MagnumPlugins/AnyShaderConverter/Test/.gitattributes new file mode 100644 index 000000000..d715cd5b7 --- /dev/null +++ b/src/MagnumPlugins/AnyShaderConverter/Test/.gitattributes @@ -0,0 +1,10 @@ +# You have to add the following to your .git/config or global +# ~/.gitconfig to make the binary diffs work (without the comment +# character, of course). Assumes spirv-dis exists: +# +# [diff "spirv"] +# textconv = spirv-dis +# binary = true +# + +*.spv binary diff=spirv diff --git a/src/MagnumPlugins/AnyShaderConverter/Test/AnyConverterTest.cpp b/src/MagnumPlugins/AnyShaderConverter/Test/AnyConverterTest.cpp index 7dee01463..6ee39a535 100644 --- a/src/MagnumPlugins/AnyShaderConverter/Test/AnyConverterTest.cpp +++ b/src/MagnumPlugins/AnyShaderConverter/Test/AnyConverterTest.cpp @@ -44,15 +44,19 @@ struct AnyConverterTest: TestSuite::Tester { void validate(); void validateNotSupported(); + void validatePreprocessNotSupported(); void validatePropagateFlags(); void validatePropagateInputVersion(); void validatePropagateOutputVersion(); + void validatePropagatePreprocess(); void convert(); void convertNotSupported(); + void convertPreprocessNotSupported(); void convertPropagateFlags(); void convertPropagateInputVersion(); void convertPropagateOutputVersion(); + void convertPropagatePreprocess(); void detectValidate(); void detectConvert(); @@ -90,15 +94,19 @@ constexpr struct { AnyConverterTest::AnyConverterTest() { addTests({&AnyConverterTest::validate, &AnyConverterTest::validateNotSupported, + &AnyConverterTest::validatePreprocessNotSupported, &AnyConverterTest::validatePropagateFlags, &AnyConverterTest::validatePropagateInputVersion, &AnyConverterTest::validatePropagateOutputVersion, + &AnyConverterTest::validatePropagatePreprocess, &AnyConverterTest::convert, &AnyConverterTest::convertNotSupported, + &AnyConverterTest::convertPreprocessNotSupported, &AnyConverterTest::convertPropagateFlags, &AnyConverterTest::convertPropagateInputVersion, - &AnyConverterTest::convertPropagateOutputVersion}); + &AnyConverterTest::convertPropagateOutputVersion, + &AnyConverterTest::convertPropagatePreprocess}); addInstancedTests({&AnyConverterTest::detectValidate}, Containers::arraySize(DetectValidateData)); @@ -133,13 +141,36 @@ void AnyConverterTest::validate() { /* Make it print a warning so we know it's doing something */ CORRADE_COMPARE(converter->validateFile(Stage::Fragment, filename), - std::make_pair(true, Utility::formatString("WARNING: {}:4: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved", filename))); + std::make_pair(true, Utility::formatString("WARNING: {}:10: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved", filename))); } void AnyConverterTest::validateNotSupported() { CORRADE_SKIP("No plugin that would support just validation exists."); } +void AnyConverterTest::validatePreprocessNotSupported() { + PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; + #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME + CORRADE_VERIFY(manager.load(ANYSHADERCONVERTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + + if(manager.load("SpirvToolsShaderConverter") < PluginManager::LoadState::Loaded) + CORRADE_SKIP("SpirvToolsShaderConverter plugin can't be loaded."); + + Containers::Pointer converter = manager.instantiate("AnyShaderConverter"); + + converter->setDefinitions({ + {"DEFINE", "hahahahah"} + }); + + std::ostringstream out; + Error redirectError{&out}; + CORRADE_COMPARE(converter->validateFile({}, Utility::Directory::join(ANYSHADERCONVERTER_TEST_DIR, "file.spv")), + std::make_pair(false, "")); + CORRADE_COMPARE(out.str(), + "ShaderTools::AnyConverter::validateFile(): SpirvToolsShaderConverter does not support preprocessing\n"); +} + void AnyConverterTest::validatePropagateFlags() { PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME @@ -160,7 +191,7 @@ void AnyConverterTest::validatePropagateFlags() { std::ostringstream out; Debug redirectDebug{&out}; CORRADE_COMPARE(converter->validateFile(Stage::Fragment, filename), - std::make_pair(false, Utility::formatString("WARNING: {}:4: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved", filename))); + std::make_pair(false, Utility::formatString("WARNING: {}:10: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved", filename))); CORRADE_COMPARE(out.str(), "ShaderTools::AnyConverter::validateFile(): using GlslShaderConverter (provided by GlslangShaderConverter)\n"); } @@ -213,6 +244,31 @@ void AnyConverterTest::validatePropagateOutputVersion() { "ShaderTools::GlslangConverter::validateData(): output format should be Unspecified but got ShaderTools::Format::Spirv\n"); } +void AnyConverterTest::validatePropagatePreprocess() { + PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; + #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME + CORRADE_VERIFY(manager.load(ANYSHADERCONVERTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + + if(manager.load("GlslangShaderConverter") < PluginManager::LoadState::Loaded) + CORRADE_SKIP("GlslangShaderConverter plugin can't be loaded."); + + Containers::Pointer converter = manager.instantiate("AnyShaderConverter"); + + const std::string filename = Utility::Directory::join(ANYSHADERCONVERTER_TEST_DIR, "file.glsl"); + + /* Check that undefining works properly -- if it stays defined, the source + won't compile */ + converter->setDefinitions({ + {"SHOULD_BE_UNDEFINED", "really"}, + {"SHOULD_BE_UNDEFINED", nullptr}, + {"reserved__identifier", "different__but_also_wrong"} + }); + + CORRADE_COMPARE(converter->validateFile(Stage::Fragment, filename), + std::make_pair(true, Utility::formatString("WARNING: {}:10: 'different__but_also_wrong' : identifiers containing consecutive underscores (\"__\") are reserved", filename))); +} + void AnyConverterTest::convert() { PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME @@ -236,13 +292,45 @@ void AnyConverterTest::convert() { CORRADE_VERIFY(Utility::Directory::exists(outputFilename)); CORRADE_COMPARE(out.str(), Utility::formatString( "ShaderTools::GlslangConverter::convertDataToData(): compilation succeeded with the following message:\n" - "WARNING: {}:4: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved\n", inputFilename)); + "WARNING: {}:10: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved\n", inputFilename)); } void AnyConverterTest::convertNotSupported() { CORRADE_SKIP("No plugin that would support just validation exists."); } +void AnyConverterTest::convertPreprocessNotSupported() { + PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; + #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME + CORRADE_VERIFY(manager.load(ANYSHADERCONVERTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + + if(manager.load("SpirvToolsShaderConverter") < PluginManager::LoadState::Loaded) + CORRADE_SKIP("SpirvToolsShaderConverter plugin can't be loaded."); + + Containers::Pointer converter = manager.instantiate("AnyShaderConverter"); + + converter->setDefinitions({ + {"DEFINE", "hahahahah"} + }); + + std::ostringstream out; + Error redirectError{&out}; + CORRADE_VERIFY(!converter->convertFileToFile({}, Utility::Directory::join(ANYSHADERCONVERTER_TEST_DIR, "file.spv"), + Utility::Directory::join(ANYSHADERCONVERTER_TEST_OUTPUT_DIR, "file.spvasm"))); + CORRADE_COMPARE(out.str(), + "ShaderTools::AnyConverter::convertFileToFile(): SpirvToolsShaderConverter does not support preprocessing\n"); + + /* It should fail for the flag as well */ + out.str({}); + converter->setDefinitions({}); + converter->setFlags(ConverterFlag::PreprocessOnly); + CORRADE_VERIFY(!converter->convertFileToFile({}, Utility::Directory::join(ANYSHADERCONVERTER_TEST_DIR, "file.spv"), + Utility::Directory::join(ANYSHADERCONVERTER_TEST_OUTPUT_DIR, "file.spvasm"))); + CORRADE_COMPARE(out.str(), + "ShaderTools::AnyConverter::convertFileToFile(): SpirvToolsShaderConverter does not support preprocessing\n"); +} + void AnyConverterTest::convertPropagateFlags() { PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME @@ -270,7 +358,7 @@ void AnyConverterTest::convertPropagateFlags() { CORRADE_COMPARE(out.str(), Utility::formatString( "ShaderTools::AnyConverter::convertFileToFile(): using GlslToSpirvShaderConverter (provided by GlslangShaderConverter)\n" "ShaderTools::GlslangConverter::convertDataToData(): compilation failed:\n" - "WARNING: {}:4: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved\n", filename)); + "WARNING: {}:10: 'reserved__identifier' : identifiers containing consecutive underscores (\"__\") are reserved\n", filename)); } void AnyConverterTest::convertPropagateInputVersion() { @@ -321,6 +409,40 @@ void AnyConverterTest::convertPropagateOutputVersion() { "ShaderTools::GlslangConverter::convertDataToData(): output format version target should be opengl4.5 or vulkanX.Y but got opengl4.0\n"); } +void AnyConverterTest::convertPropagatePreprocess() { + PluginManager::Manager manager{MAGNUM_PLUGINS_SHADERCONVERTER_INSTALL_DIR}; + #ifdef ANYSHADERCONVERTER_PLUGIN_FILENAME + CORRADE_VERIFY(manager.load(ANYSHADERCONVERTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + + if(manager.load("GlslangShaderConverter") < PluginManager::LoadState::Loaded) + CORRADE_SKIP("GlslangShaderConverter plugin can't be loaded."); + + Containers::Pointer converter = manager.instantiate("AnyShaderConverter"); + + /* Check that undefining works properly -- if it stays defined, the source + won't compile */ + converter->setDefinitions({ + {"SHOULD_BE_UNDEFINED", "really"}, + {"SHOULD_BE_UNDEFINED", nullptr}, + {"reserved__identifier", "different__but_also_wrong"} + }); + + const std::string inputFilename = Utility::Directory::join(ANYSHADERCONVERTER_TEST_DIR, "file.glsl"); + const std::string outputFilename = Utility::Directory::join(ANYSHADERCONVERTER_TEST_OUTPUT_DIR, "file.spv"); + Utility::Directory::rm(outputFilename); + CORRADE_VERIFY(!Utility::Directory::exists(outputFilename)); + + /* Make it print a warning so we know it's doing something */ + std::ostringstream out; + Warning redirectWarning{&out}; + CORRADE_VERIFY(converter->convertFileToFile(Stage::Fragment, inputFilename, outputFilename)); + CORRADE_VERIFY(Utility::Directory::exists(outputFilename)); + CORRADE_COMPARE(out.str(), Utility::formatString( + "ShaderTools::GlslangConverter::convertDataToData(): compilation succeeded with the following message:\n" + "WARNING: {}:10: 'different__but_also_wrong' : identifiers containing consecutive underscores (\"__\") are reserved\n", inputFilename)); +} + void AnyConverterTest::detectValidate() { auto&& data = DetectValidateData[testCaseInstanceId()]; setTestCaseDescription(data.name); diff --git a/src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt b/src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt index 2963c9959..653a76708 100644 --- a/src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt +++ b/src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt @@ -47,7 +47,9 @@ file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$/configure.h corrade_add_test(AnyShaderConverterTest AnyConverterTest.cpp LIBRARIES MagnumShaderTools - FILES file.glsl) + FILES + file.glsl + file.spv) target_include_directories(AnyShaderConverterTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) if(BUILD_PLUGINS_STATIC) target_link_libraries(AnyShaderConverterTest PRIVATE AnyShaderConverter) diff --git a/src/MagnumPlugins/AnyShaderConverter/Test/file.glsl b/src/MagnumPlugins/AnyShaderConverter/Test/file.glsl index 07297588e..1d785fc05 100644 --- a/src/MagnumPlugins/AnyShaderConverter/Test/file.glsl +++ b/src/MagnumPlugins/AnyShaderConverter/Test/file.glsl @@ -1,6 +1,12 @@ #version 140 +#ifdef SHOULD_BE_UNDEFINED +#error no, this should not be defined +#endif + void main() { + /* Should get potentially redefined to something else, causing a different + validation message */ float reserved__identifier = 3.0; gl_FragColor = vec4(reserved__identifier); } diff --git a/src/MagnumPlugins/AnyShaderConverter/Test/file.spv b/src/MagnumPlugins/AnyShaderConverter/Test/file.spv new file mode 100644 index 000000000..d386e4dc5 Binary files /dev/null and b/src/MagnumPlugins/AnyShaderConverter/Test/file.spv differ