Fix standalone density flicker on pan/zoom-out#118
Merged
Conversation
A kernel-less to_html density export ships the full-data overview grid (binned from every source point) plus the retained §28 sample. Two separate issues made the density jump on the slightest drag: 1. `_requestSampleRebin` re-binned the sample whenever the view was not exactly at the home extent, so a pure pan swapped the full-data grid for a sample-derived grid normalized against a much smaller maximum — the density visibly flipped between the overview and the sample. Gate the re-bin on view *span* instead: keep (and restore) the overview for pans and zoom-outs; only a genuine zoom-in re-bins, where the sample adds resolution the overview lacks. 2. The retained sample overlay was gated by `_viewInside(s.win)`, so its individual points vanished the instant the view left the sample's home window (any pan or zoom-out) while the density stayed. Draw the sample whenever it *overlaps* the view (new `_viewOverlaps`); points are positioned in data space and the GPU clips off-screen ones, so the "density + points" look now travels with the view and only drops when panned entirely off the data. Scoped to the standalone (no-kernel) path; the kernel density_view path is unchanged. Adds a headless-Chromium regression test asserting: a pan keeps the overview and spawns no worker, a zoom-in still re-bins, and the sample overlay persists through zoom-out but drops off-data.
Merging this PR will not alter performance
Comparing Footnotes
|
Contributor
|
@greptile review 🙏 |
Greptile SummaryThis PR prevents standalone density flicker during pan and zoom-out. The main changes are:
Confidence Score: 5/5This looks safe to merge. No blocking issues found in the changed code.
What T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "Fix standalone density flicker on pan/zo..." | Re-trigger Greptile |
Alek99
added a commit
that referenced
this pull request
Jul 21, 2026
Greptile P1 (verified): a wheel event landing just before the 90 ms end-of-gesture timer let "end" fire — and the apply rAF, queued earlier, then overwrite it in the view-event coalescer — while the final delta was still pending, so a continuous wheel gesture could finish without an "end" carrying the final committed range (spec §11.3/§15). _queueWheelZoom now clears the end timer whenever it queues a delta; only the applying frame re-arms it. CI, browser lifecycle smoke: the new x-only-zoom latency histogram is lognormal, so at wide canvases the center-sampled lit-pixel probe landed entirely in the near-empty tail and read 0. Give the example a home domain of (0, 250) framing the bulk — which also demonstrates domain-as- home from the pan/zoom spec; the tail stays reachable by panning and the default zoom limit clamps zoom-out at this window. Also chunk the smoke's iframe shell to 16 charts per mount: the five examples this PR adds took the static fleet to 21 concurrent WebGL contexts, past Chrome's 16-context LRU cap, which evicted the earliest slots' contexts mid-probe. CI, benchmark methodology: bench_interaction still passed the removed view_change chart kwarg; drop it — xy:view_change DOM events are always emitted and the harness listens on the chart root. Regenerate the example chart assets, which also picks up the #118 density-flicker fixes merged during the rebase (the committed assets still embedded the pre-rebase client).
Alek99
added a commit
that referenced
this pull request
Jul 21, 2026
Greptile P1 (verified): a wheel event landing just before the 90 ms end-of-gesture timer let "end" fire — and the apply rAF, queued earlier, then overwrite it in the view-event coalescer — while the final delta was still pending, so a continuous wheel gesture could finish without an "end" carrying the final committed range (spec §11.3/§15). _queueWheelZoom now clears the end timer whenever it queues a delta; only the applying frame re-arms it. CI, browser lifecycle smoke: the new x-only-zoom latency histogram is lognormal, so at wide canvases the center-sampled lit-pixel probe landed entirely in the near-empty tail and read 0. Give the example a home domain of (0, 250) framing the bulk — which also demonstrates domain-as- home from the pan/zoom spec; the tail stays reachable by panning and the default zoom limit clamps zoom-out at this window. Also chunk the smoke's iframe shell to 16 charts per mount: the five examples this PR adds took the static fleet to 21 concurrent WebGL contexts, past Chrome's 16-context LRU cap, which evicted the earliest slots' contexts mid-probe. CI, benchmark methodology: bench_interaction still passed the removed view_change chart kwarg; drop it — xy:view_change DOM events are always emitted and the harness listens on the chart root. Regenerate the example chart assets, which also picks up the #118 density-flicker fixes merged during the rebase (the committed assets still embedded the pre-rebase client).
Alek99
added a commit
that referenced
this pull request
Jul 21, 2026
Greptile P1 (verified): a wheel event landing just before the 90 ms end-of-gesture timer let "end" fire — and the apply rAF, queued earlier, then overwrite it in the view-event coalescer — while the final delta was still pending, so a continuous wheel gesture could finish without an "end" carrying the final committed range (spec §11.3/§15). _queueWheelZoom now clears the end timer whenever it queues a delta; only the applying frame re-arms it. CI, browser lifecycle smoke: the new x-only-zoom latency histogram is lognormal, so at wide canvases the center-sampled lit-pixel probe landed entirely in the near-empty tail and read 0. Give the example a home domain of (0, 250) framing the bulk — which also demonstrates domain-as- home from the pan/zoom spec; the tail stays reachable by panning and the default zoom limit clamps zoom-out at this window. Also chunk the smoke's iframe shell to 16 charts per mount: the five examples this PR adds took the static fleet to 21 concurrent WebGL contexts, past Chrome's 16-context LRU cap, which evicted the earliest slots' contexts mid-probe. CI, benchmark methodology: bench_interaction still passed the removed view_change chart kwarg; drop it — xy:view_change DOM events are always emitted and the harness listens on the chart root. Regenerate the example chart assets, which also picks up the #118 density-flicker fixes merged during the rebase (the committed assets still embedded the pre-rebase client).
Alek99
added a commit
that referenced
this pull request
Jul 21, 2026
* Add axis-scoped zoom and chart examples * Implement pan and zoom configuration contract * Format pan and zoom implementation * Reconcile interaction spec with the per-axis pan/zoom contract Rebasing onto main pulled in #122, which moved engineering docs to spec/ and added spec/api/interaction.md — the declared authority on interaction switches and event payloads — describing the pre-#117 contract. That now contradicts the shipped, tested per-axis implementation. Bring the spec and public docs back in line with the code (docs only, no behavior change): - interaction.md: drop the removed `view_change` switch; add the source toggles (wheel_zoom/box_zoom/zoom_buttons/double_click_reset) and a new §2.1 for the non-boolean axis-policy/zoom-limit keys; rewrite the event surface to {ranges, source, axes, phase, interaction_id} with x0/x1/y0/y1 aliases; fix the source vocabulary, the double-click gate (navigation + double_click_reset, not zoom), and the linked-view semantics (a navigation-locked receiver still follows its group). - wire-protocol.md, chart-grammar.md, reflex-shaped-api.md, events-and-callbacks.md, chart-kind-contract.md: the same view_change -> ranges and switch-removal corrections at their altitude. * Fix wheel-gesture end race and post-rebase CI failures Greptile P1 (verified): a wheel event landing just before the 90 ms end-of-gesture timer let "end" fire — and the apply rAF, queued earlier, then overwrite it in the view-event coalescer — while the final delta was still pending, so a continuous wheel gesture could finish without an "end" carrying the final committed range (spec §11.3/§15). _queueWheelZoom now clears the end timer whenever it queues a delta; only the applying frame re-arms it. CI, browser lifecycle smoke: the new x-only-zoom latency histogram is lognormal, so at wide canvases the center-sampled lit-pixel probe landed entirely in the near-empty tail and read 0. Give the example a home domain of (0, 250) framing the bulk — which also demonstrates domain-as- home from the pan/zoom spec; the tail stays reachable by panning and the default zoom limit clamps zoom-out at this window. Also chunk the smoke's iframe shell to 16 charts per mount: the five examples this PR adds took the static fleet to 21 concurrent WebGL contexts, past Chrome's 16-context LRU cap, which evicted the earliest slots' contexts mid-probe. CI, benchmark methodology: bench_interaction still passed the removed view_change chart kwarg; drop it — xy:view_change DOM events are always emitted and the harness listens on the chart root. Regenerate the example chart assets, which also picks up the #118 density-flicker fixes merged during the rebase (the committed assets still embedded the pre-rebase client). * Lock the wheel-gesture terminal event with a regression test The end-timer cancellation moved to the queue path in b642cec, but nothing covered it. Because view events coalesce into one animation frame, a timer surviving into the next delta does not just fire early: the apply frame queued behind it overwrites the pending "end", so the gesture delivers no terminal event at all and consumers that commit on "end" wait on a stale "update". Drive the interleave in the real client behind a deterministic clock and frame queue, placing a delta 1 ms before the deadline, and assert the consumer-visible `xy:view_change` stream still ends in exactly one "end" carrying the committed range. Verified the test fails against the pre-fix client (endCount == 0). Also null the timer handle alongside clearTimeout, matching how 50_chartview.js pairs them. * Contain pan-locked axes to their home window Excluding an axis from pan_axes while zoom could still navigate it was a hollow guarantee: cursor-anchored zoom is a scaling composed with a translation (dcenter = span * (anchor - 1/2) * (1 - f)), so a zoom-in/out chain at two cursor positions panned the "locked" axis exactly, box zoom translated it in one drag, and the default zoom_limits only cap span, not position. Exclusion now means containment. The shared clamp tightens the positional envelope (axis bounds intersected with the home window) for any axis zoom can navigate but pan cannot, so its window never extends past home on any mutation path -- pan, wheel, box, and button zoom, linked ranges, and programmatic updates. Inside the envelope the axis stays live: plain drag includes contained axes, so a zoomed-in window slides within home and pins flush at the extent, while at home magnification it cannot move at all. With navigation off nothing is contained, keeping read-only follower charts free. Spec: interaction.md gains the containment invariant (§2.2); the design doc folds it through the pipeline, precedence, wheel/box, zoom-limit, programmatic, and linked-charts sections, and notes that action-specific axis policy is only independent because of this clamp. The containment probe drives the real client in headless Chromium with pan_axes=("x",): the wheel-chain escape stays inside home on y while free x genuinely escapes, programmatic sets slide back, a zoomed-in drag moves y within home and pins at the edge, and pan_drag end events report the contained axis. Verified fail-first against the pre-change client.
FarhanAliRaza
pushed a commit
that referenced
this pull request
Jul 22, 2026
…egression corpus (#133) * Add rendering-verification design spec: LOD invariants + visual corpus Two workstreams that make renderer churn safe during the refine-and-optimize phase: - Property-based LOD invariant tests (Hypothesis through the real ctypes ABI against O(n) NumPy oracles): M4 extrema-exactness, density/hexbin count fidelity, worker/kernel re-bin equivalence, re-bin mass conservation, NaN containment, offset-encoding error bounds, view round-trips, and tier-decision consistency. - A golden visual-regression corpus of canvas-readback baselines per chart family x interaction state, pinned to one Chromium engine, with committed baselines, a size budget, and diff-triptych CI artifacts. Includes fail-first calibration against the shipped regressions (#118, #87, #79) and a phased advisory-to-hard-gate rollout. * Address rendering-verification spec review: three contract gaps - A5: per-bin oracle fidelity at the implementation's own edges replaces the nested-conservation framing; nesting is checked only on constructed k-times subdivisions so independent nice edges cannot fail a correct implementation. - A9: state the offset-encoding error bound as a formula (one f32 ULP of the offset-relative magnitude, <= 2^-23 x max(window span, |value - offset|)) instead of an undefined epsilon. - 4.2a: the settled signal the corpus waits on does not exist in the client today; specify it as a Phase-3 deliverable — a CDP-observable monotonic settle counter gating on no pending kernel round-trips, worker re-bins, or scheduled frames. * Keep the visual corpus binary-free: manifest baselines, no committed PNGs Replace committed golden PNGs with a committed text manifest of per-tile signatures (hash + per-channel means) and thresholds. The repo uses no Git LFS and should not start for regenerable data, and committed PNGs under tests/ would ship in the sdist. Images exist only as on-demand CI artifacts rendered from the PR merge-base; regen output is git-ignored, and the check fails if a PNG appears under tests/visual/. The comparison metric is restated to be computable from the manifest alone, with pixel-level diffing demoted to a CI-artifact diagnostic.
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.
Problem
In a kernel-less
to_htmldensity export, the density visibly flickered on the slightest drag — swapping between the full-data overview grid and the retained sample. Two distinct causes:Color/brightness flip on pan.
_requestSampleRebinre-binned the sample whenever the view wasn't exactly at the home extent, so a pure pan swapped the full-data overview grid for a sample-derived grid normalized against a much smaller maximum. The two normalize differently, so the density jumped.Points vanishing on pan/zoom-out. The retained sample overlay was gated by
_viewInside(s.win), so its individual points dropped out the instant the view left the sample's home window, while the density stayed — an inconsistent "density loses its grain" effect.Fix
Gate the re-bin on view span, not position: pans and zoom-outs keep (and restore) the full-data overview; only a genuine zoom-in re-bins, where the sample actually adds resolution the overview lacks.
Add
_viewOverlapsand draw the sample whenever it overlaps the view. Points are positioned in data space and the GPU clips off-screen ones, so the "density + points" look now travels with the view and only drops when panned entirely off the data.Scoped strictly to the standalone (no-kernel) path; the kernel
density_viewpath is unchanged.Tests
Adds
tests/test_density_pan_no_rebin.py— a headless-Chromium regression test asserting:Render smoke + related suites pass; bundles regenerated via
node js/build.mjs; ruff/format clean.