From 8133bb37e8cd55f9e2275db9ece54f872ecad48d Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 17 Aug 2026 23:25:17 -0400 Subject: [PATCH] Key the label projection cache on the viewport, not the canvas size _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) --- src/phosphor/channel_plot.py | 15 +++++++-- tests/test_overlays.py | 65 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/phosphor/channel_plot.py b/src/phosphor/channel_plot.py index 43f198d..e28c56d 100644 --- a/src/phosphor/channel_plot.py +++ b/src/phosphor/channel_plot.py @@ -235,11 +235,20 @@ def _screen_y_projection(self) -> tuple[float | None, float | None]: if subplot is None: return None, None buf = self._buffer - size = self._fpl_widget.size() camera = getattr(subplot, "camera", None) + # Keyed on the subplot's viewport rect rather than the canvas widget's + # size. The two usually move together, but the viewport is what + # map_world_to_screen actually reads, so the widget size was only ever + # standing in for it -- and anything that reshapes the viewport without + # resizing the canvas leaves nothing in the key to invalidate. Nothing + # else would catch it either: the sweep's camera height follows + # n_visible alone, so it does not move on a resize. The labels would + # then keep a projection built for the previous geometry -- stretched + # or compressed against the traces -- until an unrelated change to + # n_visible or the scroll position happened to evict it. + viewport = getattr(subplot, "viewport", None) key = ( - size.width(), - size.height(), + tuple(viewport.rect) if viewport is not None else None, getattr(buf, "n_visible", None), getattr(self, "_z_offset_scale", 1.0), float(camera.height) if camera is not None else None, diff --git a/tests/test_overlays.py b/tests/test_overlays.py index c580e96..2762397 100644 --- a/tests/test_overlays.py +++ b/tests/test_overlays.py @@ -7,6 +7,7 @@ """ import pytest +from PySide6 import QtCore from phosphor.overlays import ( MAX_LABEL_FONT_PX, @@ -220,3 +221,67 @@ def spy(self, *args): _, y_top2, _, y_bot2 = lines[0] assert y_bot2 == y_bot assert y_top2 < y_top + + +# ---- projection cache ------------------------------------------------------ + + +class _Viewport: + def __init__(self, rect): + self.rect = rect + + +class _Subplot: + """Enough of a subplot to probe: a viewport, a camera, and a mapping that + reads the viewport the way fastplotlib's own does.""" + + def __init__(self, rect=(0.0, 0.0, 800.0, 600.0)): + self.viewport = _Viewport(rect) + self.camera = type("Cam", (), {"height": 16.5, "world": type("W", (), {"position": (0.0, 7.5, 0.0)})()})() + + def map_world_to_screen(self, pos): + _, _, _, h = self.viewport.rect + y_offset = self.viewport.rect[1] + ndc_y = (pos[1] - self.camera.world.position[1]) / (self.camera.height / 2) + return 0.0, y_offset + (1 - ndc_y) * 0.5 * h + + +def _probe(subplot): + from phosphor.channel_plot import ChannelPlotWidget + + plot = ChannelPlotWidget.__new__(ChannelPlotWidget) # no canvas needed + plot._subplot = subplot + # A canvas whose size never changes, so that keying on it -- which is the + # mistake this pins -- would hold the cache shut rather than error out. + plot._fpl_widget = type("W", (), {"size": staticmethod(lambda: QtCore.QSize(800, 600))})() + plot._buffer = type("Buf", (), {"n_visible": 16})() + plot._z_offset_scale = 1.0 + plot._projection = None + plot._projection_key = None + return plot + + +def test_projection_follows_the_viewport_not_the_canvas_size(): + """The cache has to key on what the mapping actually reads. + + Keying on the canvas widget's size instead works right up until something + reshapes the viewport without resizing the canvas, at which point nothing + is left to evict the entry -- a sweep's camera height follows n_visible + alone, so it does not move on a resize either. The labels then keep a + projection built for the old geometry and sit stretched or compressed + against their traces indefinitely. + """ + subplot = _Subplot((0.0, 0.0, 800.0, 600.0)) + plot = _probe(subplot) + + first = plot._screen_y_projection() + assert plot._screen_y_projection() == first, "unchanged view should hit the cache" + + subplot.viewport.rect = (0.0, 40.0, 800.0, 520.0) + second = plot._screen_y_projection() + + assert second != first + y0, slope = second + for world_y in (0.0, 7.5, 15.0): + expected = subplot.map_world_to_screen((0.0, world_y, 0.0))[1] + assert y0 + slope * world_y == pytest.approx(expected, abs=1e-6)