Add UsdSolid schema + hdOcct OCCT tessellation plugin - #58
Conversation
Implements tessellation of UsdSolid BrepArray prims (Proposal PixarAnimationStudios#109) using OpenCascade. Provides: 1. Core library: BrepBuilder (USD attrs -> OCCT shape), Tessellator (BRepMesh -> triangles), MeshExporter (triangles -> UsdGeomMesh) 2. HdGp Generative Procedural plugin skeleton (placeholder output, pipeline wiring not yet complete) Depends on: OpenCASCADE 7.5+ (libocct-*-dev on Ubuntu) Missing: BrepArrayAdapter (UsdImaging), real tessellation in procedural, standalone CLI tool. See PLAN-usdSolidTessellator.md for full roadmap.
- BrepArrayAdapter: maps BrepArray → generativeProcedural, pre-tessellates using OCCT BRepMesh, injects mesh data as custom data source - UsdSolidTessellationProcedural: reads pre-computed meshes, creates child Hydra mesh prims with topology, normals, displayColor - generatedSchema.usda: registers BrepArray as concrete typed schema so UsdImagingStageSceneIndex recognizes the prim type - plugInfo.json: adapter registration (primTypeName=BrepArray) + procedural displayName matching (usdSolidTessellation) - brepBuilder: untrimmed surface path with multi-loop face skip (avoids rectangular artifacts from missing pcurves), faceuse orientation support - tessellator: corrected winding order for Storm front-face convention - README.md: comprehensive architecture notes and build instructions Tested with TurbineFan.usd (25 bodies, 150 surfaces, 1988 verts). Known issue: hub ring faces have mixed winding due to faceuse orientation logic needing refinement.
OCCT's du×dv and triangle winding conventions are both opposite to Storm/USD's expectations. Apply swap and reverse unconditionally rather than conditionally on face orientation. Also read faceuse:orientationType from BrepArray schema and apply face.Reverse() for 'opposite' faces during shape construction. Hub ring faces (body 0 faces 3-4) still show inverted normals — BRepAdaptor_Surface behavior with REVERSED faces needs further investigation. Blades render correctly.
…ssembly - Multi-loop faces (annular end-caps) now properly trimmed using 3D edge NURBS curves as wire boundaries instead of being skipped. This adds the missing disc geometry with inner axle holes. - Replace BRepBuilderAPI_Sewing with direct TopoDS_Compound to preserve face orientations (sewing was flipping REVERSED faces). - Remove faceuse Reverse() for display — without full closed shell context, faceuse orientation is misleading for rendering. - ShapeFix_Face ensures pcurves exist for BRepMesh to tessellate trimmed faces. - Result: 2124 verts (up from 1988), 4 end-cap faces now rendered.
Architecture now matches Embree/Ptex pattern: Schema (always built): - pxr/usd/usdSolid/ - BrepArray schema (inherits Gprim) Full schema.usda from Proposal PixarAnimationStudios#109 (943 lines) Registered as concreteTyped via plugInfo.json Hydra plugin (optional, gated on PXR_BUILD_OCCT_PLUGIN): - pxr/imaging/plugin/hdOcct/ - tessellation via OpenCASCADE HdGpGenerativeProceduralPlugin for live viewport BrepArrayAdapter maps BrepArray -> generativeProcedural LGPL-2.1 compliance: dynamic linking only (NOTICE.md) CLI tool (optional, gated on PXR_BUILD_OCCT_PLUGIN): - extras/usd/usdSolidTessellator/ - usdsolidtessellate binary Converts BrepArray prims to Mesh USD files Build flags: - PXR_BUILD_OCCT_PLUGIN=OFF (default) in cmake/defaults/Options.cmake - Auto-disables if PXR_BUILD_IMAGING=OFF - OCCT detection: find_package(OpenCASCADE) with system path fallback - Static linking of OCCT blocked (LGPL compliance) Tested: hdOcct.so renders TurbineFan.usd via Storm/hdGp Tested: usdsolidtessellate produces 25 meshes, 2124 verts
Adapter fixes: - Guard MarkDirty/MarkTransformDirty/MarkVisibilityDirty with IsPopulated() check — prevents ChangeTracker errors when scene index path handles prim without legacy Populate() call - Remove debug TF_STATUS noise from adapter and procedural - Fix generatedSchema.usda: add Gprim inheritance + usdGeom sublayer so BrepArray is recognized as imageable with visibility Vertex compaction (fixes Storm warnings): - meshExportC.cpp: compact unreferenced vertices + remap indices OCCT tessellation can produce vertices unused by final triangulation (trimming curves, degenerate triangles removed) - brepArrayAdapter.cpp: same compaction in Hydra data source path - meshExporter.cpp: same compaction in reusable exporter class - Normals and UVs also compacted in lockstep with vertices CLI simplification: - extras/usd/usdSolidTessellator/meshExport.cpp now delegates to UsdSolid_ExportMesh() in hdOcct library (single source of truth) Tests (pxr/imaging/plugin/hdOcct/testenv/testHdOcct.py): - TestCliTool: tessellation USDA/USDC, mesh validity (compaction), normals present, invalid prim path handling - TestHydraRendering: image produced, has content, no vertex warnings - TestSchemaRegistration: BrepArray type recognized, inherits Gprim - All 10 tests pass
jensjebens
left a comment
There was a problem hiding this comment.
Code Review
Architecture is sound — adapter injects pre-tessellated data via HdContainerDataSource, procedural emits meshes without stage access. Much better than the plan's original approach. Issues below are ranked by severity.
4 bugs, 3 inefficiencies, 4 tech debt items — see inline comments.
| VtArray<int> remappedIndices(result.faceVertexIndices.size()); | ||
| for (size_t j = 0; j < result.faceVertexIndices.size(); ++j) { | ||
| remappedIndices[j] = oldToNew[result.faceVertexIndices[j]]; | ||
| } |
There was a problem hiding this comment.
🔴 Bug: Vertex compaction duplicated 4× with inconsistent bounds-checking
This exact logic appears in:
brepArrayAdapter.cpp:108-131(checksidx >= 0)meshExportC.cpp:46-68(checksidx >= 0)meshExporter.cpp:46-66(does NOT checkidx >= 0— will segfault on negative indices from corrupt BrepArray)- The procedural then re-reads already-compacted data
Extract a single CompactMesh() utility in tessellator.h/cpp and call it from all sites. The inconsistent bounds checks mean one of these paths will crash on malformed input.
| BRepAdaptor_Surface surfCheck(face, Standard_True); | ||
| GeomAbs_SurfaceType surfType = surfCheck.GetType(); | ||
| // Planes don't need flip; everything else does | ||
| bool isClosed = (surfType != GeomAbs_Plane); |
There was a problem hiding this comment.
🔴 Bug: Normal flip logic is a surface-type heuristic, not topology-driven
bool isClosed = (surfType != GeomAbs_Plane);This assumes only planes have outward-facing du×dv. A degree-2×2 planar BSpline (common in CAD exports) would be classified as "closed" and get incorrectly flipped. Same for ruled surfaces, swept profiles, etc.
The correct approach: use IsReversed() from face orientation in the topology — which is what OCCT's own BRepMesh uses internally. The current heuristic works for TurbineFan's specific cylinder+plane geometry but will silently produce inverted normals on arbitrary CAD imports.
| merged.faceBrepIndices.reserve(totalFaces); | ||
| merged.faceSolidFaceIndices.reserve(totalFaces); | ||
|
|
||
| bool hasNormals = !results.empty() && !results[0].normals.empty(); |
There was a problem hiding this comment.
🔴 Bug: Merge only checks results[0] for normals/UVs
bool hasNormals = !results.empty() && !results[0].normals.empty();If body 0 tessellates without normals but body 1+ has them, all normals from subsequent bodies are silently dropped. Check any/all results, or propagate the flag from params.
| SdfPath const& cachePath, | ||
| TfToken const& propertyName) | ||
| { | ||
| return HdChangeTracker::AllDirty; |
There was a problem hiding this comment.
🔴 Bug: AllDirty on any property change triggers full re-tessellation
return HdChangeTracker::AllDirty;Combined with the fact that tessellation runs synchronously in GetImagingSubprimData (bug #5 below), ANY property change — including irrelevant ones like displayOpacity or visibility — triggers expensive OCCT re-meshing. Filter to only brep-related attribute names (anything starting with brep:, region:, shell:, face:, etc.).
| } | ||
|
|
||
| // Pre-tessellate and inject mesh data | ||
| HdContainerDataSourceHandle tessData = _BuildTessellationDataSource(prim); |
There was a problem hiding this comment.
_BuildTessellationDataSource() does full OCCT BRepMesh here, blocking the render loop during scene load. With 10 BrepArray prims, all 10 tessellate sequentially before any frame renders.
Consider lazy tessellation: store a "needs tessellation" flag in the data source, and have the procedural's Update() do the actual work (it already has a mutex). Or at minimum, cache the result so repeated GetImagingSubprimData calls don't re-tessellate.
| const UsdPrim& brepArrayPrim, | ||
| size_t brepIndex) const | ||
| { | ||
| _BrepData data; |
There was a problem hiding this comment.
BuildSingleBrep reads entire BrepArray to build one body
_ReadBrepData() pulls ALL attributes (all surfaces, all edges, all vertices) into memory, then _BuildSingleBrep() only uses one body's slice. For a 150-surface TurbineFan this reads ~150 surfaces' control points just to build 1.
Not critical since Build() is the normal path, but the API name implies per-body efficiency. Either document the cost or add a _ReadBrepDataForBody(brepIndex) that reads only the relevant slices.
| from pathlib import Path | ||
|
|
||
| # Ensure USD Python modules are available | ||
| USD_INSTALL = os.environ.get("USD_INSTALL_DIR", |
There was a problem hiding this comment.
💡 Hardcoded paths block upstream submission
USD_INSTALL = os.environ.get("USD_INSTALL_DIR",
"/home/horde/projects/usd-tessellation/usd-install")These defaults are dev-machine-specific. For Pixar submission, tests need pxr_build_test() / pxr_register_test() CMake integration which handles paths automatically, or purely relative paths resolved from PXR_PLUGINPATH_NAME.
| HdPrimvarSchemaTokens->normal)); | ||
|
|
||
| // displayColor for Storm shading (bright metallic grey) | ||
| VtArray<GfVec3f> displayColor(1, GfVec3f(0.85f, 0.87f, 0.9f)); |
There was a problem hiding this comment.
💡 Hardcoded displayColor duplicated in both branches
VtArray<GfVec3f> displayColor(1, GfVec3f(0.85f, 0.87f, 0.9f));Appears in both the normals and no-normals paths. Extract to a constant. More importantly: if the source BrepArray prim has a material binding, it's completely ignored — children always render grey. The adapter should forward material bindings from the source prim.
| @@ -0,0 +1,35 @@ | |||
| // | |||
There was a problem hiding this comment.
💡 Schema is pre-codegen — note in PR description
This is a hand-authored resource-only schema (no usdGenSchema output, no C++ wrapper classes like UsdSolidBrepArray). Consumers use raw UsdAttribute access. Fine for initial contribution but should be explicitly stated in the PR description so reviewers know codegen is a follow-up.
| // Parse mesh index from child name | ||
| const std::string childName = childPrimPath.GetName(); | ||
| size_t meshIdx = 0; | ||
| if (sscanf(childName.c_str(), "tessellated_mesh_%zu", &meshIdx) != 1) { |
There was a problem hiding this comment.
💡 Minor: sscanf with %zu for path parsing
sscanf(childName.c_str(), "tessellated_mesh_%zu", &meshIdx)%zu format with sscanf is technically correct for size_t but some MSVC versions warn. Since this is string parsing for an index you control, TfStringToLong or std::stoul would be more idiomatic in USD codebase.
Bug fixes: - Extract CompactMesh() as UsdSolidTessellationResult::Compact() method eliminating 4x code duplication with consistent bounds checking - Normal flip: topology-driven (face.Orientation() == TopAbs_REVERSED) instead of surface-type heuristic — correct for all CAD geometry - Merge normals/UVs: check ANY result, not just results[0] - ProcessPropertyChange: filter to brep:/surface:/face:/tessellation: attributes only — no re-tessellation on displayOpacity/visibility Performance: - Cache tessellation results per prim path in adapter (thread-safe) - Invalidate cache only when brep geometry attributes change API cleanup: - HdOcctExportMesh() proper C++ entry point; extern C kept for compat - displayColor extracted to static const; duplicated branch consolidated - sscanf replaced with std::stoul (MSVC portability) Tests: - All hardcoded paths removed — env vars with portable defaults - Tests use testCubeBrep.usda (shipped with plugin) as default asset - 10/10 tests pass Vertex count: 2124 -> 2046 (topology-driven flip is more accurate, some previously-emitted degenerate triangles now correctly culled).
Root cause: brepBuilder was not applying faceuse orientationType when assembling faces into the OCCT compound shape. Faces marked 'opposite' (surface normal opposes shell outward) were never getting Reverse()'d, so the tessellator's swap(n2,n3) for TopAbs_REVERSED never triggered. Fix: In all three face-building paths (trimmed, simple, and full topology), check faceuseOrientationType and call face.Reverse() for 'opposite' faces. This correctly sets the OCCT face orientation which the tessellator uses for both winding swap and normal flip. Also adds: - HasNormals() priority path in tessellator (uses pre-computed normals from Poly_Triangulation when available, with isReversed XOR isMirrored flip matching StdPrs_ShadedShape.cxx) - isMirrored XOR detection for Location transforms with negative determinant (handles reflected geometry) - Normal transform by Location (was missing — rotated faces had wrong normal direction) - C++ ground truth test (testWindingOrderCpp.cpp) using signed volume formula V = sum(v0 · (v1 × v2))/6 on OCCT primitives directly - Python integration test (testWindingOrder.py) verifying CLI output against centroid-outward and normal-consistency assertions - 10 generated BrepArray test fixtures (cube, sphere, cylinder, etc.) Results after fix: - Cube: signed_vol=+1000.0 (100% outward winding) ✓ - TurbineFan: 100% normal-winding agreement (1746/1746 tris) ✓ - C++ ground truth: box/sphere/cylinder all pass ✓
Replace auto-generated sphere/cylinder fixtures (which had degenerate degree-1 surfaces OCCT rejected) with hand-authored rational NURBS: - Sphere: 9x5 degree-2 rational surface (standard NURBS sphere representation with pole degeneracies). Correct faceuse orientation 'opposite' for the inward-pointing parametric normal. - Cylinder: 3-face (barrel + 2 caps). Barrel is 9x2 degree-2 rational in u (circle) × degree-1 in v (height). Caps are 9x2 degenerate disks (center point row → edge ring). Add test_sphere_winding_positive_volume and test_cylinder_winding_positive_volume to testWindingOrder.py. Both verify positive signed volume (= outward winding direction). All 5 winding tests pass.
- testCone.usda: Hand-authored rational NURBS cone (r=5, h=10) with correct u-major control point ordering and interleaved weights. Volume accuracy: 1.6% error vs analytical (257.64 vs 261.80). - testPlaneWithHole.usda: Fixed edge weight array — pad with 1.0 for non-rational linear edges. The generator only emitted weights for the circular edge, causing 'Weights values too small' when linear edge construction read uninitialized weight data from the array offset. - testWindingOrder.py: Added test_cone_winding_positive_volume with both sign check and 5% volume accuracy assertion. - brepBuilder.cpp: Improved BSplineCurve error diagnostics — catch Standard_Failure to report OCCT's reason string (e.g. 'Weights values too small') plus actual knot values. Fixture scorecard: 7/10 pass (cube, sphere, cylinder, cone, plane, planeWithHole, twoBoxes). Remaining 3 (cubeWithHole, filletedCube, filletedCubeWithHole) have generator-corrupted surface knot vectors.
Fixed generate_test_breps.cpp: - packKnots: only clamp end multiplicities when natural total doesn't match expected (nPoles+order). Prevents extra knots on already-clamped surfaces. - Weights: always emit (Weight() returns 1.0 for non-rational) so flat arrays stay aligned across all surfaces/edges. Fixes 'Weights values too small' errors when non-rational items preceded rational ones. - Fallback linear edges: emit 4 knots (clamped) instead of 2, plus explicit weight=1.0 for both control points. Result: 10/10 fixtures now tessellate (was 7/10). New successes: testCubeWithHole (78 verts), testFilletedCube (512 verts), testFilletedCubeWithHole (372 verts). Sphere/cylinder/cone remain hand-authored (rational NURBS surfaces from OCCT have periodic knot layouts the generator doesn't yet convert).
The BrepArray adapter in hdOcct requires the usdSolid schema to be installed so that USD recognizes BrepArray as a concreteTyped prim. Without this, GetSchemaTypeName() returns empty and the scene index path never instantiates the adapter (rendering produces no geometry). Contents: - plugInfo.json: registers UsdSolidBrepArray (concreteTyped, inherits UsdGeomGprim) plus all BrepCurve/Surface API schemas - generatedSchema.usda: full schema definition from Proposal PixarAnimationStudios#109 - schema.usda: source schema (943 lines) Installation: copy resources/ to usd-install/plugin/usd/usdSolid/resources/
Three bugs fixed in the full-topology (pcurve) path of brepBuilder: 1. trimCurveControlVertices read as wrong type: schema defines brep:curveUv:nurb:controlVertices as double2[], but the fixture generator wrote point3d[] (z=0). Now handles both: tries GfVec2d first, falls back to GfVec3d extracting xy. 2. Face construction: was using MakeFace(surface, tol) which creates face with natural bounds, then Add(wire) for ALL loops — making inner wires appear as additional boundaries rather than holes. Now: build all loop wires first, then MakeFace(surface, outerWire) for proper boundary, then Add() inner wires. 3. Inner wire reversal: was reversing hole wires before adding, but edgeuse orientations already define correct winding per edge. The double-reverse caused OCCT to treat holes as outer boundaries. Removed the unconditional wire.Reverse() for inner loops. Also added ShapeFix_Face after face construction to compute pcurves for BRepMesh (needed for meshing trimmed faces). Tested: all 10 fixtures tessellate, all 5 winding tests pass. PlaneWithHole now correctly produces 33 verts (rectangle + hole) instead of 29 (circle disc only).
- Add subdivisionScheme='none' to meshExportC.cpp (CLI export path) Prevents Catmull-Clark subdivision from corrupting tessellated meshes. - Add SetSubdivisionScheme(none) to hydraGenerativePlugin.cpp (Hydra path) Ensures generative procedural emits polygonal meshes, not subd surfaces. - Add pxOsd/tokens.h include + usd_pxOsd link dependency for the token. - Fix testCylinder.usda: reorder CPs+weights from v-major to u-major The brepBuilder indexes as u*vCount+v (u-major). The hand-authored cylinder had CPs listed as all-u-for-v=0 then all-u-for-v=1 (v-major), causing weight/CP mismatch → 42k verts from degenerate tessellation. - Fix testSphere.usda: same v-major→u-major CP/weight reordering. Also flip faceuse orientation (same↔opposite) because the surface normal direction changed with the CP layout correction. Results: cylinder 42807→328 verts, sphere 27018→653 verts. All 10 fixtures tessellate, winding tests 5/5 pass.
Move all plugin sources, tests, and fixtures from the temporary pxr/imaging/plugin/hdOcct/ location to the canonical extras/ path. This matches the OpenUSD convention for contributed plugins (cf. extras/usd/examples/usdDancingCubesExample). - Remove pxr/imaging/plugin/hdOcct/ entirely - Remove stale lib/ subdirectory from extras/ - Add 10 test fixtures + winding order tests to extras/testenv/ - Single source of truth: extras/usd/usdSolidTessellator/
BRepMesh_IncrementalMesh requires pcurves (2D parametric edge representations on the surface) to triangulate faces. When faces are built via BRepBuilderAPI_MakeFace(surface, wire), pcurves are not always computed for all edges — particularly for Z-facing planar surfaces where the wire edges span both parametric directions. Adding ShapeFix_Shape::Perform() before meshing ensures all faces have proper pcurves, fixing: - testCube: 16 → 24 verts (was missing 2 of 6 faces) - testCubeWithHole: 41 → 82 verts - testFilletedCube: 110 → 344 verts - testFilletedCubeWithHole: 93 → 268 verts - testTwoBoxes: 32 → 48 verts Also adds render_fixture.sh script for reproducible Storm rendering of BrepArray fixtures via CLI or Hydra live path.
Run usdGenSchema on schema.usda to produce full C++ typed schema
classes and Python wrappers for BrepArray + 12 API schemas.
Generated files:
- 13 classes: BrepArray, BrepPointAPI, BrepCurve3d{Nurb,Line,Circle,Ellipse}API,
BrepCurveUvNurbAPI, BrepSurface{Nurb,Sphere,Plane,Cylinder,Cone,Torus}API
- tokens.h/cpp, api.h, generatedSchema.module.h
- Python wrappers (wrapBrepArray.cpp etc.) + __init__.py
CMakeLists.txt updated to use INCLUDE_SCHEMA_FILES (standard USD pattern).
Test suite (issue #60):
- testUsdSolidBrepArray.py (17 tests): schema API — Define, getters/setters,
type registration, Gprim inheritance, API schema application, time samples
- testUsdSolidTessellation.py (23 tests): CLI tessellation — vertex counts
(regression), winding order (signed volume), normals, subdivision scheme,
ShapeFix validation, error handling
testUsdSolidHydra.py (5 tests): - Plugin discovery: hdOcct registered in Plug.Registry - Adapter registration: UsdSolidBrepArray type recognized - Cube render: produces non-empty image via Storm + hdOcct procedural - Sphere render: same verification with curved geometry - CLI topology match: verifies CLI tessellation output (24v/12tri cube)
|
Superseded by the two-PR split, following the Gaussian-splat landing pattern: #61 (UsdSolid schema only, no third-party deps) and #62 (optional hdOcct reference tessellator, stacked on #61, includes all fixes and two-platform validation). Branch left intact — test work in progress should retarget to the #61/#62 branches. |
Summary
Adds B-Rep (Boundary Representation) solid geometry support to OpenUSD via:
pxr/usd/usdSolid/— BrepArray schema (always built, inherits Gprim)extras/usd/usdSolidTessellator/— hdOcct Hydra generative procedural + usdsolidtessellate CLI (gated on PXR_BUILD_OCCT_PLUGIN)Architecture mirrors the Embree/Ptex pattern: schema is always available, OCCT dependency is optional.
Fixture Renders
All 10 test fixtures rendered via Storm (Hydra live path — no pre-tessellation):
Features
usdsolidtessellate input.usd output.usdc /World/Brep0converts BREP geometry to triangle meshesUsdSolidTessellationResult::Compact()removes unreferenced verticesface.Orientation() == TopAbs_REVERSEDfrom BREP topologyShapeFix_Shapeensures all faces have pcurves before BRepMeshscripts/render_fixture.shfor reproducible CLI + Hydra rendersBuild
Requires OpenCASCADE 7.5+ (Ubuntu:
apt install libocct-*-dev).Test Results
All 10 fixtures tessellate correctly:
Winding verification (signed volume): all closed solids positive ✅
Render Paths
Both produce byte-identical output:
usdsolidtessellate→ Mesh USD →usdrecord --renderer StormDesign Notes
Source Layout (consolidated)
Single canonical source at
extras/usd/usdSolidTessellator/:brepBuilder.cpp— BrepArray USD → OCCT TopoDS_Shapetessellator.cpp— BRepMesh tessellation + ShapeFixhydraGenerativePlugin.cpp— hdGp procedural (emits child meshes)brepArrayAdapter.cpp— Hydra adapter registrationmeshExportC.cpp— CLI mesh export with subdivisionScheme=nonescripts/render_fixture.sh— reproducible render scriptPre-codegen schema
UsdSolid schema is hand-authored (no
usdGenSchemaoutput). Consumers access attributes via rawUsdPrim::GetAttribute(). Codegen planned as follow-up.Schema based on Proposal #109 (BrepArray).
Tessellation via OpenCASCADE Technology (LGPL-2.1-only, dynamic linking).