Repository navigation
perf(core): look up the root only for composition elements when sizing a clip window - #6361
Merged
Merged
Conversation
Contributor
Edit accuracy: accurate 271 (base branch 271), smooth 132 of thoseThe gate passes. Quarantined, measured but not gated (0) |
Contributor
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head 5de45c8a
This is exactly equivalent to the old check. findRootCompositionElement only returns elements that match [data-composition-id], so a node without that attribute could never equal it. !== null (rather than a truthy check) keeps an empty data-composition-id="" root working. CI is green.
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.
What
runtimeElementWindowlooks up the root composition only for elements that carrydata-composition-id. Every other timed node skips the lookup.Why
#6354 made the root's clip window open-ended and decided "is this the root" with
findRootCompositionElement. That lookup ran for every timed node, and for every timed ancestorclipChainVisibleAtwalks, on every visibility pass. Each call is one or two document-widequerySelectors, and without[data-root="true"]the first one scans the whole document. So a full pass (everyrenderSeekframe) grew roughly as timed nodes × DOM size. Only adata-composition-idelement can be the root, so other nodes never need the query. Raised in review of #6354 after it had been queued.How
const isRoot = compId !== null && rawNode === findRootCompositionElement(rawNode.ownerDocument);Same result for every node, with no query for the non-composition ones.Test plan
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
timeline.test.ts72/72, including fix(core): a root without data-duration stays on screen after its GSAP timeline ends #6354's root-vs-nested-scene window test.fix(core): a root without data-duration stays on screen after its GSAP timeline ends #6354's two browser tests in
coreRuntimeBrowser.test.ts(root with nodata-duration, clip inside a discovery-stamped wrapper) pass 3 runs in a row with this change.Pre-commit lint, format, typecheck and fallow pass.
Small on purpose: a one-line follow-up to #6354's review, kept apart so it lands on its own.