Skip to content

Add support for scene hierarchy USD importer#2954

Open
Medyan-Naser wants to merge 5 commits intof3d-app:masterfrom
Medyan-Naser:feature/scene-hierarchy-USD-importer
Open

Add support for scene hierarchy USD importer#2954
Medyan-Naser wants to merge 5 commits intof3d-app:masterfrom
Medyan-Naser:feature/scene-hierarchy-USD-importer

Conversation

@Medyan-Naser
Copy link
Contributor

Describe your changes

  • Add scene hierarchy support for USD importer: Implemented GetOrCreateHierarchyNode() method to build a proper vtkDataAssembly hierarchy from USD scene structure
  • Enhanced USD test coverage: Added unit test verification for scene hierarchy nodes and actor attributes

Issue ticket number and link if any

closes #2950

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

@Medyan-Naser
Copy link
Contributor Author

@mwestphal
Please review my changes.

@mwestphal mwestphal requested review from Meakk and mwestphal and removed request for Meakk March 17, 2026 06:49
Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes needed

@Medyan-Naser Medyan-Naser force-pushed the feature/scene-hierarchy-USD-importer branch from 41ff2eb to 05efaf3 Compare March 20, 2026 12:19
@Medyan-Naser Medyan-Naser requested a review from mwestphal March 21, 2026 20:11
@Medyan-Naser Medyan-Naser force-pushed the feature/scene-hierarchy-USD-importer branch from f81b22a to 7a58cfe Compare March 21, 2026 20:13
@mwestphal
Copy link
Member

\ci fast

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I defer to @Meakk

Copy link
Member

@Meakk Meakk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please fix the style and we'll merge it

@Meakk Meakk added ci:main and removed ci:fast labels Mar 22, 2026
@codecov
Copy link

codecov bot commented Mar 22, 2026

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.13%. Comparing base (52eabba) to head (5a74a03).

Files with missing lines Patch % Lines
plugins/usd/module/vtkF3DUSDImporter.cxx 97.43% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2954      +/-   ##
==========================================
- Coverage   97.14%   97.13%   -0.01%     
==========================================
  Files         206      206              
  Lines       16508    16535      +27     
==========================================
+ Hits        16036    16062      +26     
- Misses        472      473       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Meakk
Copy link
Member

Meakk commented Mar 23, 2026

Please check why the coverage misses a line. The test is also failing with VTK 9.3, add a CMake condition like other scene hierarchy tests

@Medyan-Naser
Copy link
Contributor Author

Please check why the coverage misses a line. The test is also failing with VTK 9.3, add a CMake condition like other scene hierarchy tests

Regarding the coverage gap on line 165: this corresponds to return it->second;, which is the cache-hit path in GetOrCreateHierarchyNode. This branch is only exercised when the same USD path is queried more than once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add scene hierarchy for USD importer

3 participants