fix/incremental restyle run topology - #66
Closed
thejustinwalsh wants to merge 1 commit into
Closed
Conversation
…ology A fontSize-only restyle invalidates metrics without shaping, but run merging compares layout scalars including font_size: a span crossing value-equality with its neighbor changes the pending run-table topology while prepare_shape retains the stale merged shape. Glyphless run ownership then rejects the mismatch as invalidRequest and the session never recovers. Guard the shape-retention early return with a committed-vs-pending run boundary comparison so any topology change reshapes. Proven by a red-green integration regression (animated span fontSize passing through root equality with a paint difference keeping style segments split), byte-exact replay of the captured production frames including the failing one, and a clean full presentation soak. Raw wasm budget rises ~50 bytes for the comparator; compressed budgets unchanged.
thejustinwalsh
marked this pull request as ready for review
August 12, 2026 03:14
size-limit report 📦
|
This was referenced Aug 12, 2026
thejustinwalsh
added a commit
that referenced
this pull request
Aug 13, 2026
…rement Community feedback root-caused: feeding a raw f32 contentWidth back as the next exact constraint flips line counts at knife-edge widths (39 of 811 swept), producing high-frequency break/unbreak flapping and CPU churn in reactive layout engines; rounding the fed-back width up to the point scale flips zero. The rule lived only in the conformance fixture's roundUpToPointScale; the integration document now states it with the stable Yoga callback pattern and notes the pre-#66 session storms and the 11.17 synchronous-measure outlook.
thejustinwalsh
added a commit
that referenced
this pull request
Aug 20, 2026
Ports the regression scenario from #66, which was written against the pre-rename `packages/text` tree and so could never merge. A colored span whose fontSize animates through value-equality with its root keeps its style segment while the shaping-run table merges across it -- runs coalesce on `same_layout_style`, which compares layout scalars and ignores paint, and `font_size` is one of those scalars. The next tick moves the size off equality and the table splits again under metrics-only invalidation. The engine once retained the merged shape against the rebuilt table and rejected every later frame with invalidRequest, poisoning the session; `shaping_run_topology_stable` was added to stop that and is already on main. The test does not isolate that guard. Forcing `shaping_run_topology_stable` to return `true` -- the pre-fix behaviour -- and rebuilding the shaper leaves the whole suite green, so a later path absorbs the split as well. It lands as a canary over the retained-shape path, and the comment says so rather than claiming a proof it does not deliver. That guard has no other coverage: no Rust test exercises it.
Collaborator
Author
|
Superseded — both halves of this PR are now on The fix ( One finding from the port, recorded in #81 and in the test's own comment: forcing |
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
1 participant
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
A metrics-only restyle could permanently poison an engine session. The presentation soak surfaced it: paragraph 5's animated emphasis
fontSizepasses through the root's value, every subsequent frame fails withinvalidRequest, and the session never recovers.Mechanism
fontSizeinvalidates only metrics, soprepare_shaperetained the committed shape. But shaping-run merging compares layout scalars includingfont_size— when a span's animated size crosses value-equality with its neighbor, the rebuilt pending run table merges (or splits) runs the committed shape doesn't have. Glyphless run ownership then rejects the shape/table mismatch, and because the failure happens after the pending table is built but before commit, every retry hits the same wall.Fix
prepare_shape's retention early-return now also requires the committed and pending run tables to agree on run boundaries; any topology change reshapes.Evidence
three-engine-runtime.test.mjs: animated span fontSize passing through root equality, with a paint difference keeping style segments split while runs merge. Fails withinvalidRequestbefore the fix, passes after.Debugging notes
Two missteps recorded in the log for posterity:
Option::ok_orevaluates its error argument eagerly (a poisoned-site tracker silently lied until switched took_or_else), and the first regression attempt passed because a fontSize-only span at value-equality collapses to a single style segment — full invalidation hides the bug unless paint keeps the segments split.