Browse Source

AnyShaderConverter: propagate preprocessor definitions.

pull/481/head
Vladimír Vondruš 6 years ago
parent
commit
bd3903e637
  1. 60
      src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp
  2. 1
      src/MagnumPlugins/AnyShaderConverter/AnyConverter.h
  3. 10
      src/MagnumPlugins/AnyShaderConverter/Test/.gitattributes
  4. 132
      src/MagnumPlugins/AnyShaderConverter/Test/AnyConverterTest.cpp
  5. 4
      src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt
  6. 6
      src/MagnumPlugins/AnyShaderConverter/Test/file.glsl
  7. BIN
      src/MagnumPlugins/AnyShaderConverter/Test/file.spv

60
src/MagnumPlugins/AnyShaderConverter/AnyConverter.cpp

@ -25,6 +25,7 @@
#include "AnyConverter.h"
#include <Corrade/Containers/Array.h>
#include <Corrade/Containers/String.h>
#include <Corrade/Containers/StringStl.h>
#include <Corrade/PluginManager/Manager.h>
@ -39,6 +40,9 @@ namespace Magnum { namespace ShaderTools {
struct AnyConverter::State {
Format inputFormat, outputFormat;
Containers::String inputVersion, outputVersion;
Containers::Array<std::pair<Containers::String, Containers::String>> definitions;
Containers::Array<std::pair<Containers::StringView, Containers::StringView>> definitionViews;
};
AnyConverter::AnyConverter(PluginManager::Manager<AbstractConverter>& 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<const std::pair<Containers::StringView, Containers::StringView>> definitions) {
/* We have to make a local copy, unfortunately, and then a view on that
local copy */
_state->definitions = Containers::Array<std::pair<Containers::String, Containers::String>>{definitions.size()};
_state->definitionViews = Containers::Array<std::pair<Containers::StringView, Containers::StringView>>{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<bool, Containers::String> 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<AbstractConverter> converter = static_cast<PluginManager::Manager<AbstractConverter>*>(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<AbstractConverter> converter = static_cast<PluginManager::Manager<AbstractConverter>*>(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);

1
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<const std::pair<Containers::StringView, Containers::StringView>> definitions) override;
MAGNUM_ANYSHADERCONVERTER_LOCAL std::pair<bool, Containers::String> doValidateFile(Stage stage, Containers::StringView filename) override;
MAGNUM_ANYSHADERCONVERTER_LOCAL bool doConvertFileToFile(Stage stage, Containers::StringView from, Containers::StringView to) override;

10
src/MagnumPlugins/AnyShaderConverter/Test/.gitattributes vendored

@ -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

132
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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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<AbstractConverter> 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);

4
src/MagnumPlugins/AnyShaderConverter/Test/CMakeLists.txt

@ -47,7 +47,9 @@ file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>/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}/$<CONFIG>)
if(BUILD_PLUGINS_STATIC)
target_link_libraries(AnyShaderConverterTest PRIVATE AnyShaderConverter)

6
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);
}

BIN
src/MagnumPlugins/AnyShaderConverter/Test/file.spv

Binary file not shown.
Loading…
Cancel
Save