Skip to content

Add scene hierarchy widget#2915

Open
Meakk wants to merge 19 commits intof3d-app:masterfrom
Meakk:scene-hierarchy
Open

Add scene hierarchy widget#2915
Meakk wants to merge 19 commits intof3d-app:masterfrom
Meakk:scene-hierarchy

Conversation

@Meakk
Copy link
Member

@Meakk Meakk commented Feb 27, 2026

Describe your changes

  • Add a scene hierarchy widget
  • Add a scene hierarchy to the generic importer in case of multi-blocks
  • Add a default scene hierarchy for importers that doesn't provide one
  • Factorize custom VTK events to a single class
  • Fixed ghost blocks created in the OCCT reader

Left for future work:

  • Add a f3d::scene API to retrieve the scene hierarchy
  • Add a f3d::scene API to change a node visibility

Issue ticket number and link if any

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.

@Meakk Meakk self-assigned this Feb 27, 2026
@Meakk Meakk marked this pull request as draft February 27, 2026 14:04
@Meakk Meakk force-pushed the scene-hierarchy branch 3 times, most recently from 0c380e0 to 5ad0007 Compare March 3, 2026 21:43
}

//----------------------------------------------------------------------------
void vtkF3DRenderer::ConfigureColoring()
Copy link
Member

@mwestphal mwestphal Mar 4, 2026

Choose a reason for hiding this comment

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

maybe mention in the doc of this method that it is also responsible for handling visiblity.
It already was before, I know, but now it is way more explicit.

Unless we want to split actor visibility in another method to avoid redoing the whole coloring logic on visibility change.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we want to split because if we turn on visibility, we need to go through the coloring function to know which actor visibility to enable.

Copy link
Member

Choose a reason for hiding this comment

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

why though ? coloring and visibility are two difference concepts, are they not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because ConfigureColoring() switch visibility between the original actor and the colored one.

if (this->SceneHierarchyVisible != show)
{
this->SceneHierarchyVisible = show;
this->UIActor->SetSceneHierarchyVisibility(show);
Copy link
Member

Choose a reason for hiding this comment

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

better to handle that in a simple ConfureSceneHierachy method, similar to ConfigureMetaData above.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't get it

return result;
}

class vtkF3DVisibilityDataAssemblyVisitor : public vtkDataAssemblyVisitor
Copy link
Member

Choose a reason for hiding this comment

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

I dont fully understand the logic of this visitor.

Can you explain (and documment in comment) how it works ? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added comments before each visitor, let me know if it makes more sense.

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.

some changes and questions

@Meakk Meakk changed the title [wip] Scene hierarchy Add scene hierarchy widget Mar 5, 2026
Meakk and others added 4 commits March 9, 2026 17:17
Co-authored-by: Medyan Naser <medyan7.naser@gmail.com>
@Meakk Meakk force-pushed the scene-hierarchy branch from e9df9c6 to bdcc495 Compare March 9, 2026 16:17
@Meakk Meakk marked this pull request as ready for review March 9, 2026 16:18
@Meakk Meakk added the ci:fast label Mar 9, 2026
@Meakk Meakk added ci:main and removed ci:fast labels Mar 9, 2026
@Meakk Meakk force-pushed the scene-hierarchy branch from d289096 to 4d5ba39 Compare March 9, 2026 20:32
@Meakk Meakk force-pushed the scene-hierarchy branch from 4d5ba39 to ad1d481 Compare March 9, 2026 20:41
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 98.77551% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.10%. Comparing base (d76b176) to head (194eb09).

Files with missing lines Patch % Lines
plugins/occt/module/vtkF3DOCCTReader.cxx 94.44% 1 Missing ⚠️
vtkext/private/module/vtkF3DRenderer.cxx 94.73% 1 Missing ⚠️
vtkext/private/module/vtkF3DUIActor.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2915      +/-   ##
==========================================
+ Coverage   96.99%   97.10%   +0.11%     
==========================================
  Files         204      204              
  Lines       16221    16355     +134     
==========================================
+ Hits        15733    15882     +149     
+ Misses        488      473      -15     

☔ 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 Meakk added ci:full and removed ci:main labels Mar 10, 2026
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.

3 participants