perf: round separator drag pane heights to prevent permanent full repaints - #853
Merged
Merged
Conversation
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
Round the pane heights computed during a separator drag to integers.
Why
SeparatorWidget._pressedTouchMouseMoveEventderives the drag distance fromevent.pageY(src/widget/SeparatorWidget.ts:116). On browsers that report sub-pixel mouse/touch coordinates (Firefox/Safari, or any browser under non-100% zoom / fractional HiDPI scaling),pageYis fractional, so the computedreducedPaneHeight/increasedPaneHeightare fractional and get stored viasetBounding/setOptions.A fractional pane height never equals
container.clientHeight, which is always an integer. The guard inDrawWidget.updateImp(src/widget/DrawWidget.ts:75-80) therefore fails on every subsequent update and keeps escalating toUpdateLevel.Drawer: from that moment on, every crosshair mousemove triggers a full main + overlay repaint of both adjacent panes, permanently, until reload.Measured on the dev UMD bundle with a programmatic scenario (chart 800x500, candle pane + VOL pane, 500 bars; 50 crosshair mousemoves; canvas repaints counted via
clearRectinstrumentation, fractionalpageYdrag simulated through the real event path):How
Two
Math.round()calls at the source, where the heights are computed:reducedPaneHeight = Math.round(Math.max(startDragReducedPaneHeight - Math.abs(dragDistance), reducedPaneMinHeight))increasedPaneHeight = Math.round(startDragIncreasedPaneHeight + diffHeight)This is safe because:
Chart._chartBoundingisMath.floor'd), so rounding can only converge with the layout, never diverge from it.layout({ measureHeight: true })call, which the drag handler already performs on every move.bounding.height === container.clientHeighthold again, so normal updates stay atUpdateLevel.Overlay.Verification
pnpm type-check— 0 errorspnpm code-lint(biome) — 0 errorsNotes
pageYfor mouse events, so the fractional coordinate path mainly affects Firefox/Safari and zoomed/HiDPI environments; the benchmark simulates a fractionalpageYto exercise the same code path.setPaneOptions; that API path is intentionally out of scope for this PR.