fix(event-display): stop animations overriding cut and label visibility - #1038
Open
EdwardMoyse wants to merge 1 commit into
Open
EdwardMoyse wants to merge 1 commit into
EdwardMoyse wants to merge 1 commit into
Conversation
Cuts hide objects by setting `visible = false` on the collection's children (SceneManager.collectionFilter), but `animateEvent` treated that same flag as animation-owned state: it hid every generic event object at the start and then unconditionally set `visible = true` once the expanding animation sphere reached it. Its completion handler calls the update with a sphere of infinite radius, so *every* cut-hidden object in the event was forced visible at the end of each animation. The Cut model itself was never touched, so the menu kept reporting the cut as active while the scene no longer honoured it. Reported for a 122411 MeV CaloCluster in CaloCalTopoCluster_xAOD, which reappeared after animating despite an energy cut that excluded it. Objects now record whether they were visible before the animation hid them, and are restored to that state instead of being switched on. The labels group had the same defect in two places. `animateEvent` hid it on start and restored it with an unconditional `true`, so animating with labels toggled off turned them back on; `animateEventWithClipping` hid it on start and never restored it at all, leaving labels hidden after every clipping animation. Both now remember and restore the previous state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🚀 Preview deployed: http://phoenix-pr-1038.surge.sh Built from cea8ba7. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Running an animation makes objects reappear that an active cut had hidden. The Phoenix menu still shows the cut as applied, so the GUI and the 3D scene disagree.
Reproduced with a 122411 MeV
CaloClusterinCaloCalTopoCluster_xAOD, hidden by an energy cut, which comes back as soon as the animation runs:https://phoenixatlas.web.cern.ch/PhoenixATLAS/?file=data%2FBriefings%2FJiveXML_481638_696395260.xml&theme=dark&type=jivexml&config=data%2FBriefings%2Frun481638_evt696395260.json
Cause
Cuts and the animation both drive the same
Object3D.visibleflag, and the animation treats it as if it owns it.SceneManager.collectionFilterhides a filtered object withchild.visible = false.animateEventthen hides every generic event object at the start, and reveals each one as the expanding animation sphere reaches it:It never checks whether the object was visible to begin with. The completion handler then calls that same update with a sphere of infinite radius:
so at the end of every animation every cut-hidden object in the event is forced visible. The
Cutmodel is never touched, which is exactly why the menu still reports the cut as active — the scene has simply stopped honouring it.Fix
Objects now record whether they were visible before the animation hid them, and are restored to that state rather than switched on.
animateEventWithClippingis unaffected by this part: it animates with clipping planes and never touchesvisible.Labels: the same defect, two more places
Noticed while reading the same function, and fixed here too:
animateEventhid the labels group on start and restored it with an unconditionalvisible = true, so running an animation with labels toggled off turned them back on.animateEventWithClippinghid the labels group on start and never restored it at all — labels stayed hidden after every clipping animation.Both now remember and restore the previous state.
Tests
Three regression tests added, each verified to fail without its fix:
should keep objects hidden by a cut hidden through the animation— builds aCaloCalTopoCluster_xAODcollection with one passing cluster and one cut-hidden 122411 cluster, runs the animation to completion, and asserts the cut one stays hidden while the other still animates in. Without the fix:Expected: false, Received: true.should restore the labels after a clipping animationshould leave labels off after an animation if they were toggled offFull
phoenix-event-displaysuite: 402 tests pass. Prettier and eslint clean.🤖 Generated with Claude Code