USD Import: USD Shapes Support #104707
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#104707
Loading…
Reference in New Issue
No description provided.
Delete Branch "CharlesWardlaw/blender:D16344-usd-shapes-export"
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 patch adds the ability to import USD Shape primitives (Gprims). They are imported as Blender Meshes using the USD API to convert, so that they appear the same as they would in other applications. USD Shapes are important in many workflows, particularly in gaming, where they are used for stand-in geometry or for collision primitives.
Example file with the five types are attached, as is one for testing animated value changes.
@ -1 +1 @@
Subproject commit 4331c8e76c2f42b9fd903716c333d6cdeaa5cebd
Subproject commit 8d04896a130c12eb440b7b9ac33bf9dc152506b5
These commit hashes shouldn't be included in the PR.
@ -0,0 +1,256 @@
Missing license header
Added.
@ -0,0 +8,4 @@
#include "DNA_mesh_types.h"
#include "DNA_meshdata_types.h"
#include "DNA_object_types.h"
#include "DNA_space_types.h" /* for FILE_MAX */
The comment says the header is added
for FILE_MAX
, butFILE_MAX
isn't used here. Make sure the headers included here are actually necessary.Removed.
@ -0,0 +37,4 @@
void USDShapeReader::create_object(Main *bmain, double motionSampleTime)
{
UNUSED_VARS(motionSampleTime);
Change this to
/*motionSampleTime*/
in the function argumentsChanged.
@ -0,0 +52,4 @@
if (read_mesh != mesh) {
/* FIXME: after 2.80; `mesh->flag` isn't copied by #BKE_mesh_nomain_to_mesh() */
/* read_mesh can be freed by BKE_mesh_nomain_to_mesh(), so get the flag before that happens. */
This isn't true anymore, this logic with
mesh->flag
can be removed.Removed.
@ -0,0 +88,4 @@
}
bool USDShapeReader::read_mesh_values(double motionSampleTime,
pxr::VtVec3fArray &positions,
Looks like this is missing clang format here
Reformatted file.
@ -0,0 +93,4 @@
pxr::VtIntArray &face_counts) const
{
const bool is_capsule = prim_.IsA<pxr::UsdGeomCapsule>();
const bool is_cylinder = prim_.IsA<pxr::UsdGeomCylinder>();
No need for these separate boolean variables IMO, just put the check directly inside the if statements.
Changed.
@ -0,0 +130,4 @@
return false;
}
struct Mesh *USDShapeReader::read_mesh(struct Mesh *existing_mesh,
No need for the struct keyword in C++ code
Removed.
@ -0,0 +142,4 @@
return existing_mesh;
}
UNUSED_VARS(read_flag);
Same here with the unused arguments
Changed.
@ -0,0 +171,4 @@
}
}
BKE_mesh_calc_edges(active_mesh, false, false);
Why do you need to calculate the edges if you didn't change anything above?
mesh_from_prim() does not calculate edges; I deferred their calculation to this spot.
Yes, but my point is, shouldn't it be in the
if (active_mesh != existing_mesh) {
where the corner vertices are actually set? Otherwise it looks like it's calculating edges unnecessarily.No, because
active_mesh
was created bymesh_from_prim()
which does not calculate the edges. They need to be calculated either way.@ -0,0 +172,4 @@
}
BKE_mesh_calc_edges(active_mesh, false, false);
BKE_mesh_normals_tag_dirty(active_mesh);
No need to tag normals dirty on a new mesh. You aren't changing the positions here anyway, so it's not the correct update tag if any was actually needed.
Seems to be fine now without-- removed.
@ -0,0 +178,4 @@
}
Mesh *USDShapeReader::mesh_from_prim(Mesh *existing_mesh, double motionSampleTime,
Clang format!
Reformatted file.
@ -0,0 +196,4 @@
existing_mesh, positions.size(), 0, 0, face_indices.size(), face_counts.size());
}
MutableSpan<float3> verts = active_mesh->vert_positions_for_write();
For consistency, change the
verts
name tovert_positions
Changed.
@ -0,0 +212,4 @@
const bool is_capsule = prim_.IsA<pxr::UsdGeomCapsule>();
const bool is_cylinder = prim_.IsA<pxr::UsdGeomCylinder>();
const bool is_cone = prim_.IsA<pxr::UsdGeomCone>();
const bool is_cube = prim_.IsA<pxr::UsdGeomCube>();
Same here with the no need for separate variables
Removed.
@ -0,0 +1,58 @@
Missing license header
Added.
@ -0,0 +33,4 @@
/* Read the pxr:UsdGeomMesh values and convert them to a Blender Mesh,
* also returning face_indices and counts for further loop processing. */
Mesh *mesh_from_prim(Mesh *existing_mesh, double motionSampleTime,
Clang format! Please configure your editor to format when you save the file, it will save everyone's time.
Reformatted file.
@ -0,0 +43,4 @@
void create_object(Main *bmain, double motionSampleTime) override;
void read_object_data(Main *bmain, double motionSampleTime) override;
struct Mesh *read_mesh(struct Mesh *existing_mesh,
No need for these struct keywords in C++ code, even headers.
Removed.
Removed.
@ -0,0 +49,4 @@
const char **err_str) override;
bool is_time_varying();
virtual bool topology_changed(const Mesh * /* existing_mesh */, double /* motionSampleTime */)
Consistency:
/* existing_mesh */
->/*existing_mesh*/
, etc.Changed.
USD Import: USD Shapes Support.to USD Import: USD Shapes SupportIs there any particular reason this is optional? Seems like it would be nice to avoid adding the option unless it's actually helpful (maybe it is!)
Some folks aren't going to want Shapes imported at all, or at all during certain stages in a pipeline. In a future patch I could see removing the option from the main file picker tool panel area, but I think it is necessary in the Operator for pipelines.
@ -0,0 +141,4 @@
MutableSpan<MPoly> polys = active_mesh->polys_for_write();
MutableSpan<MLoop> loops = active_mesh->loops_for_write();
int loop_index = 0;
Declare
loop_index
at the smallest scope possible, right above thefor (int i = 0; i < face_counts.size(); i++) {
loop.Thanks, looks good now. Just one suggestion for an early return and some missed changes from earlier inline.
@ -0,0 +123,4 @@
return false;
}
Mesh *USDShapeReader::read_mesh(struct Mesh *existing_mesh,
struct Mesh
->Mesh
@ -0,0 +138,4 @@
/* Should have a good set of data by this point-- copy over. */
Mesh *active_mesh = mesh_from_prim(existing_mesh, motionSampleTime, face_indices, face_counts);
MutableSpan<MPoly> polys = active_mesh->polys_for_write();
Suggestion here so the reader has fewer things to keep track of at the same time:
@ -0,0 +47,4 @@
void create_object(Main *bmain, double /*motionSampleTime*/) override;
void read_object_data(Main *bmain, double motionSampleTime) override;
Mesh *read_mesh(struct Mesh *existing_mesh,
struct Mesh
->Mesh