Skip to content

Key the label projection cache on the viewport, not the canvas size - #8

Merged
cboulay merged 1 commit into
mainfrom
fix/projection-cache-viewport
Aug 18, 2026
Merged

Key the label projection cache on the viewport, not the canvas size#8
cboulay merged 1 commit into
mainfrom
fix/projection-cache-viewport

Conversation

@cboulay

@cboulay cboulay commented Aug 18, 2026

Copy link
Copy Markdown
Member

Channel labels could end up stretched or compressed against their traces after a window resize, and stay that way.

The bug

ChannelPlotWidget._screen_y_projection caches the world→screen-y mapping that ChannelLabelOverlay places its text with. The mapping is computed from subplot.viewport.rect, but the cache was keyed on self._fpl_widget.size() — the canvas widget. The widget size was only ever standing in for the viewport.

The two normally move together, so this held up in the common case. It breaks whenever something reshapes the viewport without resizing the canvas: there is then nothing left in the key to evict the entry. The camera terms cannot cover for it either — SweepWidget._apply_auto_scale derives camera.height from n_visible alone, so it does not move on a resize.

The overlay then keeps a projection built for the previous geometry indefinitely, until an unrelated change to n_visible or the scroll position happens to change the key.

The fix

Key on viewport.rect — the thing map_world_to_screen actually reads. getattr-guarded, consistent with the camera terms alongside it.

Nudging the viewport 40px with the canvas size held fixed, before and after:

settled           viewport=(1.0, 24.0, 898.0, 529.0) err=0.00px
viewport nudged   viewport=(1.0, 64.0, 898.0, 449.0) err=36.36px   <- before
viewport nudged   viewport=(1.0, 64.0, 898.0, 449.0) err=0.00px    <- after

Verification

Ground truth came from comparing where the overlay paints against where traces actually land in the wgpu framebuffer, rather than re-deriving the same arithmetic. Across canvas heights 306–1366 px, QT_SCALE_FACTOR 1 / 1.25 / 1.5 / 2, and live showFullScreen / showMaximized / showNormal / resize cycles driven through the Qt event loop: glyph-to-trace offset is a constant −0.5 px and mapping error is 0.00 px.

tests/test_overlays.py::test_projection_follows_the_viewport_not_the_canvas_size pins it. The stub deliberately carries a fixed-size _fpl_widget, so reverting the fix fails on the stale projection value rather than on an AttributeError — confirmed by stashing the fix and re-running. Full suite: 128 passed.

Not addressed here

A separate, cosmetic issue turned up while chasing this and is left alone: MIN_LABEL_FONT_PX = 8 pins fm.height() at 10 px while the row span tracks canvas height, so once the span drops below ~10 px, step (overlays.py:163) goes to 2 or 3 and the chips grow to 1–2 rows tall.

canvas row span step chip/row
1066 px 14.9 1 0.67
666 px 9.1 2 1.10
386 px 5.0 2 1.99

At step 1 each label sits cleanly on its own trace. At step 2–3 the chips merge into a near-solid column straddling neighbours, and which channels get labeled changes on every resize. That is aligned-but-ambiguous rather than misaligned — a leader tick from chip to trace, or suppressing labels that cannot fit their row, would be the fix.

🤖 Generated with Claude Code

_screen_y_projection caches the world->screen-y mapping the channel-label
overlay places its text with, but keyed that cache on the canvas widget's
size while the mapping is computed from the subplot's viewport rect. The
two usually move together, so the widget size stood in for the viewport --
until something reshaped the viewport without resizing the canvas, at which
point nothing was left to evict the entry. The camera terms could not cover
for it either: a sweep's camera height follows n_visible alone, so it does
not move on a resize.

The labels then kept a projection built for the previous geometry and sat
stretched or compressed against their traces indefinitely, until an
unrelated change to n_visible or the scroll position happened to change the
key. Nudging the viewport by 40px with the canvas size held fixed put them
36px out and left them there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cboulay
cboulay merged commit 5c49082 into main Aug 18, 2026
11 checks passed
@cboulay
cboulay deleted the fix/projection-cache-viewport branch August 18, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant