USD export: prototype invoking Python chasers #108823
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#108823
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "makowalski/blender:test-usd-export-chaser"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This branch implements an experimental prototype for invoking Python
chaser
orhook
functions to extend the USD export functionality.This code is intended to be a limited proof of concept, as part of a
design discussion for implementing Python hooks for USD IO in
#108825.
Added support for registering subclasses of a new
bpy.types.USDHook
type which may implement the hooks as member functions. Supported
hook functions are
on_export()
andon_material_export()
. Also addeddefinitions and Python registration for
USDSceneExportContext
andUSDMaterialExportContext
structs that encapsulate argumentsto these functions.
The hook functions should return
True
on success orFalse
if theoperation was bypasssed or otherwise failed to complete. Exceptions raised
by these functions will be reported in Blender, with the exception details printed
to the console.
On Scene Export
Immediately before saving the USD stage, the C++ export code invokes the
on_export()
Python function defined on all registeredUSDHook
instances.This function is called with an
export_context
argument of typeUSDSceneExportContext
which provides the following accessors to the scene data:
export_context.get_stage()
: returns the USD stage to be saved.export_context.get_depsgraph()
: returns the Blender scene dependency graph.On Material Export
The
on_material_export()
hook function is called for each material that is exported,allowing updates or modifications to the USD material (e.g., shader generation).
The hook is passed three arguments:
export_context
: An instance of the internally defined typeUSDMaterialExportContext
,providing the function
export_context.get_stage()
which returns the stage to be saved.bl_material
: The source Blender material.usd_material
: The target USD material to be exported.Note that the target USD material might already have connected shaders created by the USD exporter or
by other material export hooks.
Python Bindings
This implementation relies on the
boost::python
bindings for USD typesprovided by the USD library. Therefore, the
boost::python
API is usedto invoke the Python hooks instead of the function call mechanism provided
by RNA.
Example Add-On
To test, install the attached
usd_hook_test
add-on. This add-on registersa
USDHookTest
class which implements anon_export()
function to addcustom data to the stage's root layer and an
on_material_export()
functionto create a simple
MaterialX
shader on the USD material.When exporting to USDA, the root layer custom data will be visible at the top of the USDA file:
Note that the
blenderFilepath
value will be empty if the Blender file was not previously saved.The
MaterialX
shader added by the hook will be visible under the Material, with the color set tothe material's viewport color, similar to the following:
@blender-bot build
One important improvement to this example would be the ability to provide the Blender context or other Blender data as arguments to the chaser, in addition to the stage. E.g., it would be nice to support a function definition like
def usd_on_export(stage, context)
Of course, users can currently work around this by accessing the context directly through the
bpy
module, E.g.:However, it would be much cleaner to provide the Blender data as arguments directly.
I'm currently looking into how to invoke Blender's Python bindings in C++, for example, to get the
PyObject
representation of the context.@ideasman42 Is there any example code I could look at for doing something like this, specifically to convert a
bContext*
to aPyObject*
? I see there is an interestingCTX_py_dict_get()
which suggests it might do something like this, but this appears to return null by default.One way to do implement this would be uses the RNA subclassing mechanism that we for extending Blender in general. The C side implementation is a bit arcane, but you should be able to get something working by copying a simple case like
KeyingSetInfo
. The way that works actually does not require dealing withPyObject
types directly, it's all abstracted by RNA.That way you could have something like this in Python:
I would also try to avoid passing context when possible, since that comes with things like a a specific window or editor, or a scene that the user may be editing, while in general USD export should be able to run isolated in a background job.
In terms of user interface, it's of course a bit obscure for a users to know what the name of the chaser is. Easiest could be to just enable all by default if that makes sense,?
And then if there is a need, the UI could have checkboxes to turn chasers on/off, and perhaps these could have a boolean indicating if they should be enabled by default.
The term "chaser" is also a bit obscure to me, I had not heard it before but see it is called like that in maya-usd too. Personally I would find "translator" or "hook" easier to guess its meaning.
Thank you, Brecht. This is very helpful. I'll investigate the approach you suggested. I won't pass the context as an argument, in that case. Providing as arguments the depsgraph and a dictionary that maps USD primitives to Blender objects would probably make sense, but this can be discussed further.
@blender-bot build
@blender-bot build
@brecht Following your suggestion, I've implemented a new
USDHook
class, usingKeyingSetInfo
as an example. I'm fairly new to working withRNA
, so doubtless I missed some things, but this appears to be working on a basic level. As mentioned in the updated description, this is a hybrid approach because I need to use theboost::python
function call API, rather than the one provided byRNA
, but maybe there is a more elegant way to solve this issue.If you need to pass around USD data structures or other custom data structures then it may indeed be required to call directly through the Python API. API documentation will be missing them, but I think that can be solved by adding those methods in
bpy_types.py
with a docstring.@ -0,0 +17,4 @@
using namespace boost;
/* USD Hook Type declarations */
static ListBase usd_hook_types = {NULL, NULL};
This list should be freed on Blender exit.
You could use a
std::list
to make that automatic.I updated the code to use
std::list
, per your suggestion.@ -152,0 +156,4 @@
typedef struct USDHook {
struct USDHook *next, *prev;
/* identifier used for class name, which KeyingSet instances reference as "Typeinfo Name" */
KeyingSet -> USDHook
@ -152,0 +158,4 @@
/* identifier used for class name, which KeyingSet instances reference as "Typeinfo Name" */
char idname[64];
/* identifier so that user can hook this up to a KeyingSet (used as label). */
KeyingSet -> USDHook
I agree with the approach @brecht suggests here, where Hooks are opt out from the UI perspective. In general, if an add on has been enabled by the user, I think they'd implicitly expect it to just run by default.
An even more general solution would be to let hooks register and draw their own properties for the USD operators, with the simplest case being a boolean to turn themselves on or off. I don't think there is a convenient mechanism in Blender to do that now, but if there is really a need for it, it could be done.
Thanks for the review @brecht. I've made the requested changes.
@blender-bot build
Can you add a code example in
doc/python_api/examples/bpy.types.USDHook.py
?Though I think we should perhaps only land this when the implementation is complete enough to support a practical use case,to validate that it works.
I'm also wondering if/how this will work for Hydra. If there is a method to specifically export one individual material, it's an easier fit because then you don't have to worry about how that is part of the scene, and it can work with incremental updates automatically.
@ -0,0 +127,4 @@
FunctionRNA *func;
PropertyRNA *parm;
srna = RNA_def_struct(brna, "USDHook", NULL);
Add with
RNA_def_struct_ui_text
with a description of what this is.I completely agree that this shouldn't land until the implementation is more complete.
Indeed, I agree that supporting hooks for exporting individual materials is essential (this is our use case when converting to MDL in our local branch). I will update this PR with a prototype implementation of material export as well.
@ -0,0 +151,4 @@
RNA_def_property_string_sdna(prop, NULL, "description");
RNA_def_property_string_maxlength(prop, RNA_DYN_DESCR_MAX); /* else it uses the pointer size! */
RNA_def_property_flag(prop, PROP_REGISTER_OPTIONAL);
RNA_def_property_ui_text(prop, "Description", "A short description of the keying set");
keying set -> USD hook
@ -0,0 +110,4 @@
python::call_method<void>(hook_obj,
func_name,
python::object(python::handle<>(depsgraph_obj)),
stage);
Would it make sense to pass in a struct or object with the stage as a field instead of the stage directly?
I'm not sure how much work it would be, but it would have the advantage that when you add extra parameters, they could simply be added as fields on the struct/object.
Maya does something similar where chasers are passed a
factoryContext
, and you dofactoryContext.GetStage()
to get the stage.The reason I suggest that is because if in the future we add more arguments, it changes the function signature, thereby potentially breaking existing add-ons unless they also specify
*args, **kwargs
to capture unknowns. Whereas if they simply accept a singular object as the argument, then new parameters can be passed in on that object, keeping the signature of the function intact.Thanks for the suggestion, Dhruv. What you propose definitely makes sense. I'm guessing falling back on
*args, **kwargs
for arbitrary arguments would be the least amount of work, but a custom struct would be a much cleaner API. We can discuss further. I'm currently finishing up the changes requested by Brecht as well as a simple example of material conversion. I should be done with this in the next couple of days. In the meantime, I'll give some thought to this issue.Dhruv, I've determined that providing a struct as an argument isn't much extra work, so I'll update the prototype to use this approach in the next day or so. Thanks, again, for suggesting this.
That's great to hear, and thank you for doing that. I think it'll help long term with API stability.
That's great to hear, and thank you for doing that. I think it'll help long term with API stability.
Just leaving a general note:I think the approach and add-on API interface here looks really good to me.
Nothing in the code implementation stood out as needing changes either other than the Struct thing mentioned above, and what Brecht mentioned.
I think this will be a really great change, and will help a lot of production pipelines.
@blender-bot build
@DhruvGovil @brecht Just FYI, I've updated the code with the suggested changes, that includes the following:
register_export_hook_converters()
function that must be called at runtime. Maybe there is a better way to do this.I think an open question is what other arguments does it make sense to expose in these structs. For example, for the scene
on_export()
callback, we could provide a mapping between USD prims and the Blender objects they were converted from, which I believe Maya does. One way to do this is to add a function to the struct to query the Blender object from a given USD path. But this might not be essential to add right away.I agree with Brecht that it would be good to try out real-world use cases before merging this in. @DhruvGovil would you be interested in doing such a test? I.e., maybe try implementing a chaser with this system and see if it works for you and what might be missing? No worries if you don't have time to do this.
Yep I’m down for that. I’ll take some time tomorrow and next week to get this branch building locally and then create a little add on.
@BogdanNagirniak, @BrianSavery, do you think the Material export mechanism here will work for Hydra as well? So that the MaterialX add-on implements a USD Hook, and this can then affect both USD export and Hydra?
Hey @makowalski , this looks to be working really well for my test cases.
I did have one suggestion though which is that there is currently no error handling.
I think it would be good if raised exceptions could error out the current export to prevent exports that seem like they finished but may be accidentally missing the results of a Hook if it errored out. It would also help during development of a Hook for TDs to be able to see how they're tripping up.
Thanks so much for testing, @DhruvGovil. That's an excellent point. I'll add the error handling, and I appreciate the suggestion.
Sure, the materialx add-on exports a materialx xml though, not sure if you want to convert that to USD Shade with export as well, but should be easy.
This looks good to me now, I just have one nitpick (besides the error handling that was mentioned).
@ -0,0 +24,4 @@
- ``get_stage()`` returns the USD stage to be saved.
- ``get_blender_material()`` returns the source Blender material being convered.
- ``get_usd_material()`` returns the target USD material to be exported.
I like the context for future proofing, but would still make the Blender and USD material arguments to
on_material_export
, to clarify when they are available.Thanks for the suggestion. I see your point. To clarify, the arguments should be
on_material_export(blender_material, usd_material, export_context)
and remove the
export_context.get_blender_material()
andexport_context.get_usd_material()
accessors. Is that right?Yes, that's what I meant.
Just maybe change the argument order, we generally put the context as first argument.
Also now that I think about it, shouldn't these methods be marked with
@staticmethod
? Otherwise the first argument would be expected to beself
.I'm incrementally implementing the requested changes, but this work is still in progress. My apologies for the delay. I added improved handling for exceptions so far and will address Brecht's comments next. I'll provide an update when this is done.
@blender-bot build
@blender-bot package
Package build started. Download here when ready.
This is looking very close now, just one comment.
@ -0,0 +178,4 @@
python::object format_exception = traceback.attr("format_exception");
python::object formatted_list = format_exception(htype, hvalue, htraceback);
python::object formatted = python::str("\n").join(formatted_list);
std::string err_msg = python::extract<std::string>(formatted);
I'm unsure why there is custom code to print exceptions, I feel like this should either be trivial with the Python C API already, or there is some code to share with the rest of Blender.
Can you use
PyC_Err_PrintWithFunc
or perhaps directlyPyErr_Print()
to print the exception? For other cases we print the stacktrace to the console, I think we can do the same here.Hi Brecht. I appreciate the comment. I was using
PyErr_Print()
originally, but was concerned the user might not notice the error in the console. The work here is to display the full exception error message text as a Blender error, so it would be more obvious. But I certainly see your point that this might be excessive. Perhaps a compromise might be to usePyErr_Print()
to print the exception details and report a short, high level message in Blender. So, maybe line 183 can be:WM_reportf( RPT_ERROR, "An exception occurred invoking USD hook '%s'. Please see the console for details", hook->name);
Would this be acceptable?
Yes, that's fine and consistent with other Python error reporting I think.
@blender-bot package
Package build started. Download here when ready.
I believe at this point all requested changes have been addressed, but please let me know if I've overlooked something.
A package of the build with the latest changes for additional testing is available here:
https://builder.blender.org/download/patch/PR108823
@BrianSavery Just FYI, this pull request is now ready for testing, in case you want to try the material conversion hook. If you read the PR description and take a look at the attached
usd_hook_test.zip
example, that should provide the information you need, but please let me know if you have any questions or run into issues. No worries at all if you don't have time to try this.As per the comment above, this patch build is available for convenience:
https://builder.blender.org/download/patch/PR108823
I'll remove the WIP prefix since that means it's not ready for review, but I think this is.
WIP: USD export: prototype invoking Python chasersto USD export: prototype invoking Python chasersThank you, @brecht!
One remaining question: would it make sense for the hooks to return some sort of code or enum indicating status? For example, the functions could return one of 'Success', 'Bypass' (indicating a no-op) or 'Error' (for functions that choose not to throw an exception for errors).
In our local implementation of import hooks for materials, we found it useful to know when a material converter was a no-op, so that the USD importer could fall back on a default material conversion, but this may be less important for export.
I don't want to over-engineer this, and no sense adding this behavior if it won't be necessary. This could also be deferred and handled in the future by providing a function in the context struct for setting the status code, rather than returning a value. That way we don't necessarily need to make a decision about this now.
@DhruvGovil Do the chasers in Maya set or return some sort of status code? Do you have any opinion on this?
I think it makes sense to have something like that. For operators in Blender there are these return values, not sure if we could just reuse those terms of it it's unclear:
There is no error return value, but there is a
self.reports
for debug/info/warning/error messages. That could be provided here as well if there is a need for something more than exceptions.Simplest would be to just return True or False depending if it should block or bypass, and then use exceptions and reports for errors.
Probably that is good enough, and we can added more complicated return values without breaking the API later.
That sounds good. I will go ahead and implement returning True or False for this PR.
Hi @makowalski.
Could you please merge this with latest
main
? I would like to test export blender materials to MaterialX for Hydra scene delegate.I need to make one more change to this pull request today (return boolean from chaser). I will let you know when this is done.
I just committed #110836, which adds USD preview surface support for Hydra.
If
call_material_export_hooks
is moved intocreate_usd_material
inusd_writer_material.c
, it will be called for both USD and Hydra. Maybe that will make material hooks just work for Hydra, maybe more work is needed.@brecht
call_material_export_hooks()
is now called increate_usd_material()
. Also, the hooks are now expected to return booleans, as previously discussed. Note that the return values aren't actually used in the code at the moment, but supporting such return values in the API is future-proofing in case we need them at some point.Please have a final look and if the changes are okay I can merge this in. Thanks!
Looks good, thanks.
@blender-bot build
@DagerD This is now merged and ready for you to test.