Apple : Compilation support for MaterialX 1.38.X and 1.39.X #3159
Conversation
|
Filed as internal issue #USD-9837 ❗ Please make sure that a signed CLA has been submitted! |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -285,12 +289,15 @@ HdStMaterialXShaderGen<Base>::_EmitMxSurfaceShader( | |||
| if (outputConnection) { | |||
|
|
|||
| std::string finalOutput = outputConnection->getVariable(); | |||
| #if MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION==38 | |||
There was a problem hiding this comment.
Worth doing "MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION < 39"?
There was a problem hiding this comment.
I don't think any version of MaterialX prior to 1.38.something was ever used or would compile against the USD codebase, so this seemed more direct, but if you prefer the < I have no objections to changing it.
Maybe #if MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION<=38 is better overall?
| const std::string type = mxType | ||
| ? Base::_syntax->getTypeName(mxType) : "vec2"; | ||
|
|
||
| #if MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION==38 |
There was a problem hiding this comment.
I appreciated how most of the other ifdefs are hidden away inside helper functions; is there a way to do that here?
There was a problem hiding this comment.
Sounds good - I'll look to see if this is possibly cleanly.
pxr/imaging/hdSt/materialXFilter.cpp
Outdated
| _GetMxTypeDescription(std::string const& typeName) | ||
| { | ||
| #if MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION==38 | ||
| // Add whatever is necessary for current codebase: | ||
| static const auto _typeLibrary = |
There was a problem hiding this comment.
Minor thing, but I wonder if you could have an ifdef around
"using typename MxTypeDesc = const mx::TypeDesc"
or something and then only define the map once, since aside from the map type everything is character-for-character identical. You'd still need another ifdef around the find/return code below, but that's less likely to be edited than this type map table.
There was a problem hiding this comment.
yep - seems like a good improvement.
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
May you please rebase materialXFilter.cpp ? |
ld-kerley
force-pushed
the
materialx1_39_build_updates
branch
from
68cee6c to
1033d49
Compare
This reverts commit 993ec32. MaterialX has some upstream issues which haven't been quite worked out, yet[1]. It is therefore reasonable to wait with an update in nixpkgs. It also breaks downstream packages like python312Packages.openusd as well as blender[2] [1]: PixarAnimationStudios/OpenUSD#3159 [2]: NixOS#326466 (comment) Signed-off-by: Florian Brandes <[email protected]>
This reverts commit 993ec32. MaterialX has some upstream issues which haven't been quite worked out, yet[1]. It is therefore reasonable to wait with an update in nixpkgs. It also breaks downstream packages like python312Packages.openusd as well as blender[2] [1]: PixarAnimationStudios/OpenUSD#3159 [2]: NixOS#326466 (comment) Signed-off-by: Florian Brandes <[email protected]>
This reverts commit 993ec32. MaterialX has some upstream issues which haven't been quite worked out, yet[1]. It is therefore reasonable to wait with an update in nixpkgs. It also breaks downstream packages like python312Packages.openusd as well as blender[2] [1]: PixarAnimationStudios/OpenUSD#3159 [2]: NixOS#326466 (comment) Signed-off-by: Florian Brandes <[email protected]>
|
bump :p |
|
/AzurePipelines run |
|
Pull request contains merge conflicts. |
This reverts commit 993ec32. MaterialX has some upstream issues which haven't been quite worked out, yet[1]. It is therefore reasonable to wait with an update in nixpkgs. It also breaks downstream packages like python312Packages.openusd as well as blender[2] [1]: PixarAnimationStudios/OpenUSD#3159 [2]: NixOS#326466 (comment) Signed-off-by: Florian Brandes <[email protected]>
Description of Change(s)
MaterialX 1.39.X introduced API changes, this PR allows OpenUSD to be built against either MaterialX 1.38.X or 1.39.X.
Most of the changed arise from an API change around
MaterialX::TypeDescbeing passed by value instead of pointer. The other change is the complete removal of thechannelsattribute.NOTE : The MaterialX versioning PR (#3157) should be merged before this.
Fixes Issue(s)
Fixes compile issues when using MaterialX 1.39.X