USD: import scenegraph instances. #115076
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
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
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
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
EEVEE & Viewport
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
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
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
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#115076
Loading…
Reference in New Issue
No description provided.
Delete Branch "makowalski/blender:usd-scene-instancing-import"
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?
Added support for importing USD instanceable primitives into Blender
as collection instances.
Added a new
USDInstanceReader
class for importing USDinstances as Blender objects that instance collections containing
prototype data.
Extended the
USDStageReader
to read USD prototype prims intocollections that are instanced on the objects created by the instance
readers.
Removed the
Import Instance Proxies
import option.Importing instances is enabled with a new
Scene Instancing
importoption, which is true by default. If this option is off, instances will be
imported as copies (which is the functionality previously enabled by
the
Import Instance Proxies
option).Removed calls to
UsdSkelBindingAPI::Apply()
in the skeleton andblend shape import code, as these calls were unnecessary and were
generating errors when importing instance prototypes with
UsdSkel
data.
Nested instancing and animated prototypes are supported.
Please see the USD documentation on the scenegraph instancing specifications and API.
Discussion
USD supports two approaches to instancing:
This pull request does not handle point instancing. As per the discussion in #96747, point instancers are probably best represented by the
Instance on Points
geometry node, and there is already work in progress on implementing this in #113107.In this proposal, scene graph instances are imported as Blender objects that instance prototype collections. This design was chosen as it seems to be an intuitive approach to referencing parts of the scene graph in arbitrary nodes in the hierarchy, while allowing prototype geometry to be easily accessed, edited and transformed. Moreover, in the future each prototype collection could potentially be associated with a USD file on disk (as per #68933), allowing for more control over scene composition and incremental updates of prototype assets.
However, I'm very open to modifying this design if there are better approaches I might have overlooked.
Testing
When testing this feature, be sure that the
Scene Instancing
option is on. (If this toggle is off, instances will be imported as "real"
Blender objects.)
The Pixar Kitchen Set example includes a
Kitchen_set_instanced.usd
file that may be used for testing.
The attached
nested_instances.usda
andcragInstance.usd
demonstratenested instances and an animating prototype, respectively.
@blender-bot build
It looks like the
convert_to_z_up
processing isn't being triggered for the original geometry because they're not root xforms. So viewing the original "crag" geo you'll see him laying on his back in the scene. Same for the nested torus example as well. [edit] Also the import Scale adjustment is affected too and can be seen in the Kitchen Set example.The nested example also happens to hit unfortunate collection naming collisions. The outer instances use "Instance0" as well as the inner instances etc. which leads to "Instance0.001". But the weird thing is that the ".001" suffix occurs in both lists randomly depending on the order of collection creation. I'm wondering if it would be possible to disambiguate the names somehow?
@ -55,0 +67,4 @@
Collection *coll = BKE_collection_add(bmain, parent, name);
if (coll) {
id_fake_user_set(&coll->id);
I want to say that setting the fake user for collections isn't really necessary but will need Bastien to weigh in. I believe they'll always have a real user because there's always a 'parent' collection available.
I removed the call to add the fake user. Thanks for catching this.
Thanks for the comments! In reply to the first issue:
This is a consequence of the fact that up-axis and scale conversions are implemented by applying global transforms on the root objects of the imported scene hierarchy, of course. But, as far as I can tell, we can't apply these same root transforms to objects in the prototype collections because then we'd get double transforms when the collection is instanced in the scene.
But perhaps there is a way to do this that I'm missing. I'm definitely open to suggestions for improving the workflow.
I'm attaching
instanced_teapots.usda
as an example of instancing in a Z-up scene, for comparison.Unfortunately, name collision of objects imported from USD is fairly common, because we're converting from USD's hierarchical namespace (where nested prim names may be duplicated) to Blender's flat naming structure. I'm not sure how to work around this without deviating from the original names too much.
For what it's worth, I think these particular examples are probably worst-case scenarios for this issue. The instances were created in Houdini using the copy-to-points operator with the default base name "Instance" for all the instances. I think in many real world examples the base names would probably be more descriptive and varied, with fewer collisions.
Yeah, I thought about using the path to the prim in the collection name but Collection names are limited to just 62 bytes and I actually couldn't find a way to fetch what I wanted out from the USD API to prototype it even.
As for scale/z-up, it looks like
usdview
handles it and I'd be surprised if other software didn't also? I don't immediately know where to place the transform for Blender to handle this correctly though.Looks good to me! Just a couple minor comments to consider.
@ -659,3 +659,3 @@
"import_instance_proxies",
true,
false,
"Import Instance Proxies",
Changing this behavior to false is clearly the correct thing to do, but I think the wording of this could be confusing. The way it is currently worded seems to imply that the proxies are something that exists in the asset being imported, and by turning the option off, you are choosing to not import some part of the asset. Some users may be tempted to just enable all the check boxes to ensure they are getting everything, which would of course be silly in this case.
Consider this alternative: "Convert instances to proxies"
Or, maybe even "Convert instances to copies", as I'm not sure proxy is really the right term here either.
Then it is clear that you are choosing to avoid a conversion step rather than omitting some part of the asset.
I like
Convert Instances to Copies
. I agree it's much clearer. Thanks for the suggestion!@ -342,1 +400,3 @@
collect_readers(bmain, root);
collect_readers(bmain, root, readers_);
if (!params_.import_instance_proxies) {
Not so important to me, but just observation that now that scene instances are supported, this reads rather strange, and it may be a good idea to change the name and meaning of the underlying variable.
E.g.,
if (params_.support_scene_instancing) {
You make a good point.
If I understand your idea correctly, we'd still have just one import option related to instancing: "Convert Instances to Copies", as you suggested above.
But the
params_.import_instance_proxies
variable would be renamed toparams_.support_scene_instancing
, which would be set totrue
if the "Convert Instances to Copies" option is off.Is that correct?
Per discussion with Matt today, I will replace the "Convert Instances to Copies" option with a "Support Scene Instancing" option in the UI and import params.
I replaced the
Import Instance Proxies
with a newScene Instancing
option.WIP: USD: import scenegraph instances.to USD: import scenegraph instances.I believe I've addressed the requested changes and have removed the WIP label.
This pull request is ready for further review.
@blender-bot build
@blender-bot build
@blender-bot build
@ -0,0 +10,4 @@
#include "DNA_collection_types.h"
#include "DNA_object_types.h"
#include <iostream>
unused include
Thanks for spotting this!
This is good from my side now. Left 1 minor comment which doesn't require another review.
@blender-bot build
Will trust the other reviewers for the general behavior testing.
Only did a quick code check, LGTM besides a few minor points noted below.
Once these are addressed, no extra review should be needed from me.
@ -55,0 +66,4 @@
Collection *coll = BKE_collection_add(bmain, parent, name);
if (coll) {
Is that depsgraph tagging actually needed? Afaik newly created data does not exist in the depsgraph, so it is automatically fully evaluated on the next update?
In fact, I think it would be much more correct to tag the parent collection, since that one sees its hierarchy modified (and also rather use
ID_RECALC_HIERARCHY
).@ -55,0 +94,4 @@
instance_reader->set_instance_collection(it->second);
}
else {
std::cerr << "WARNING: Couldn't find prototype collection for " << instance_reader->prim_path()
Should use
CLOG
instead of 'raw' prints.@ -479,0 +607,4 @@
pair.first);
if (it == proto_collection_map.end()) {
std::cerr << "WARNING: Couldn't find collection when adding objects for prototype "
Use
CLOG
instead.@blender-bot build