A tiled image born on a placeholder rendered solid black - #62
Merged
Conversation
Tile mode entered on a flat placeholder frame -- `imshow` of zeros before real
data exists -- derives the fixed quantisation band `raw_min`/`raw_max` from that
placeholder, a degenerate `(0, 0)`, and nothing re-derived it on a later
`set_data`. The two ends of the protocol then disagreed about what `(0, 0)`
means:
* the Python encoder (`_tile_quant_clim`) treats a degenerate band as UNSET
and quantises the tile bytes over the display window instead;
* the JS LUT honoured it literally -- `hMin + raw/255 * (range||1)` over a
zero range put all 256 codes in `[0, 1]`, below any sane display floor.
Every code clamped to `colormap[0]`: a 256-entry black LUT, on the WebGPU shader
path and the Canvas2D blit alike, beside perfectly healthy stats and histograms,
with no warning anywhere. Downstream saw every 8k still open into a black pane.
Python: extract the guard `enable_tile` already had into `_ensure_tile_band()`
and call it from `update_tile_source` -- the single seam every data swap goes
through, so `set_data` and the public `update_tile_source` are both covered. A
band that is already VALID is still never re-derived, so a contrast change keeps
re-windowing in the LUT with no pixel re-encode.
JS: read the band through one `_rawBand()`, shared by `_buildLut32` (both render
paths) and by the colorbar tick placement, which read the band the same way and
was equally wrong. The fallback is gated on `tile_enabled`, because a degenerate
band means two DIFFERENT things: on the plain path `_normalize_image` emits
all-zero bytes and reports `(c, c)`, where the band names the constant value code
0 stands for and the old reconstruction was correct. Falling back unconditionally
turns every constant untiled image black -- it broke seven layer-blend tests.
`test_set_data_stays_identical_between_plain_and_tile` asserted byte-identical
plain/tiled payloads, but only passed BECAUSE of this bug: on unmodified HEAD,
giving it a real base instead of zeros fails at 161 differing bytes, since a
tiled plot quantises over its band and a plain one over the clim. It is now
parametrised over both bases and asserts what the viewer actually sees --
identical within one colormap step.
Also adds `set_tile_band(vmin, vmax)` to pin the band from a range the caller
already knows (a camera's bit depth, a detector's saturation point), so hosts
stop reaching into `_plot2d._state["raw_min"/"raw_max"]`.
FIGURE_ESM.md anchors were already 42 lines stale from an earlier commit, so all
55 were re-measured against the source rather than blind-shifted.
Closes #60
2144 passed, 58 skipped.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62 +/- ##
==========================================
+ Coverage 90.88% 90.90% +0.01%
==========================================
Files 40 40
Lines 4532 4550 +18
==========================================
+ Hits 4119 4136 +17
- Misses 413 414 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Closes #60.
Why
A tiled
imshowthat entered tile mode on a flat placeholder (zeros, before realdata exists) derived the fixed quantisation band
raw_min/raw_maxfrom thatplaceholder — a degenerate
(0, 0)— and nothing re-derived it on a laterset_data. The two ends of the protocol then disagreed about what(0, 0)meant:_tile_quant_clim) treats a degenerate band as unset andquantises the tile bytes over the display window instead;
hMin + raw/255 * (range||1)over a zero rangeputs all 256 codes in
[0, 1], below any sane display floor.Every code clamped to
colormap[0]: a 256-entry black LUT, on the WebGPU shader pathand the Canvas2D blit alike. Stats and histograms bypass the LUT, so they looked
perfectly healthy next to a black pane, and nothing warned. Downstream, every 8k still
opened into a figure laid out on its zeros placeholder showed this.
What
Both ends, as the issue suggested:
enable_tilealready had into_ensure_tile_band()and call it from
update_tile_source, the single seam every data swap goes through,so
set_dataand the publicupdate_tile_sourceare both covered. A band that isalready valid is still never re-derived, so a contrast change keeps re-windowing in
the LUT with no pixel re-encode.
_rawBand(), shared by_buildLut32(bothrender paths) and by the colorbar tick placement, which read the band the same way
and was equally wrong.
One correction to the issue's suggested JS rule: mirroring
_tile_quant_climunconditionally is wrong, because a degenerate band means two different things. On
the plain path
_normalize_imageemits all-zero bytes and reports(c, c)— the bandnames the constant value code 0 stands for, and the old reconstruction was correct.
The unconditional version turns every constant untiled image black; it broke seven
layer-blend tests. The fallback is gated on
tile_enabled.Also adds
set_tile_band(vmin, vmax)— the issue's third ask. The fix alone doesnot let a host drop its live-camera workaround, since that pins a known range rather
than a data-derived one, and a valid band is deliberately never re-derived.
Tests
12 new cases in
test_tiled_imshow.py, including two Playwright ones. Verified tofail without the fix — the browser test reproduces the exact symptom,
{'lo': 0, 'hi': 0, 'mean': 0}.One existing test changed.
test_set_data_stays_identical_between_plain_and_tileasserted byte-identical plain/tiled payloads, but only passed because of this bug:
on unmodified
main, giving it a real base instead of zeros fails at 161 differingbytes, since a tiled plot quantises over its band and a plain one over the clim. It is
now parametrised over both bases and asserts what the viewer actually sees — identical
within one colormap step (measured:
max |diff| = 1). The wire-byte guarantee atconstruction is untouched and still passes.
2144 passed, 58 skipped.Notes for review
api_changeper AGENTS.md: a tiled plot born on aplaceholder now quantises frames over the frame's own range rather than the display
window, matching what
imshowof a large frame has always done. Wire bytes candiffer by a rounding step from the equivalent untiled plot; the displayed image does
not.
FIGURE_ESM.mdanchors were already 42 lines stale from an earlier commit, so all 55were re-measured against the source rather than blind-shifted, and the verified line
count in
AGENTS.mdwas corrected.