perf(context): skip redundant transform refreshes in semantic snapshots - #577
Merged
dli7319 merged 6 commits intoSep 15, 2026
Merged
Conversation
Contributor
Author
|
this is actually just removing one line with a bunch of tests |
buildSemanticTree already calls scene.updateMatrixWorld(true) before it traverses, so the extra object.updateMatrixWorld(true) inside createSemanticNode re-walked each node's subtree again for every semantic node. getWorldPosition already refreshes the ancestor chain it needs, and bounds computation refreshes its own subtree, so the forced per-node refresh was pure repeated work. Keeps the scene-wide refresh at the start of the snapshot and leaves getWorldPosition in place, so custom overrides such as UIElement's presentation-aware getWorldPosition stay authoritative. Adds a colocated test covering positions and bounds under transformed parents, freshness after later movement and scale changes, the absence of a forced per-node subtree refresh, and UI elements whose logical and presentation transforms differ.
salmanmkc
force-pushed
the
salmanmkc-context-transform-reuse
branch
from
September 12, 2026 05:06
5e87df8 to
46a9a52
Compare
Collaborator
|
Can you fix the merge conflict? |
salmanmkc
force-pushed
the
salmanmkc-context-transform-reuse
branch
from
September 15, 2026 16:57
ec6c096 to
71feaf6
Compare
Contributor
Author
Yup done! |
salmanmkc
force-pushed
the
salmanmkc-context-transform-reuse
branch
2 times, most recently
from
September 15, 2026 17:01
90be083 to
93ce92a
Compare
Collaborator
|
Thanks. |
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.
Description
buildSemanticTreealready callsscene.updateMatrixWorld(true)once before it traverses the scene. Despite that,createSemanticNodecalledobject.updateMatrixWorld(true)again for every node it visited, which forced a full recursive refresh of that node's subtree. In a deep hierarchy the same descendants get refreshed once per ancestor, so the cost grows with depth while the matrices are already current.This removes that per-node call. The scene-wide refresh at the start of the snapshot stays exactly where it is, and
object.getWorldPosition(tempPosition)stays too, since it performs the ancestor walk the node actually needs. Bounds are unaffected becausegetObjectBounds/getUIObjectBoundsrefresh their own subtrees, andBox3.expandByObjectupdates each object it visits.Keeping
getWorldPositionmatters for correctness, not just style.UIElementoverrides it so a UI node reports the world position of its registered uikit presentation object rather than its logical transform. ReadingmatrixWorlddirectly instead, as a first pass did, silently reports the wrong position for those elements once the logical and presentation transforms diverge.There is no caching across frames or snapshots, no change to camera or XR pose timing, and no public API change.
On performance, this is a redundant-work cleanup rather than a headline win. An isolated synthetic benchmark over a deep semantic snapshot showed a small, repeatable reduction in per-snapshot build time. Semantic context polling defaults to 3000 ms, so this is not expected to move frame rate in a typical app.
Covered by a new colocated
SemanticTreeBuilder.test.ts:getWorldPositionoverride remains authoritativeUICard/UIPanelregistered through the presentation helper under a separate render root, where the logical and presentation transforms differThree of those cases fail against the behavior they guard, so they are live regressions rather than restatements.
Verified in Chromium as well as in unit tests: the same page was loaded against a build with and a build without this change, and the resulting semantic trees, visible-object contexts, and bounds were byte-identical across three snapshots taken before and after moving, rotating, and scaling both plain meshes and spatial UI.
Type of Change
Media / Screen Recordings & Screenshots (If Applicable)
Checklist