Keep a decoded raster out of the state a zoom writes back - #76
Merged
Merged
Conversation
A raster marker cached its decoded image on the marker set itself, and the
marker set is part of the panel state the wheel and pan handlers serialise back
through the model. A decoded canvas serialises to `{}`, so after the echo the
panel held a truthy "cached image" under a matching key, skipped the decode,
and drawImage threw. The model swallows listener errors, so the draw stopped
after the grid: the raster and every marker drawn after it vanished until the
next push from Python.
The decoded image now lives in a per-panel map keyed by the marker set's id,
pruned to the sets still present.
Found in SpyDE's orientation-mapping refine window, where zooming an IPF
heatmap blanked it until the crosshair moved.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #76 +/- ##
==========================================
+ Coverage 91.18% 91.20% +0.02%
==========================================
Files 41 41
Lines 4880 4880
==========================================
+ Hits 4450 4451 +1
+ Misses 430 429 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The cache key, regression coverage, and renderer documentation require updates.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes PlotXY raster blanking after zoom/pan by moving decoded-image caching out of serialized marker state.
Changes:
- Adds panel-local raster caching and pruning.
- Adds a browser regression test.
- Documents the bug fix.
File summaries
| File | Summary |
|---|---|
upcoming_changes/76.bugfix.rst |
Documents the raster zoom fix. |
anyplotlib/tests/test_plotxy/test_raster_zoom.py |
Tests raster visibility after zooming; should also verify markers drawn afterward. |
anyplotlib/figure_esm.js |
Implements panel-local caching; cache keys should include dimensions, and the renderer map/note should be updated. |
Review details
Suppressed comments (1)
anyplotlib/tests/test_plotxy/test_raster_zoom.py:40
- This regression test only counts red raster pixels. The reported failure also prevents every marker after the raster from being drawn when
drawImagethrows, so a future regression could leave the raster visible while still breaking subsequent markers without failing this test. Add a differently colored marker afteradd_rasterand assert its pixels after the wheel round-trip.
coordinates.add_raster(red, extent=(0.0, 1.0, 0.0, 1.0))
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+7418
to
+7433
| function _rasterBitmap(p, id, b64, width, height){ | ||
| if(!p._rasterCache) p._rasterCache=new Map(); | ||
| const cached=p._rasterCache.get(id); | ||
| if(cached && cached.key===b64) return cached.bitmap; | ||
| let bitmap=null; | ||
| try{ | ||
| const bin=atob(b64); | ||
| const bytes=new Uint8ClampedArray(bin.length); | ||
| for(let i=0;i<bin.length;i++) bytes[i]=bin.charCodeAt(i); | ||
| bitmap=new OffscreenCanvas(width,height); | ||
| bitmap.getContext('2d').putImageData(new ImageData(bytes, width, height),0,0); | ||
| }catch(_){ bitmap=null; } | ||
| p._rasterCache.set(id, {key:b64, bitmap}); | ||
| return bitmap; | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
- Right:
image_b64is only the bytes, so the same bytes at 1×4 and 2×2 collide. The cache now matches on the bytes and both dimensions (9658b5c). - New
TestRasterRedecodeschanges a raster's shape under the same bytes and checks the redraw. It fails on the previous commit, where the 4×1 draw came from the stale 2×2 decode.
Comment on lines
+7415
to
+7417
| // The decoded image of raster marker set `id`, re-decoded only when its bytes | ||
| // change, or null if they cannot be decoded. Kept in a per-panel map rather | ||
| // than on the marker set itself, which is serialised state. |
Owner
Author
There was a problem hiding this comment.
- Fixed in 9658b5c. The
_rasterBitmapinsertion moved 62 anchors inFIGURE_ESM.md; I shifted them by the diff hunks and checked every function anchor against its declaration line (no mismatches). _rasterBitmapis now in the 1-D row, and the raster note describes the per-panel cache instead ofms._rasterBmp.- AGENTS.md's "last verified" line count is now 12,425.
The per-panel raster cache matched on the bytes alone, but the same bytes are a different image at a different shape (1x4 against 2x2), so a replaced raster of the same bytes drew from the stale decode. The cache now matches on the bytes and both dimensions. The zoom test also draws a marker after the raster: a raster that throws takes every later marker down with it, which is what made the original bug look like a blank panel. A second test swaps a raster's shape under the same bytes and checks the redraw. FIGURE_ESM.md: the _rasterBitmap insertion moved 62 anchors; they are shifted by the diff hunks and every function anchor checked against its declaration. The raster note now describes the per-panel cache.
Owner
Author
|
On Copilot's suppressed note about
|
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.
Zooming or panning a coordinate axis (
PlotXY) that holds anadd_rasterimage blanked the panel — the raster and every marker drawn after it — until the next update from Python.Cause
The raster drawer cached its decoded image on the marker set itself (
ms._rasterBmp/ms._rasterKey). The marker set is part of the panel state, and the wheel and pan handlers write that state back through the model, whose change listener redraws from the JSON round trip. A decodedOffscreenCanvasserialises to{}, so after the echo the panel held a truthy "cached image" under a matching key, skipped the decode, and calleddrawImage({}). That throws; the model swallows listener errors, so the draw stopped after the grid.Fix
The decoded image lives in a per-panel map keyed by the marker set's id (
_rasterBitmap), pruned to the sets still present. Nothing decoded is stored on serialised state any more.Tests
tests/test_plotxy/test_raster_zoom.py: a red raster on a coordinate axis, three wheel ticks, red pixels read back from the canvases. Fails onmain(0 red pixels after the zoom), passes here.tests/test_plotxy,tests/test_markersandtests/test_plot2d/test_zoom_view_write.pypass (196).Found in SpyDE's orientation-mapping refine window (per-phase IPF heatmaps), where zooming a heatmap blanked it until the crosshair moved; with this file swapped in, the zoomed panel keeps its heatmap, outline, labels and marker.