-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LOOKDEVX-470 : Initial support for export. #1177
Conversation
@@ -11,5 +11,5 @@ | |||
<look name="defaultLook"> | |||
<materialassign name="defaultMaterialAssign" collection="defaultCollection" material="defaultMaterial"/> | |||
</look> | |||
<lookgroup name="defaultLookGroup" looks="defaultLook" activeLook="defaultLook" /> | |||
<lookgroup name="defaultLookGroup" looks="defaultLook" enabled="defaultLook" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultLook.mtlx fix
@@ -0,0 +1,48 @@ | |||
<?xml version="1.0"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed for test case.
@@ -2719,3 +2719,54 @@ TEST_CASE("Runtime: duplicate name", "[runtime]") | |||
REQUIRE(graph1.getOutputSocket(ADD5)); | |||
REQUIRE(duplicateCount(ADD5) == 1); | |||
} | |||
|
|||
|
|||
TEST_CASE("Export", "[runtime]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple test case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some logic comments and also that we require export at the core level and not inside runtime
for general usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
source/MaterialXCore/Document.cpp
Outdated
LookPtr mainLook = mainLookGroup->combineLooks(); | ||
// Delete the mainLookGroup | ||
removeChild(mainLookGroup->getName()); | ||
// Append look names that don't belong to the mainLook to lookNames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"append the looks which are not the main look."
source/MaterialXFormat/XmlIo.cpp
Outdated
@@ -362,6 +372,32 @@ string writeToXmlString(DocumentPtr doc, const XmlWriteOptions* writeOptions) | |||
return stream.str(); | |||
} | |||
|
|||
static void mergeLooks(DocumentPtr doc, const XmlExportOptions* exportOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move this to the anonymous namespace at the top and make non-static.
I've encountered shading artifacts in normal mapped MaterialX materials, and upon closer inspection I believe that these are caused by the missing basis orthogonalizations @Tellusim brought to attention in PR #1049. However, instead of using a second normalize(cross(..)) call, I've applied the Gram-Schmidt algorithm.
No description provided.