From 9fcef07612dd1dfceb4293e2e5eb7074a3956418 Mon Sep 17 00:00:00 2001 From: Bernard Kwok Date: Thu, 5 Dec 2024 23:08:57 -0500 Subject: [PATCH 1/7] Allow for choice of font in Graph Editor (#2046) Allow the user to specify a true-type font file and size from the command line. --- source/MaterialXGraphEditor/Main.cpp | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/source/MaterialXGraphEditor/Main.cpp b/source/MaterialXGraphEditor/Main.cpp index 38c755cdd9..0c550dbdd1 100644 --- a/source/MaterialXGraphEditor/Main.cpp +++ b/source/MaterialXGraphEditor/Main.cpp @@ -30,6 +30,8 @@ const std::string options = " --path [FILEPATH] Specify an additional data search path location (e.g. '/projects/MaterialX'). This absolute path will be queried when locating data libraries, XInclude references, and referenced images.\n" " --library [FILEPATH] Specify an additional data library folder (e.g. 'vendorlib', 'studiolib'). This relative path will be appended to each location in the data search path when loading data libraries.\n" " --uiScale [FACTOR] Manually specify a UI scaling factor\n" + " --font [FILENAME] Specify the name of the custom font file to use. If not specified the default font will be used.\n" + " --fontSize [SIZE] Specify font size to use for the custom font. If not specified a default of 18 will be used.\n" " --captureFilename [FILENAME] Specify the filename to which the first rendered frame should be written\n" " --help Display the complete list of command-line options\n"; @@ -67,6 +69,8 @@ int main(int argc, char* const argv[]) int viewWidth = 256; int viewHeight = 256; float uiScale = 0.0f; + std::string fontFilename; + int fontSize = 18; std::string captureFilename; for (size_t i = 0; i < tokens.size(); i++) @@ -102,6 +106,14 @@ int main(int argc, char* const argv[]) { parseToken(nextToken, "float", uiScale); } + else if (token == "--font") + { + parseToken(nextToken, "string", fontFilename); + } + else if (token == "--fontSize") + { + parseToken(nextToken, "integer", fontSize); + } else if (token == "--captureFilename") { parseToken(nextToken, "string", captureFilename); @@ -172,7 +184,15 @@ int main(int argc, char* const argv[]) io.IniFilename = NULL; io.LogFilename = NULL; - io.Fonts->AddFontDefault(); + ImFont* customFont = nullptr; + if (!fontFilename.empty()) + { + customFont = io.Fonts->AddFontFromFileTTF(fontFilename.c_str(), fontSize); + } + if (!customFont) + { + io.Fonts->AddFontDefault(); + } // Setup Dear ImGui style ImGui::StyleColorsDark(); From 142460efb8f1d788b530fbbf5ba93fb26f06046a Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Fri, 6 Dec 2024 15:01:27 -0800 Subject: [PATCH 2/7] Simplify element equivalence interface (#2133) This changelist simplifies the interface of the Element::isEquivalent method in MaterialX C++, aligning it with the behavior of Element::validate and proposed use cases for element equivalence in JavaScript. --- source/MaterialXCore/Element.cpp | 68 +++++++++++-------- source/MaterialXCore/Element.h | 45 ++---------- .../MaterialXTest/MaterialXCore/Document.cpp | 19 ++---- .../PyMaterialX/PyMaterialXCore/PyElement.cpp | 18 +---- 4 files changed, 55 insertions(+), 95 deletions(-) diff --git a/source/MaterialXCore/Element.cpp b/source/MaterialXCore/Element.cpp index 78c59e9efc..5d0cf693d0 100644 --- a/source/MaterialXCore/Element.cpp +++ b/source/MaterialXCore/Element.cpp @@ -39,12 +39,6 @@ const string ValueElement::UI_ADVANCED_ATTRIBUTE = "uiadvanced"; const string ValueElement::UNIT_ATTRIBUTE = "unit"; const string ValueElement::UNITTYPE_ATTRIBUTE = "unittype"; const string ValueElement::UNIFORM_ATTRIBUTE = "uniform"; -const string ElementEquivalenceResult::ATTRIBUTE = "attribute"; -const string ElementEquivalenceResult::ATTRIBUTE_NAMES = "attribute names"; -const string ElementEquivalenceResult::CHILD_COUNT = "child count"; -const string ElementEquivalenceResult::CHILD_NAME = "child name"; -const string ElementEquivalenceResult::NAME = "name"; -const string ElementEquivalenceResult::CATEGORY = "category"; Element::CreatorMap Element::_creatorMap; @@ -375,19 +369,22 @@ bool Element::hasInheritanceCycle() const return false; } -bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options, - ElementEquivalenceResultVec* results) const +bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options, string* message) const { if (getName() != rhs->getName()) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::NAME)); + if (message) + { + *message += "Name of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } if (getCategory() != rhs->getCategory()) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::CATEGORY)); + if (message) + { + *message += "Category of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } @@ -413,14 +410,16 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& if (attributeNames != rhsAttributeNames) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE_NAMES)); + if (message) + { + *message += "Attribute names of '" + getNamePath() + "' differ from '" + rhs->getNamePath() + "\n"; + } return false; } for (const string& attr : rhsAttributeNames) { - if (!isAttributeEquivalent(rhs, attr, options, results)) + if (!isAttributeEquivalent(rhs, attr, options, message)) { return false; } @@ -447,8 +446,10 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& } if (children.size() != rhsChildren.size()) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::CHILD_COUNT)); + if (message) + { + *message += "Child count of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } for (size_t i = 0; i < children.size(); i++) @@ -468,26 +469,29 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& rhsElement = rhs->getChild(childName); if (!rhsElement) { - if (results) - results->push_back(ElementEquivalenceResult(children[i]->getNamePath(), "", - ElementEquivalenceResult::CHILD_NAME, childName)); + if (message) + { + *message += "Child name `" + childName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } } } - if (!children[i]->isEquivalent(rhsElement, options, results)) + if (!children[i]->isEquivalent(rhsElement, options, message)) return false; } return true; } bool Element::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName, - const ElementEquivalenceOptions& /*options*/, ElementEquivalenceResultVec* results) const + const ElementEquivalenceOptions& /*options*/, string* message) const { if (getAttribute(attributeName) != rhs->getAttribute(attributeName)) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName)); + if (message) + { + *message += "Attribute name `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } return true; @@ -710,7 +714,7 @@ const string& ValueElement::getActiveUnit() const } bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName, - const ElementEquivalenceOptions& options, ElementEquivalenceResultVec* results) const + const ElementEquivalenceOptions& options, string* message) const { // Perform value comparisons bool performedValueComparison = false; @@ -737,8 +741,10 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr { if (thisValue->getValueString() != rhsValue->getValueString()) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName)); + if (message) + { + *message += "Attribute `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } } @@ -756,8 +762,10 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr { if (uiValue->getValueString() != rhsUiValue->getValueString()) { - if (results) - results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName)); + if (message) + { + *message += "Attribute `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n"; + } return false; } } @@ -769,7 +777,7 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr // If did not peform a value comparison, perform the default comparison if (!performedValueComparison) { - return Element::isAttributeEquivalent(rhs, attributeName, options, results); + return Element::isAttributeEquivalent(rhs, attributeName, options, message); } return true; diff --git a/source/MaterialXCore/Element.h b/source/MaterialXCore/Element.h index 05eb2235f3..6b145a57b1 100644 --- a/source/MaterialXCore/Element.h +++ b/source/MaterialXCore/Element.h @@ -72,8 +72,6 @@ using ElementMap = std::unordered_map; using ElementPredicate = std::function; class ElementEquivalenceOptions; -class ElementEquivalenceResult; -using ElementEquivalenceResultVec = vector; /// @class Element /// The base class for MaterialX elements. @@ -608,21 +606,21 @@ class MX_CORE_API Element : public std::enable_shared_from_this /// criteria provided. /// @param rhs Element to compare against /// @param options Equivalence criteria - /// @param results Results of comparison if argument is specified. + /// @param message Optional text description of differences /// @return True if the elements are equivalent. False otherwise. - bool isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options, - ElementEquivalenceResultVec* results = nullptr) const; + bool isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options, + string* message = nullptr) const; /// Return true if the attribute on a given element is equivalent /// based on the equivalence criteria provided. /// @param rhs Element to compare against /// @param attributeName Name of attribute to compare /// @param options Equivalence criteria - /// @param results Results of comparison if argument is specified. + /// @param message Optional text description of differences /// @return True if the attribute on the elements are equivalent. False otherwise. virtual bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName, const ElementEquivalenceOptions& options, - ElementEquivalenceResultVec* results = nullptr) const; + string* message = nullptr) const; /// @} /// @name Traversal @@ -1125,11 +1123,11 @@ class MX_CORE_API ValueElement : public TypedElement /// @param rhs Element to compare against /// @param attributeName Name of attribute to compare /// @param options Equivalence criteria - /// @param results Results of comparison if argument is specified. + /// @param message Optional text description of differences /// @return True if the attribute on the elements are equivalent. False otherwise. bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName, const ElementEquivalenceOptions& options, - ElementEquivalenceResultVec* results = nullptr) const override; + string* message = nullptr) const override; /// @} /// @name Validation @@ -1353,35 +1351,6 @@ class MX_CORE_API StringResolver StringMap _geomNameMap; }; -/// @class ElementEquivalenceResult -/// A comparison result for the functional equivalence of two elements. -class MX_CORE_API ElementEquivalenceResult -{ - public: - ElementEquivalenceResult(const string& p1, const string& p2, const string& type, - const string& attrName = EMPTY_STRING) - { - path1 = p1; - path2 = p2; - differenceType = type; - attributeName = attrName; - } - ElementEquivalenceResult() = delete; - ~ElementEquivalenceResult() = default; - - string path1; - string path2; - string differenceType; - string attributeName; - - static const string ATTRIBUTE; - static const string ATTRIBUTE_NAMES; - static const string CHILD_COUNT; - static const string CHILD_NAME; - static const string NAME; - static const string CATEGORY; -}; - /// @class ElementEquivalenceOptions /// A set of options for comparing the functional equivalence of elements. class MX_CORE_API ElementEquivalenceOptions diff --git a/source/MaterialXTest/MaterialXCore/Document.cpp b/source/MaterialXTest/MaterialXCore/Document.cpp index b593969ccb..223aae5694 100644 --- a/source/MaterialXTest/MaterialXCore/Document.cpp +++ b/source/MaterialXTest/MaterialXCore/Document.cpp @@ -215,17 +215,16 @@ TEST_CASE("Document equivalence", "[document]") comment->setDocString("Comment 3"); mx::ElementEquivalenceOptions options; - mx::ElementEquivalenceResultVec results; + std::string message; // Check that this fails when not performing value comparisons options.performValueComparisons = false; - bool equivalent = doc->isEquivalent(doc2, options, &results); + bool equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(!equivalent); // Check attibute values options.performValueComparisons = true; - results.clear(); - equivalent = doc->isEquivalent(doc2, options, &results); + equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(equivalent); unsigned int currentPrecision = mx::Value::getFloatPrecision(); @@ -236,14 +235,13 @@ TEST_CASE("Document equivalence", "[document]") options.floatPrecision = currentPrecision; // Check attribute filtering of inputs - results.clear(); options.attributeExclusionList = { mx::ValueElement::UI_MIN_ATTRIBUTE, mx::ValueElement::UI_MAX_ATTRIBUTE }; for (mx::InputPtr floatInput : floatInputs) { floatInput->setAttribute(mx::ValueElement::UI_MIN_ATTRIBUTE, "0.9"); floatInput->setAttribute(mx::ValueElement::UI_MAX_ATTRIBUTE, "100.0"); } - equivalent = doc->isEquivalent(doc2, options, &results); + equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(equivalent); for (mx::InputPtr floatInput : floatInputs) { @@ -255,12 +253,10 @@ TEST_CASE("Document equivalence", "[document]") mx::ElementPtr mismatchElement = doc->getDescendant("mygraph/input_color4"); std::string previousName = mismatchElement->getName(); mismatchElement->setName("mismatch_color4"); - results.clear(); - equivalent = doc->isEquivalent(doc2, options, &results); + equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(!equivalent); mismatchElement->setName(previousName); - results.clear(); - equivalent = doc->isEquivalent(doc2, options, &results); + equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(equivalent); // Check for functional nodegraphs @@ -272,7 +268,6 @@ TEST_CASE("Document equivalence", "[document]") REQUIRE(nodeGraph2); doc2->addNodeDef("ND_mygraph"); nodeGraph2->setNodeDefString("ND_mygraph"); - results.clear(); - equivalent = doc->isEquivalent(doc2, options, &results); + equivalent = doc->isEquivalent(doc2, options, &message); REQUIRE(!equivalent); } diff --git a/source/PyMaterialX/PyMaterialXCore/PyElement.cpp b/source/PyMaterialX/PyMaterialXCore/PyElement.cpp index 004fa0a670..05f858d4c3 100644 --- a/source/PyMaterialX/PyMaterialXCore/PyElement.cpp +++ b/source/PyMaterialX/PyMaterialXCore/PyElement.cpp @@ -31,9 +31,9 @@ void bindPyElement(py::module& mod) .def(py::self != py::self) .def("isEquivalent", [](const mx::Element& elem, mx::ConstElementPtr& rhs, const mx::ElementEquivalenceOptions& options) { - mx::ElementEquivalenceResultVec results; - bool res = elem.isEquivalent(rhs, options, &results); - return std::pair(res, results); + std::string message; + bool res = elem.isEquivalent(rhs, options, &message); + return std::pair(res, message); }) .def("setCategory", &mx::Element::setCategory) .def("getCategory", &mx::Element::getCategory) @@ -211,18 +211,6 @@ void bindPyElement(py::module& mod) py::class_(mod, "GenericElement") .def_readonly_static("CATEGORY", &mx::GenericElement::CATEGORY); - py::class_(mod, "ElementEquivalenceResult") - .def_readonly_static("ATTRIBUTE", &mx::ElementEquivalenceResult::ATTRIBUTE) - .def_readonly_static("ATTRIBUTE_NAMES", &mx::ElementEquivalenceResult::ATTRIBUTE_NAMES) - .def_readonly_static("CHILD_COUNT", &mx::ElementEquivalenceResult::CHILD_COUNT) - .def_readonly_static("CHILD_NAME", &mx::ElementEquivalenceResult::CHILD_NAME) - .def_readonly_static("NAME", &mx::ElementEquivalenceResult::NAME) - .def_readonly_static("CATEGORY", &mx::ElementEquivalenceResult::CATEGORY) - .def_readwrite("path1", &mx::ElementEquivalenceResult::path1) - .def_readwrite("path2", &mx::ElementEquivalenceResult::path2) - .def_readwrite("differenceType", &mx::ElementEquivalenceResult::differenceType) - .def_readwrite("attributeName", &mx::ElementEquivalenceResult::attributeName); - py::class_(mod, "ElementEquivalenceOptions") .def_readwrite("performValueComparisons", &mx::ElementEquivalenceOptions::performValueComparisons) .def_readwrite("floatFormat", &mx::ElementEquivalenceOptions::floatFormat) From e7f2840974761a852d856e6637e5901bb7b98f2f Mon Sep 17 00:00:00 2001 From: fnRaihanKibria <45946106+fnRaihanKibria@users.noreply.github.com> Date: Fri, 6 Dec 2024 23:13:18 +0000 Subject: [PATCH 3/7] Add checks for loops being created on nodes (#2029) Addressing issue #1932 The cause of the crash is a buffer overrun in ShaderGraph::topologicalSort() in the line `_nodeOrder[count++] = node;`. This is caused by a loop edge being present on the 'color_mix' node of the graph object this was called on, which made the sort algorithm malfunction. Unfortunately I was unable to determine in the time I have available why this loop is created. I didn't want to add a change that hides the underlying problem, which still needs to be fixed. Instead, this PR adds some checks to two locations in the code that throw exceptions when a case of a loop being created is detected, to aid in fixing the real issue and maybe help diagnosing other problems in the future. With this PR the graph editor does not crash any more, instead an error message "Upstream node 'color_mix' has itself as downstream node, creating a loop" is printed to console. --- source/MaterialXGenShader/ShaderGraph.cpp | 5 +++++ source/MaterialXGenShader/ShaderNode.cpp | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/source/MaterialXGenShader/ShaderGraph.cpp b/source/MaterialXGenShader/ShaderGraph.cpp index 8cc71567e4..a8ec936014 100644 --- a/source/MaterialXGenShader/ShaderGraph.cpp +++ b/source/MaterialXGenShader/ShaderGraph.cpp @@ -138,6 +138,11 @@ void ShaderGraph::createConnectedNodes(const ElementPtr& downstreamElement, { // We have a node downstream ShaderNode* downstream = getNode(downstreamNode->getName()); + if (downstream == newNode) + { + throw ExceptionShaderGenError("Upstream node '" + downstream->getName() + "' has itself as downstream node, creating a loop"); + } + if (downstream && connectingElement) { ShaderInput* input = downstream->getInput(connectingElement->getName()); diff --git a/source/MaterialXGenShader/ShaderNode.cpp b/source/MaterialXGenShader/ShaderNode.cpp index d52810ffc4..12b9bbf548 100644 --- a/source/MaterialXGenShader/ShaderNode.cpp +++ b/source/MaterialXGenShader/ShaderNode.cpp @@ -57,6 +57,13 @@ void ShaderInput::makeConnection(ShaderOutput* src) if (src) { // Make the new connection. + if (src->getNode() == getNode() && !getNode()->isAGraph()) + { + throw ExceptionShaderGenError( + "Tried to create looping connection on node " + getNode()->getName() + + " from output: " + src->getFullName() + " to input: " + getFullName()); + } + _connection = src; src->_connections.push_back(this); } From 9be30a29102028b2d1ed3afd5c196d2cbe9da3ee Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Sun, 8 Dec 2024 11:08:36 -0800 Subject: [PATCH 4/7] Static analysis fixes (#2135) This changelist addresses a handful of static analysis warnings flagged by PVS-Studio: - Validate `downstream` before dereferencing in `ShaderGraph::createConnectedNodes`. - Pass a string argument by const reference in `StructTypeDesc::addMember`. - Avoid repeated calls to `Value::asA` in `Graph::setConstant`. --- source/MaterialXGenShader/ShaderGraph.cpp | 31 +++++++++++++---------- source/MaterialXGenShader/TypeDesc.cpp | 2 +- source/MaterialXGenShader/TypeDesc.h | 2 +- source/MaterialXGraphEditor/Graph.cpp | 30 ++++++++++++++-------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/source/MaterialXGenShader/ShaderGraph.cpp b/source/MaterialXGenShader/ShaderGraph.cpp index a8ec936014..c60910aaed 100644 --- a/source/MaterialXGenShader/ShaderGraph.cpp +++ b/source/MaterialXGenShader/ShaderGraph.cpp @@ -138,24 +138,27 @@ void ShaderGraph::createConnectedNodes(const ElementPtr& downstreamElement, { // We have a node downstream ShaderNode* downstream = getNode(downstreamNode->getName()); - if (downstream == newNode) + if (downstream) { - throw ExceptionShaderGenError("Upstream node '" + downstream->getName() + "' has itself as downstream node, creating a loop"); - } + if (downstream == newNode) + { + throw ExceptionShaderGenError("Upstream node '" + downstream->getName() + "' has itself as downstream node, creating a loop"); + } - if (downstream && connectingElement) - { - ShaderInput* input = downstream->getInput(connectingElement->getName()); - if (!input) + if (connectingElement) + { + ShaderInput* input = downstream->getInput(connectingElement->getName()); + if (!input) + { + throw ExceptionShaderGenError("Could not find an input named '" + connectingElement->getName() + + "' on downstream node '" + downstream->getName() + "'"); + } + input->makeConnection(output); + } + else { - throw ExceptionShaderGenError("Could not find an input named '" + connectingElement->getName() + - "' on downstream node '" + downstream->getName() + "'"); + throw ExceptionShaderGenError("Could not find downstream node ' " + downstreamNode->getName() + "'"); } - input->makeConnection(output); - } - else - { - throw ExceptionShaderGenError("Could not find downstream node ' " + downstreamNode->getName() + "'"); } } else diff --git a/source/MaterialXGenShader/TypeDesc.cpp b/source/MaterialXGenShader/TypeDesc.cpp index 5d51680211..5ea0e31764 100644 --- a/source/MaterialXGenShader/TypeDesc.cpp +++ b/source/MaterialXGenShader/TypeDesc.cpp @@ -145,7 +145,7 @@ TYPEDESC_REGISTER_TYPE(MATERIAL, "material") // StructTypeDesc methods // -void StructTypeDesc::addMember(const string& name, TypeDesc type, string defaultValueStr) +void StructTypeDesc::addMember(const string& name, TypeDesc type, const string& defaultValueStr) { _members.emplace_back(StructTypeDesc::StructMemberTypeDesc(name, type, defaultValueStr)); } diff --git a/source/MaterialXGenShader/TypeDesc.h b/source/MaterialXGenShader/TypeDesc.h index 95d2851915..d19fea604a 100644 --- a/source/MaterialXGenShader/TypeDesc.h +++ b/source/MaterialXGenShader/TypeDesc.h @@ -255,7 +255,7 @@ class MX_GENSHADER_API StructTypeDesc /// Empty constructor. StructTypeDesc() noexcept{} - void addMember(const string& name, TypeDesc type, string defaultValueStr); + void addMember(const string& name, TypeDesc type, const string& defaultValueStr); void setTypeDesc(TypeDesc typedesc) { _typedesc = typedesc; } /// Return a type description by index. diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index d09961d42f..9e075b9dc4 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -926,7 +926,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert if (val && val->isA()) { // Update the value to the default for new nodes - float prev = val->asA(), temp = val->asA(); + float prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA() : 0.f; float max = maxVal ? maxVal->asA() : 100.f; float speed = (max - min) / 1000.0f; @@ -946,7 +947,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - int prev = val->asA(), temp = val->asA(); + int prev, temp; + prev = temp = val->asA(); int min = minVal ? minVal->asA() : 0; int max = maxVal ? maxVal->asA() : 100; float speed = (max - min) / 100.0f; @@ -966,7 +968,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - mx::Color3 prev = val->asA(), temp = val->asA(); + mx::Color3 prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA()[0] : 0.f; float max = maxVal ? maxVal->asA()[0] : 100.f; float speed = (max - min) / 1000.0f; @@ -990,7 +993,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - mx::Color4 prev = val->asA(), temp = val->asA(); + mx::Color4 prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA()[0] : 0.f; float max = maxVal ? maxVal->asA()[0] : 100.f; float speed = (max - min) / 1000.0f; @@ -1016,7 +1020,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - mx::Vector2 prev = val->asA(), temp = val->asA(); + mx::Vector2 prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA()[0] : 0.f; float max = maxVal ? maxVal->asA()[0] : 100.f; float speed = (max - min) / 1000.0f; @@ -1036,7 +1041,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - mx::Vector3 prev = val->asA(), temp = val->asA(); + mx::Vector3 prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA()[0] : 0.f; float max = maxVal ? maxVal->asA()[0] : 100.f; float speed = (max - min) / 1000.0f; @@ -1056,7 +1062,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - mx::Vector4 prev = val->asA(), temp = val->asA(); + mx::Vector4 prev, temp; + prev = temp = val->asA(); float min = minVal ? minVal->asA()[0] : 0.f; float max = maxVal ? maxVal->asA()[0] : 100.f; float speed = (max - min) / 1000.0f; @@ -1076,7 +1083,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - std::string prev = val->asA(), temp = val->asA(); + std::string prev, temp; + prev = temp = val->asA(); ImGui::InputText("##constant", &temp); // Set input value and update materials if different from previous value @@ -1094,7 +1102,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert if (val && val->isA()) { - std::string temp = val->asA(), prev = val->asA(); + std::string prev, temp; + prev = temp = val->asA(); ImGui::PushStyleColor(ImGuiCol_Button, ImVec4(.15f, .15f, .15f, 1.0f)); ImGui::PushStyleColor(ImGuiCol_ButtonHovered, ImVec4(.2f, .4f, .6f, 1.0f)); @@ -1142,7 +1151,8 @@ void Graph::setConstant(UiNodePtr node, mx::InputPtr& input, const mx::UIPropert mx::ValuePtr val = input->getValue(); if (val && val->isA()) { - bool prev = val->asA(), temp = val->asA(); + bool prev, temp; + prev = temp = val->asA(); ImGui::Checkbox("", &temp); // Set input value and update materials if different from previous value From 8929e07e7a3888b32656eb0ffef05b2d39b4381a Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Sun, 8 Dec 2024 16:32:51 -0800 Subject: [PATCH 5/7] Improve usage of setDataLibrary (#2136) This changelist improves the usage of Document::setDataLibrary in MaterialXRender functions, leveraging this more efficient alternative to Document::importLibrary where appropriate. --- source/MaterialXRender/ShaderMaterial.cpp | 6 ++---- source/MaterialXRender/Util.cpp | 10 +++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/source/MaterialXRender/ShaderMaterial.cpp b/source/MaterialXRender/ShaderMaterial.cpp index c61c6d3c04..6fcc1a661d 100644 --- a/source/MaterialXRender/ShaderMaterial.cpp +++ b/source/MaterialXRender/ShaderMaterial.cpp @@ -68,10 +68,8 @@ bool ShaderMaterial::generateEnvironmentShader(GenContext& context, { // Read in the environment nodegraph. DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); - DocumentPtr envDoc = createDocument(); - readFromXmlFile(envDoc, filename); - doc->importLibrary(envDoc); + doc->setDataLibrary(stdLib); + readFromXmlFile(doc, filename); NodeGraphPtr envGraph = doc->getNodeGraph("envMap"); if (!envGraph) diff --git a/source/MaterialXRender/Util.cpp b/source/MaterialXRender/Util.cpp index c206514412..9aab6b4811 100644 --- a/source/MaterialXRender/Util.cpp +++ b/source/MaterialXRender/Util.cpp @@ -24,7 +24,7 @@ ShaderPtr createConstantShader(GenContext& context, { // Construct the constant color nodegraph DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); + doc->setDataLibrary(stdLib); NodeGraphPtr nodeGraph = doc->addNodeGraph(); NodePtr constant = nodeGraph->addNode("constant"); constant->setInputValue("value", color); @@ -41,7 +41,7 @@ ShaderPtr createDepthShader(GenContext& context, { // Construct a dummy nodegraph. DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); + doc->setDataLibrary(stdLib); NodeGraphPtr nodeGraph = doc->addNodeGraph(); NodePtr constant = nodeGraph->addNode("constant"); OutputPtr output = nodeGraph->addOutput(); @@ -61,7 +61,7 @@ ShaderPtr createAlbedoTableShader(GenContext& context, { // Construct a dummy nodegraph. DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); + doc->setDataLibrary(stdLib); NodeGraphPtr nodeGraph = doc->addNodeGraph(); NodePtr constant = nodeGraph->addNode("constant"); OutputPtr output = nodeGraph->addOutput(); @@ -82,7 +82,7 @@ ShaderPtr createEnvPrefilterShader(GenContext& context, { // Construct a dummy nodegraph. DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); + doc->setDataLibrary(stdLib); NodeGraphPtr nodeGraph = doc->addNodeGraph(); NodePtr constant = nodeGraph->addNode("constant"); OutputPtr output = nodeGraph->addOutput(); @@ -104,7 +104,7 @@ ShaderPtr createBlurShader(GenContext& context, { // Construct the blur nodegraph DocumentPtr doc = createDocument(); - doc->importLibrary(stdLib); + doc->setDataLibrary(stdLib); NodeGraphPtr nodeGraph = doc->addNodeGraph(); NodePtr imageNode = nodeGraph->addNode("image", "image"); NodePtr blurNode = nodeGraph->addNode("blur", "blur"); From 12235809a1e26a7a80da9c990723e804fd6562fc Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Mon, 9 Dec 2024 09:57:53 -0800 Subject: [PATCH 6/7] Use setDataLibrary in Graph Editor (#2137) This changelist updates the Graph Editor to use Element::setDataLibrary rather than Element::importLibrary, improving the efficiency of document loading. Additionally, it removes an unused method and member variable from the RenderView class. --- source/MaterialXGraphEditor/Graph.cpp | 6 +++--- source/MaterialXGraphEditor/RenderView.cpp | 18 ++++-------------- source/MaterialXGraphEditor/RenderView.h | 4 ++-- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 9e075b9dc4..1c3dfa66ba 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -171,7 +171,7 @@ Graph::Graph(const std::string& materialFilename, // Create a renderer using the initial startup document. mx::FilePath captureFilename = "resources/Materials/Examples/example.png"; std::string envRadianceFilename = "resources/Lights/san_giuseppe_bridge_split.hdr"; - _renderer = std::make_shared(_graphDoc, meshFilename, envRadianceFilename, + _renderer = std::make_shared(_graphDoc, _stdLib, meshFilename, envRadianceFilename, _searchPath, viewWidth, viewHeight); _renderer->initialize(); for (const std::string& ext : _renderer->getImageHandler()->supportedExtensions()) @@ -249,7 +249,7 @@ mx::DocumentPtr Graph::loadDocument(const mx::FilePath& filename) if (!filename.isEmpty()) { mx::readFromXmlFile(doc, filename, _searchPath, &readOptions); - doc->importLibrary(_stdLib); + doc->setDataLibrary(_stdLib); std::string message; if (!doc->validate(&message)) { @@ -3050,7 +3050,7 @@ void Graph::clearGraph() _newLinks.clear(); _currPins.clear(); _graphDoc = mx::createDocument(); - _graphDoc->importLibrary(_stdLib); + _graphDoc->setDataLibrary(_stdLib); _currGraphElem = _graphDoc; if (_currUiNode != nullptr) diff --git a/source/MaterialXGraphEditor/RenderView.cpp b/source/MaterialXGraphEditor/RenderView.cpp index 92cd8f07c7..fe419c7908 100644 --- a/source/MaterialXGraphEditor/RenderView.cpp +++ b/source/MaterialXGraphEditor/RenderView.cpp @@ -108,6 +108,7 @@ void RenderView::setDocument(mx::DocumentPtr document) } RenderView::RenderView(mx::DocumentPtr doc, + mx::DocumentPtr stdLib, const std::string& meshFilename, const std::string& envRadianceFilename, const mx::FileSearchPath& searchPath, @@ -162,6 +163,7 @@ RenderView::RenderView(mx::DocumentPtr doc, _genContext.getOptions().shaderInterfaceType = mx::SHADER_INTERFACE_COMPLETE; setDocument(doc); + _stdLib = stdLib; } void RenderView::initialize() @@ -214,18 +216,6 @@ void RenderView::assignMaterial(mx::MeshPartitionPtr geometry, mx::GlslMaterialP } } -mx::ElementPredicate RenderView::getElementPredicate() -{ - return [this](mx::ConstElementPtr elem) - { - if (elem->hasSourceUri()) - { - return (_xincludeFiles.count(elem->getSourceUri()) == 0); - } - return true; - }; -} - void RenderView::updateGeometrySelections() { _geometryList.clear(); @@ -935,7 +925,7 @@ mx::ImagePtr RenderView::getShadowMap() { try { - mx::ShaderPtr hwShader = mx::createDepthShader(_genContext, _document, "__SHADOW_SHADER__"); + mx::ShaderPtr hwShader = mx::createDepthShader(_genContext, _stdLib, "__SHADOW_SHADER__"); _shadowMaterial = mx::GlslMaterial::create(); _shadowMaterial->generateShader(hwShader); } @@ -949,7 +939,7 @@ mx::ImagePtr RenderView::getShadowMap() { try { - mx::ShaderPtr hwShader = mx::createBlurShader(_genContext, _document, "__SHADOW_BLUR_SHADER__", "gaussian", 1.0f); + mx::ShaderPtr hwShader = mx::createBlurShader(_genContext, _stdLib, "__SHADOW_BLUR_SHADER__", "gaussian", 1.0f); _shadowBlurMaterial = mx::GlslMaterial::create(); _shadowBlurMaterial->generateShader(hwShader); } diff --git a/source/MaterialXGraphEditor/RenderView.h b/source/MaterialXGraphEditor/RenderView.h index 354e898ce1..c4c709dc9f 100644 --- a/source/MaterialXGraphEditor/RenderView.h +++ b/source/MaterialXGraphEditor/RenderView.h @@ -29,6 +29,7 @@ class RenderView { public: RenderView(mx::DocumentPtr doc, + mx::DocumentPtr stdLib, const std::string& meshFilename, const std::string& envRadianceFilename, const mx::FileSearchPath& searchPath, @@ -159,7 +160,6 @@ class RenderView { return _xincludeFiles; } - mx::ElementPredicate getElementPredicate(); // Request a capture of the current frame, writing it to the given filename. void requestFrameCapture(const mx::FilePath& filename) @@ -232,7 +232,6 @@ class RenderView void updateGeometrySelections(); mx::ImagePtr getShadowMap(); - mx::ImagePtr _renderMap; void renderFrame(); void renderScreenSpaceQuad(mx::GlslMaterialPtr material); @@ -268,6 +267,7 @@ class RenderView // Document management mx::DocumentPtr _document; + mx::DocumentPtr _stdLib; DocumentModifiers _modifiers; mx::StringSet _xincludeFiles; From 31d8240036ba45bf363414db83b6542d0c3eabac Mon Sep 17 00:00:00 2001 From: Bernard Kwok Date: Mon, 9 Dec 2024 13:43:42 -0500 Subject: [PATCH 7/7] Improve input filtering in Graph Editor (#2134) - Move "show all inputs" to the top. - Make name field expand to right marge on panel. - Format the title of the property editor. - Remove extraneous "Inputs:" line since only inputs are displayed. --- source/MaterialXGraphEditor/Graph.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 1c3dfa66ba..aa04339469 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -3288,7 +3288,20 @@ void Graph::graphButtons() void Graph::propertyEditor() { + // Get parent dimensions + ImVec2 textPos = ImGui::GetCursorScreenPos(); // Position for the background + float parentWidth = ImGui::GetContentRegionAvail().x; // Available width in the parent + + // Draw the title bar + const ImGuiStyle& style = ImGui::GetStyle(); + ImVec4 menuBarBgColor = style.Colors[ImGuiCol_MenuBarBg]; + ImU32 bgColor = ImGui::ColorConvertFloat4ToU32(menuBarBgColor); // Convert to 32-bit color + ImDrawList* drawList = ImGui::GetWindowDrawList(); + drawList->AddRectFilled(textPos, + ImVec2(textPos.x + parentWidth, textPos.y + ImGui::GetTextLineHeight()), + bgColor); ImGui::Text("Node Property Editor"); + if (_currUiNode) { // Set and edit name @@ -3296,7 +3309,11 @@ void Graph::propertyEditor() ImGui::SameLine(); std::string original = _currUiNode->getName(); std::string temp = original; + float availableWidth = ImGui::GetContentRegionAvail().x; + ImGui::PushItemWidth(availableWidth); ImGui::InputText("##edit", &temp); + ImGui::PopItemWidth(); + std::string docString = "NodeDef Doc String: \n"; if (_currUiNode->getNode()) { @@ -3423,7 +3440,8 @@ void Graph::propertyEditor() ImGui::SetTooltip("%s", _currUiNode->getNode()->getNodeDef()->getDocString().c_str()); } - ImGui::Text("Inputs:"); + ImGui::Checkbox("Show all inputs", &_currUiNode->_showAllInputs); + int count = 0; for (UiPinPtr input : _currUiNode->inputPins) { @@ -3487,14 +3505,12 @@ void Graph::propertyEditor() ImGui::SetWindowFontScale(1.0f); } } - ImGui::Checkbox("Show all inputs", &_currUiNode->_showAllInputs); } else if (_currUiNode->getInput() != nullptr) { ImGui::Text("%s", _currUiNode->getCategory().c_str()); std::vector inputs = _currUiNode->inputPins; - ImGui::Text("Inputs:"); int count = static_cast(inputs.size()); if (count) @@ -3545,7 +3561,6 @@ void Graph::propertyEditor() { std::vector inputs = _currUiNode->inputPins; ImGui::Text("%s", _currUiNode->getCategory().c_str()); - ImGui::Text("Inputs:"); int count = 0; for (UiPinPtr input : inputs) {