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)