Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for TGeoTessellated and TGeoVGShape persistency. #14327

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

agheata
Copy link
Member

@agheata agheata commented Jan 10, 2024

The previous implementation required TGeoTessellated shapes to be read attached to a TGeoManager. This removes the limitation, and allows also reading geometry files containing shapes converted to VecGeom corresponding solids.

This Pull request:

Changes or fixes:

  • Restructured the TGeoFacet helper class, eliminating referencing vertices stored in the associated TGeoTessellated object, since this required calling a specific method TGeoTessellated::AfterStreamer to fix all the facet objects. The new version moves all vertex operations from TGeoFacet to the TGeoTessellated class, making the latter readable from a root file even if not connected to a TGeoManager
  • Added persistency to the class TGeoVGShape which interfaces TGeoShape to vecgeom::VPlacedVolume. This allows to write/read geometry files after being converted to VecGeom. Upon reading, a check is made that the current root version was also compiled with VecGeom support, and if this is not the case a Fatal exception is fired.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #14283

The previous implementation required TGeoTessellated shapes to be read attached to a TGeoManager. This removes the limitation, and allows also reading geometry files containing shapes converted to VecGeom corresponding solids. Fixes #root-project#14283
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

Copy link
Member

@bellenot bellenot left a comment

Choose a reason for hiding this comment

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

LGTM, and I trust you 😉

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2204/nortcxxmod.
Running on root-ubuntu-2204-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

Copy link

Test Results

     9 files       9 suites   2d 1h 52m 26s ⏱️
 2 488 tests  2 485 ✅   0 💤 3 ❌
21 360 runs  21 182 ✅ 175 💤 3 ❌

For more details on these failures, see this check.

Results for commit 8f531ea.

@agheata
Copy link
Member Author

agheata commented Jan 11, 2024

The failed tests are unrelated

@agheata agheata merged commit aeffc74 into root-project:master Jan 11, 2024
10 of 15 checks passed
maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024
…14327)

The previous implementation required TGeoTessellated shapes to be read attached to a TGeoManager. This removes the limitation, and allows also reading geometry files containing shapes converted to VecGeom corresponding solids. Fixes #root-project#14283
andresailer added a commit to andresailer/root that referenced this pull request Jan 13, 2024
The function is no longer implemented, see root-project#14327
Can lead to linking issues when undefined symbols are not ignored.
agheata pushed a commit that referenced this pull request Jan 15, 2024
The function is no longer implemented, see #14327
Can lead to linking issues when undefined symbols are not ignored.
iarspider added a commit to iarspider/DD4hep that referenced this pull request Jan 15, 2024
vgvassilev pushed a commit to vgvassilev/root that referenced this pull request Feb 8, 2024
…14327)

The previous implementation required TGeoTessellated shapes to be read attached to a TGeoManager. This removes the limitation, and allows also reading geometry files containing shapes converted to VecGeom corresponding solids. Fixes #root-project#14283
vgvassilev pushed a commit to vgvassilev/root that referenced this pull request Feb 8, 2024
…ject#14339)

The function is no longer implemented, see root-project#14327
Can lead to linking issues when undefined symbols are not ignored.
@ihrivnac
Copy link
Contributor

Hi @agheata ,

With this update, there is removed public function Vertex_t & TGeoFacet::GetVertex(int ivert) which is used in VGM; see:
vmc-project/vgm#15

Would it be possible to restore this function ? Thank you.

@olifre
Copy link
Contributor

olifre commented Mar 28, 2024

This is probably not as relevant as persistence was broken anyways, but since class members were added / removed (and hence, the schema and streamers have changed), I believe these changes do also require a class version bump, no?

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

Successfully merging this pull request may close these issues.

Non-trivial problems with the root persistence for tessellated solids while converting VecGeom shapes.
5 participants