From 8eb97dc79c809e9978948a4f2729f0705ca03b1e Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 18 Jul 2018 19:35:28 -0700 Subject: [PATCH] PR feedback --- libraries/gpu/src/gpu/Shader.cpp | 48 ++++++++++++------- libraries/gpu/src/gpu/Shader.h | 5 +- .../procedural/src/procedural/Procedural.cpp | 4 +- tests/shaders/src/ShaderTests.cpp | 2 +- 4 files changed, 35 insertions(+), 24 deletions(-) diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index 1166e86cd1..cea7ec6e14 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -82,8 +82,8 @@ Shader::Pointer Shader::createOrReuseDomainShader(Type type, const Source& sourc if (0 != reflection.count(BindingType::UNIFORM_BUFFER)) { populateSlotSet(shader->_uniformBuffers, reflection.find(BindingType::UNIFORM_BUFFER)->second); } - if (0 != reflection.count(BindingType::STORAGE_BUFFER)) { - populateSlotSet(shader->_resourceBuffers, reflection.find(BindingType::STORAGE_BUFFER)->second); + if (0 != reflection.count(BindingType::RESOURCE_BUFFER)) { + populateSlotSet(shader->_resourceBuffers, reflection.find(BindingType::RESOURCE_BUFFER)->second); } if (0 != reflection.count(BindingType::TEXTURE)) { populateSlotSet(shader->_textures, reflection.find(BindingType::TEXTURE)->second); @@ -91,8 +91,8 @@ Shader::Pointer Shader::createOrReuseDomainShader(Type type, const Source& sourc if (0 != reflection.count(BindingType::SAMPLER)) { populateSlotSet(shader->_samplers, reflection.find(BindingType::SAMPLER)->second); } - if (0 != reflection.count(BindingType::PUSH_CONSTANT)) { - populateSlotSet(shader->_uniforms, reflection.find(BindingType::PUSH_CONSTANT)->second); + if (0 != reflection.count(BindingType::UNIFORM)) { + populateSlotSet(shader->_uniforms, reflection.find(BindingType::UNIFORM)->second); } _domainShaderMaps[type].emplace(source, std::weak_ptr(shader)); return shader; @@ -144,8 +144,8 @@ ShaderPointer Shader::createOrReuseProgramShader(Type type, const Pointer& verte if (0 != reflection.count(BindingType::UNIFORM_BUFFER)) { populateSlotSet(program->_uniformBuffers, reflection.find(BindingType::UNIFORM_BUFFER)->second); } - if (0 != reflection.count(BindingType::STORAGE_BUFFER)) { - populateSlotSet(program->_resourceBuffers, reflection.find(BindingType::STORAGE_BUFFER)->second); + if (0 != reflection.count(BindingType::RESOURCE_BUFFER)) { + populateSlotSet(program->_resourceBuffers, reflection.find(BindingType::RESOURCE_BUFFER)->second); } if (0 != reflection.count(BindingType::TEXTURE)) { populateSlotSet(program->_textures, reflection.find(BindingType::TEXTURE)->second); @@ -153,8 +153,8 @@ ShaderPointer Shader::createOrReuseProgramShader(Type type, const Pointer& verte if (0 != reflection.count(BindingType::SAMPLER)) { populateSlotSet(program->_samplers, reflection.find(BindingType::SAMPLER)->second); } - if (0 != reflection.count(BindingType::PUSH_CONSTANT)) { - populateSlotSet(program->_uniforms, reflection.find(BindingType::PUSH_CONSTANT)->second); + if (0 != reflection.count(BindingType::UNIFORM)) { + populateSlotSet(program->_uniforms, reflection.find(BindingType::UNIFORM)->second); } } @@ -215,6 +215,13 @@ Shader::ReflectionMap getShaderReflection(const std::string& reflectionJson) { return {}; } +#define REFLECT_KEY_INPUTS "inputs" +#define REFLECT_KEY_OUTPUTS "outputs" +#define REFLECT_KEY_UBOS "uniformBuffers" +#define REFLECT_KEY_SSBOS "storageBuffers" +#define REFLECT_KEY_UNIFORMS "uniforms" +#define REFLECT_KEY_TEXTURES "textures" + auto doc = QJsonDocument::fromJson(reflectionJson.c_str()); if (doc.isNull()) { qWarning() << "Invalid shader reflection JSON" << reflectionJson.c_str(); @@ -223,21 +230,26 @@ Shader::ReflectionMap getShaderReflection(const std::string& reflectionJson) { Shader::ReflectionMap result; auto json = doc.object(); - if (json.contains("inputs")) { - updateBindingsFromJsonObject(result[Shader::BindingType::INPUT], json["inputs"].toObject()); + if (json.contains(REFLECT_KEY_INPUTS)) { + updateBindingsFromJsonObject(result[Shader::BindingType::INPUT], json[REFLECT_KEY_INPUTS].toObject()); } - if (json.contains("outputs")) { - updateBindingsFromJsonObject(result[Shader::BindingType::OUTPUT], json["outputs"].toObject()); + if (json.contains(REFLECT_KEY_OUTPUTS)) { + updateBindingsFromJsonObject(result[Shader::BindingType::OUTPUT], json[REFLECT_KEY_OUTPUTS].toObject()); } - if (json.contains("uniformBuffers")) { - updateBindingsFromJsonObject(result[Shader::BindingType::UNIFORM_BUFFER], json["uniformBuffers"].toObject()); + if (json.contains(REFLECT_KEY_UBOS)) { + updateBindingsFromJsonObject(result[Shader::BindingType::UNIFORM_BUFFER], json[REFLECT_KEY_UBOS].toObject()); } - if (json.contains("textures")) { - updateBindingsFromJsonObject(result[Shader::BindingType::TEXTURE], json["textures"].toObject()); + if (json.contains(REFLECT_KEY_TEXTURES)) { + updateBindingsFromJsonObject(result[Shader::BindingType::TEXTURE], json[REFLECT_KEY_TEXTURES].toObject()); } - if (json.contains("uniforms")) { - updateBindingsFromJsonObject(result[Shader::BindingType::PUSH_CONSTANT], json["uniforms"].toObject()); + if (json.contains(REFLECT_KEY_UNIFORMS)) { + updateBindingsFromJsonObject(result[Shader::BindingType::UNIFORM], json[REFLECT_KEY_UNIFORMS].toObject()); } + if (json.contains(REFLECT_KEY_SSBOS)) { + updateBindingsFromJsonObject(result[Shader::BindingType::UNIFORM], json[REFLECT_KEY_SSBOS].toObject()); + } + + return result; } diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index d713433bd6..ad828a0cff 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -51,9 +51,8 @@ public: TEXTURE, SAMPLER, UNIFORM_BUFFER, - STORAGE_BUFFER, - PUSH_CONSTANT, // Equivalent to uniforms in GL - SPECIALIZATION_CONSTANT, + RESOURCE_BUFFER, + UNIFORM, }; using LocationMap = std::unordered_map; diff --git a/libraries/procedural/src/procedural/Procedural.cpp b/libraries/procedural/src/procedural/Procedural.cpp index 644212864e..09e30eab2e 100644 --- a/libraries/procedural/src/procedural/Procedural.cpp +++ b/libraries/procedural/src/procedural/Procedural.cpp @@ -253,10 +253,10 @@ void Procedural::prepare(gpu::Batch& batch, // Build the fragment shader std::string opaqueShaderSource = replaceProceduralBlock(_opaquefragmentSource.getCode()); auto opaqueReflection = _opaquefragmentSource.getReflection(); - auto& opaqueUniforms = opaqueReflection[gpu::Shader::BindingType::PUSH_CONSTANT]; + auto& opaqueUniforms = opaqueReflection[gpu::Shader::BindingType::UNIFORM]; std::string transparentShaderSource = replaceProceduralBlock(_transparentfragmentSource.getCode()); auto transparentReflection = _transparentfragmentSource.getReflection(); - auto& transparentUniforms = transparentReflection[gpu::Shader::BindingType::PUSH_CONSTANT]; + auto& transparentUniforms = transparentReflection[gpu::Shader::BindingType::UNIFORM]; // Set any userdata specified uniforms int customSlot = procedural::slot::uniform::Custom; diff --git a/tests/shaders/src/ShaderTests.cpp b/tests/shaders/src/ShaderTests.cpp index 74ddc1f0eb..dffd69054d 100644 --- a/tests/shaders/src/ShaderTests.cpp +++ b/tests/shaders/src/ShaderTests.cpp @@ -129,7 +129,7 @@ void ShaderTests::testShaderLoad() { const auto& uniformRemap = shaderObject.uniformRemap; #endif auto uniforms = gl::Uniform::load(program); - auto expectedUniforms = expectedBindings[gpu::Shader::BindingType::PUSH_CONSTANT]; + auto expectedUniforms = expectedBindings[gpu::Shader::BindingType::UNIFORM]; if (uniforms.size() != expectedUniforms.size()) { qDebug() << "Found" << toStringList(uniforms, [](const auto& v) { return v.name.c_str(); }); qDebug() << "Expected" << toStringList(expectedUniforms, [](const auto& v) { return v.first.c_str(); });