Repository navigation
fix(event-display): release resources when removing labels - #1051
Merged
EdwardMoyse merged 1 commit intoNov 1, 2026
Merged
Conversation
Track the original camera controls and callback per label ID so removal, replacement and event clearing unsubscribe the listener that owns the old glyph. Dispose removed text geometry/material without touching a same-named physics object. Cover listener ownership, rebuilt object UUIDs, unrelated listeners and repeated event clearing with real OrbitControls and text geometry.
|
🚀 Preview deployed: http://phoenix-pr-1051.surge.sh Built from 7a87ef9. |
Member
|
Thanks! |
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.
Removing or replacing a label can leave its old camera-change callback registered and its detached text geometry/material undisposed. Event clearing can retain these label resources too. This change owns the original controls/callback pair per label ID, releases it on removal/replacement/clear, and disposes the detached glyph. Label lookup stays inside the Labels group so a same-named physics object is preserved.
Four regressions cover removal, replacement with different controls, replacement after an object UUID changes, and repeated event clearing. They also check that unrelated camera listeners survive and replacement text still follows the camera. The unchanged regression file fails all four cases on base
2329146181f72c62285ddf1d3f6450a184adc98band passes with this change.Validation (Node 22.22.1 / Yarn 3.3.1):
yarn workspace phoenix-event-display build— passed.yarn lint— passed.yarn test:ci— core 423 tests passed; Angular 244 tests passed and 7 skipped; root exit 0.yarn docs:coverage— 100%, passed.The resource-lifecycle regressions use actual OrbitControls, Font and TextGeometry in jsdom. Browser/WebGL rendering and measured heap/GPU reclamation were not assessed. The upstream Node 20 build and tests and smoke tests passed on
7a87ef97. PR-title and commit-message checks also passed.Implementation and regression-test preparation used AI assistance.