PixarAnimationStudios
Reloading MaterialX layers in Usdview fails
Description of Issue
When selecting reload all layers on a scene referencing a materialx file, the layer won't load correctly
Steps to Reproduce
- Load a file referencing a .mtlx file in usdview
- Select Reload All Layers from the File menu The MaterialX scope will now be empty and you will get an error message along the lines of: Warning: in ReadMaterials at line 2124 of X:\code\USD\pxr\usd\plugin\usdMtlx\reader.cpp -- Failed to create material 'Marble_3D'
I've attached a sample file causing the issue (don't forget to repath the reference to the materialx file) materialx_issue.zip
System Information (OS, Hardware)
Windows 10, amd64, v 21.05
Package Versions
Build Flags
--materialx
Hi @laserallan , we have not seen this error, and cannot, in 22.08 on Linux, reproduce the problem with the test asset provided. Are you still seeing this?
Simpler reproduction case: Load any mtlx file into usdview. In the interpreter window run:
usdviewApi.stage.GetRootLayer().Reload(True)
This is, I'm afraid, because the MaterialX file format plugin is fundamentally broken. The implementation of UsdMtlxFileFormat::Read creates a USD stage, opens the mtlx doc, and calls UsdMtlxRead to create a USD representation of teh mtlx doc. But it does this with USD APIs. SdfLayer::Reload created an SdfChangeBlock around the code that calls UsdMtlxFileFormat::Read. Which means USD APIs are unsafe to use here.
To fix this, UsdMtlxRead must be reimplemented as SdfMtlxRead using only SDF APIs. I would suggest keeping the UsdMtlxRead API around, but just have it call SdfMtlxRead(GetEditTarget(), reader). There would also be a side benefit of allowing the UsdMtlxFileFormat::Read call to avoid a call to TransferContent. And generally faster performance because the whole load would be done inside a change block.
😱 OMG, @marktucker I can't believe we never noticed that, and have even been promoting the pattern of using a UsdStage to import foreign formats with our exemplar usdObj plugin. I think this warrants a humble PSA on usd-interest, but I just want to confer with the team first. Thanks for setting us straight!
I was wondering if there might be some way to "re-enable" notifications within UsdMtlxFileFormat::Read, or enable notifications for just that one stage or something? But I guessed that would be an architectural nightmare deep in the guts of USD. However, if this is a common pattern (and/or one you want to promote, because it would be a pretty nice way to implement things if it worked) maybe it's worth the effort?
Hey @marktucker, we've identified a possible strategy for pulling that off... Alex has an idea for "private layer registries", but we're concerned it may make the system as a whole more difficult to reason about. Given that we additionally concede that this plugin is likely going to see alot more use than we'd originally hoped (going to be awhile if ever that most DCC's emit MaterialX materials as .usd instead of .mtlx), we think the better, if more painful, thing to do is to change it to author via Sdf instead of Usd, for the additional performance benefits.
We may not get to this right away, but @here please advise if this becomes a real workflow impediment.
Hey @spiffmon, we don't think this will be a huge workflow impediment. We have implemented a workaround in Houdini by which we simply ignore any requests to reload MTLX files. This in unfortunate (as you have to restart Houdini to see the effect of updated mtlx files), but is far less disruptive than having the contents of these files disappear.
I've just ran into this in Houdini and was surprised at the behaviour, took me a while to figure out it wasn't an issue on my end. (Also hey @marktucker been a while!)
If I could add a +1 to a SdfReadMtlx implementation, I know it's a bit painful!
It's still on our roadmap, @havess , it just has gotten bumped a couple times with high priority projects.