Skip to content

Commit

Permalink
Viewer fixes merge to master (#271)
Browse files Browse the repository at this point in the history 
- Fix so that node implementation cache is cleared before each shader generate to avoid accidental reuse. This fixes the ability to generate proper code for shader graphs that use UDIMs defined in images nodes which are part of a nodedef.
- Allow for a shared shader to be used for all UDIMs so that when editing in the Property Editor editing one viewer Material ends up editing the display for all geometry using the shared Shader (though different Materials are assigned as UDIM association is at the Material level)
- Fix regression in editing inputs which affected uniforms which have generated variable names different than the ShaderPort name. When generation was added, the editor code was not updated to use the variable getter interface but was instead using the existing name getter interface. This resulted in the shader uniform not being found and no updates occurring. 
Note: Any inputs without generated names would still work before this fix.
  • Loading branch information
@bernardkwok @jstone-lucasfilm
bernardkwok authored and jstone-lucasfilm committed Jul 26, 2019
1 parent 9f1322c commit 5ee03bb
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 45 deletions.
2 changes: 1 addition & 1 deletion source/MaterialXRender/Util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ ShaderPtr createConstantShader(GenContext& context,
return createShader(shaderName, context, output);
}

unsigned int getUIProperties(ValueElementPtr nodeDefElement, UIProperties& uiProperties)
unsigned int getUIProperties(ConstValueElementPtr nodeDefElement, UIProperties& uiProperties)
{
if (!nodeDefElement)
{
Expand Down
4 changes: 2 additions & 2 deletions source/MaterialXRender/Util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace MaterialX

/// Get the UI properties for a given nodedef element.
/// Returns the number of properties found.
unsigned int getUIProperties(ValueElementPtr nodeDefElement, UIProperties& uiProperties);
unsigned int getUIProperties(ConstValueElementPtr nodeDefElement, UIProperties& uiProperties);

/// Get the UI properties for a given element path. If the path is to a node, a target
/// identifier can be provided.
Expand All @@ -71,7 +71,7 @@ namespace MaterialX
/// Interface for holding the UI properties associated shader port
struct UIPropertyItem
{
std::string label;
string label;
ValuePtr value;
ShaderPort* variable = nullptr;
UIProperties ui;
Expand Down
18 changes: 9 additions & 9 deletions source/MaterialXRenderGlsl/GlslProgram.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1197,14 +1197,14 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
{
const ShaderPort* v = constants[i];
// There is no way to match with an unnamed variable
if (v->getName().empty())
if (v->getVariable().empty())
{
continue;
}

// TODO: Shoud we really create new ones here each update?
InputPtr inputPtr = std::make_shared<Input>(-1, -1, int(v->getType()->getSize()), EMPTY_STRING);
_uniformList[v->getName()] = inputPtr;
_uniformList[v->getVariable()] = inputPtr;
inputPtr->isConstant = true;
inputPtr->value = v->getValue();
inputPtr->typeString = v->getType()->getName();
Expand All @@ -1225,12 +1225,12 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
{
const ShaderPort* v = uniforms[i];
// There is no way to match with an unnamed variable
if (v->getName().empty())
if (v->getVariable().empty())
{
continue;
}

auto inputIt = _uniformList.find(v->getName());
auto inputIt = _uniformList.find(v->getVariable());
if (inputIt != _uniformList.end())
{
Input* input = inputIt->second.get();
Expand All @@ -1244,7 +1244,7 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
{
errors.push_back(
"Pixel shader uniform block type mismatch [" + uniforms.getName() + "]. "
+ "Name: \"" + v->getName()
+ "Name: \"" + v->getVariable()
+ "\". Type: \"" + v->getType()->getName()
+ "\". Semantic: \"" + v->getSemantic()
+ "\". Value: \"" + (v->getValue() ? v->getValue()->getValueString() : "<none>")
Expand All @@ -1263,7 +1263,7 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
for (size_t i = 0; i < uniforms.size(); ++i)
{
const ShaderPort* v = uniforms[i];
auto inputIt = _uniformList.find(v->getName());
auto inputIt = _uniformList.find(v->getVariable());
if (inputIt != _uniformList.end())
{
Input* input = inputIt->second.get();
Expand All @@ -1277,7 +1277,7 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
{
errors.push_back(
"Vertex shader uniform block type mismatch [" + uniforms.getName() + "]. "
+ "Name: \"" + v->getName()
+ "Name: \"" + v->getVariable()
+ "\". Type: \"" + v->getType()->getName()
+ "\". Semantic: \"" + v->getSemantic()
+ "\". Value: \"" + (v->getValue() ? v->getValue()->getValueString() : "<none>")
Expand Down Expand Up @@ -1391,7 +1391,7 @@ const GlslProgram::InputMap& GlslProgram::updateAttributesList()
for (size_t i = 0; i < vertexInputs.size(); ++i)
{
const ShaderPort* v = vertexInputs[i];
auto inputIt = _attributeList.find(v->getName());
auto inputIt = _attributeList.find(v->getVariable());
if (inputIt != _attributeList.end())
{
Input* input = inputIt->second.get();
Expand All @@ -1403,7 +1403,7 @@ const GlslProgram::InputMap& GlslProgram::updateAttributesList()
else
{
errors.push_back(
"Vertex shader attribute type mismatch in block. Name: \"" + v->getName()
"Vertex shader attribute type mismatch in block. Name: \"" + v->getVariable()
+ "\". Type: \"" + v->getType()->getName()
+ "\". Semantic: \"" + v->getSemantic()
+ "\". Value: \"" + (v->getValue() ? v->getValue()->getValueString() : "<none>")
Expand Down
38 changes: 19 additions & 19 deletions source/MaterialXView/Editor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ ng::FloatBox<float>* PropertyEditor::makeFloatWidget(ng::Widget* container, cons
if (uniform)
{
material->getShader()->bind();
material->getShader()->setUniform(uniform->getName(), value);
material->getShader()->setUniform(uniform->getVariable(), value);
}
}
});
Expand All @@ -186,7 +186,7 @@ ng::FloatBox<float>* PropertyEditor::makeFloatWidget(ng::Widget* container, cons
if (uniform)
{
material->getShader()->bind();
material->getShader()->setUniform(uniform->getName(), value);
material->getShader()->setUniform(uniform->getVariable(), value);
}
}
});
Expand Down Expand Up @@ -250,11 +250,11 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
material->getShader()->bind();
if (v < (int) enumValues.size())
{
material->getShader()->setUniform(uniform->getName(), enumValues[v]->asA<int>());
material->getShader()->setUniform(uniform->getVariable(), enumValues[v]->asA<int>());
}
else
{
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
}
});
Expand All @@ -278,7 +278,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
if (uniform)
{
material->getShader()->bind();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
}
});
Expand Down Expand Up @@ -311,7 +311,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
if (uniform)
{
material->getShader()->bind();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
}
Expand Down Expand Up @@ -342,7 +342,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
ng::Vector2f v;
v.x() = c.r();
v.y() = c.g();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
ng::Color c2 = c;
c2.b() = 0.0f;
c2.w() = 1.0f;
Expand Down Expand Up @@ -395,7 +395,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.x() = c[0];
v.y() = c[1];
v.z() = c[2];
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
}
});
Expand Down Expand Up @@ -424,7 +424,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.x() = c.r();
v.y() = c.g();
v.z() = c.b();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
}
Expand Down Expand Up @@ -458,7 +458,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.y() = c.g();
v.z() = c.b();
v.w() = c.w();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
}
Expand Down Expand Up @@ -488,7 +488,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
ng::Vector2f v;
v.x() = f;
v.y() = v2->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v1->setSpinnable(editable);
Expand All @@ -502,7 +502,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
ng::Vector2f v;
v.x() = v1->value();
v.y() = f;
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v2->setSpinnable(editable);
Expand Down Expand Up @@ -539,7 +539,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.x() = f;
v.y() = v2->value();
v.z() = v3->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v1->setSpinnable(editable);
Expand All @@ -554,7 +554,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.x() = v1->value();
v.y() = f;
v.z() = v3->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v2->setSpinnable(editable);
Expand All @@ -569,7 +569,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.x() = v1->value();
v.y() = v2->value();
v.z() = f;
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v3->setSpinnable(editable);
Expand Down Expand Up @@ -611,7 +611,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.y() = v2->value();
v.z() = v3->value();
v.w() = v4->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v1->setSpinnable(editable);
Expand All @@ -627,7 +627,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.y() = f;
v.z() = v3->value();
v.w() = v4->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v2->setSpinnable(editable);
Expand All @@ -643,7 +643,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.y() = v2->value();
v.z() = f;
v.w() = v4->value();
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v3->setSpinnable(editable);
Expand All @@ -659,7 +659,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
v.y() = v2->value();
v.z() = v3->value();
v.w() = f;
material->getShader()->setUniform(uniform->getName(), v);
material->getShader()->setUniform(uniform->getVariable(), v);
}
});
v4->setSpinnable(editable);
Expand Down
12 changes: 6 additions & 6 deletions source/MaterialXView/Material.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ bool Material::loadSource(const mx::FilePath& vertexShaderFile, const mx::FilePa

void Material::updateUniformsList()
{
_uniformNames.clear();
_uniformVariable.clear();

// Must bind to be able to inspect the uniforms
_glShader->bind();
Expand All @@ -173,7 +173,7 @@ void Material::updateUniformsList()
GLint uniformLocation = glGetUniformLocation(_programId, uniformName);
if (uniformLocation >= 0)
{
_uniformNames.insert(uniformName);
_uniformVariable.insert(uniformName);
}
}
delete[] uniformName;
Expand Down Expand Up @@ -326,7 +326,7 @@ void Material::bindImages(mx::GLTextureHandlerPtr imageHandler, const mx::FileSe
{
continue;
}
const std::string& uniformName = uniform->getName();
const std::string& uniformVariable = uniform->getVariable();
std::string filename;
if (uniform->getValue())
{
Expand All @@ -335,10 +335,10 @@ void Material::bindImages(mx::GLTextureHandlerPtr imageHandler, const mx::FileSe

// Extract out sampling properties
mx::ImageSamplingProperties samplingProperties;
samplingProperties.setProperties(uniformName, *publicUniforms);
samplingProperties.setProperties(uniformVariable, *publicUniforms);

mx::ImageDesc desc;
mx::FilePath resolvedFilename = bindImage(filename, uniformName, imageHandler, desc, samplingProperties, udim, &fallbackColor);
mx::FilePath resolvedFilename = bindImage(filename, uniformVariable, imageHandler, desc, samplingProperties, udim, &fallbackColor);
if (!resolvedFilename.isEmpty())
{
_boundImages.push_back(resolvedFilename);
Expand Down Expand Up @@ -602,7 +602,7 @@ mx::ShaderPort* Material::findUniform(const std::string& path) const
});

// Check if the uniform exists in the shader program
if (port && !_uniformNames.count(port->getName()))
if (port && !_uniformVariable.count(port->getVariable()))
{
port = nullptr;
}
Expand Down
9 changes: 8 additions & 1 deletion source/MaterialXView/Material.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ class Material
/// Generate a shader from the given inputs.
bool generateShader(mx::GenContext& context);

/// Copy shader from one material to this one
void copyShader(MaterialPtr material)
{
_hwShader = material->_hwShader;
_glShader = material->_glShader;
}

/// Generate a constant color shader
bool generateConstantShader(mx::GenContext& context,
mx::DocumentPtr stdLib,
Expand Down Expand Up @@ -170,7 +177,7 @@ class Material

std::string _udim;
bool _hasTransparency;
mx::StringSet _uniformNames;
mx::StringSet _uniformVariable;

std::vector<mx::FilePath> _boundImages;
};
Expand Down
Loading

0 comments on commit 5ee03bb

Please sign in to comment.