fix(core): keep the globalZIndex clone aligned with its source graphic - #2110
Conversation
The clone that beforeSetInteractive hoists into the interactive layer only carried globalZIndex/zIndex, so it lost the transform its source inherits from its ancestors. Drawing was unaffected because beforeDrawInteractive renders the baseGraphic and re-applies the matrix by hand, but the clone's own bounds - and therefore the bounds of the shadow root, _interactive_group and the whole _builtin_interactive layer - ended up short by that transform. pickGroup gates traversal on those bounds, so a pointer position outside the shifted box skipped the entire interactive layer and the hoisted graphic became unpickable even though it was painted on top. Give the clone a postMatrix built from its source's parent global matrix, which composes outside the local transform and reproduces what the ancestor chain contributes.
xuefei1313
left a comment
xuefei1313
left a comment
There was a problem hiding this comment.
The fix addresses translated ancestors, but the clone still diverges from the source for supported transform cases. Please address the inline blocker and extend the regression coverage.
…e transform Addresses review feedback on the first pass: - graphic.clone() already copies the source postMatrix, and overwriting it with the parent global matrix dropped it. The ancestor transform is now composed with the source postMatrix instead of replacing it. - doUpdateGlobalMatrix applies the parent scrollX/scrollY after the local matrix. Since that translation sits to the right of the local matrix it cannot be pre-multiplied directly, so it is conjugated through the source transform (a pure translation stays a translation) and folded into the ancestor part, giving clonePost = P x T(own . scroll) x ownPost. - The matrix is cached on the clone and rewritten in place, so an elevated graphic no longer allocates a Matrix on every render pass. Covered by four added cases: source postMatrix, parent scroll, the two combined with a scaled ancestor and a rotated source, and matrix reuse.
…e source matrix
Measured what the two quantities actually do under a scrolling parent, with
parent {x:100, y:80, scrollX:30, scrollY:20} and a circle at {x:50, y:50}:
ordinary graphic globalTransMatrix.e/f = (180, 250)
globalAABBBounds centre = (150, 230)
painted pixels = (179.5, 249.4)
globalAABBBounds does not carry the parent scroll, while the matrix and the
painted position do. Picking pre-filters on AABBBounds, so matching the matrix
is the wrong target: it moved the clone's bounds away from the source's by
exactly the scroll.
Same setup, clone bounds centre against a source bounds centre of (150, 130):
unpatched (50, 50)
ancestor transform only (150, 130) <- matches
ancestor transform + scroll (180, 150) <- off by the scroll
So the scroll term is dropped and the test now pins that behaviour instead.
Source postMatrix preservation and the cached matrix are unchanged.
|
Correction to my previous comment — I had asserted from reading the code that the paint path drops the parent scroll. I could not actually reproduce that (the elevated graphic never painted in the harness I used, so the measurement was inconclusive), so please disregard that part. Measuring instead surfaced something more relevant, and it changes what this PR should do. Pushed as b442500. What I measured. Parent
Why that matters here. Picking pre-filters on
So the scroll conjugation I added made the clone's bounds worse than leaving it out: it fixed matrix parity while breaking the parity that picking actually consumes. b442500 drops the scroll term, and the test now pins that deliberately, with the reasoning in the comment so nobody re-adds it. Your first point still stands and is unchanged — The mismatch between |
|
Follow-up, and I need to un-retract part of what I said two comments ago. My "the paint path drops the parent scroll" claim was right; the measurement I used to walk it back was broken — the interactive layer renders into its own canvas, and my pixel scan only looked at the first one, which is why the elevated graphic came back as "not painted". Scanning every canvas gives a complete picture. Same setup as before — parent
Two separate things fall out: 1. For an elevated graphic, painting matches the bounds, not the matrix. So the scroll case in your review does hold as an observation — the matrices genuinely differ by 30 — but the conclusion that the interactive-layer bounds stay wrong and picking can still be skipped is not what happens: the bounds line up. I think the mismatch is a red herring for this particular fix, since 2. There is a real scroll inconsistency, but it is in the ordinary path. An ordinary graphic is painted at Reproduction for both tables is the same self-contained page as the issue, plus a per-canvas pixel scan; I can attach it to an issue if useful. Sorry for the noise of the retraction-and-reinstatement — the underlying claim held up, my first attempt to verify it did not. |
xuefei1313
left a comment
xuefei1313
left a comment
There was a problem hiding this comment.
Re-reviewed b442500. The source postMatrix is now preserved, the cached Matrix is reused, and the scroll behavior matches the bounds and paint semantics consumed by interactive-layer picking. Targeted tests, additional parent/source transform update cases, vrender-core TypeScript compilation, and changed-file lint all pass.
2 participants
🤔 This is a ...
🔗 Related issue link
fix #2109
💡 Background and solution
The clone that
InteractiveDrawItemInterceptorContribution.beforeSetInteractivehoists into_builtin_interactiveonly carriedglobalZIndex/zIndex, so it lost the transform its source inherits from its ancestors.Drawing was unaffected, because
beforeDrawInteractiverenders thebaseGraphicand re-applies the matrix by hand:But the clone's own geometry was never corrected, so its
globalAABBBounds— and therefore the bounds of the shadow root, of_interactive_group, and of the whole interactive layer — ended up short by exactly that transform.Picking reads those bounds.
DefaultPickService.pickGroupgates traversal on them:So whenever the real pointer position fell outside the shifted box, the entire interactive layer was skipped and the hoisted graphic was unreachable even though it was painted on top. Graphics that happened to fall inside the shifted box still picked correctly, which made it look intermittent — in a chart, data points near one edge of the plot area stopped responding to hover while the rest worked.
The fix gives the clone a
postMatrixbuilt from its source's parent global matrix.postMatrixcomposes outside the local transform (Graphic.doUpdateLocalMatrix), so it reproduces exactly what the ancestor chain contributes, and rendering is untouched since the draw path still goes throughbaseGraphic.There was already a
// const m = graphic.globalTransMatrix;left commented out at that spot, so this looks like it was on someone's radar.Verified against the reproduction attached to the issue (a graphic with
globalZIndexinside a translated group, occluded by a sibling group):stage.pickgoes from returning the coveringrectto returning thecircle, and the clone's bounds become identical to the source's.📝 Changelog
globalZIndexbecoming unpickable when an ancestor carries a transform.globalZIndex的图元无法被拾取的问题。☑️ Self-Check before Merge
Added
packages/vrender-core/__tests__/unit/render/interactive-graphic-bounds.test.ts, which fails ondevelop(clone lands ate = 150instead of250) and passes with the fix.tsc --noEmitand eslint are clean forvrender-core.