fix(embed): resolve pixel tokens on export, not just off-wire - #52
Merged
Conversation
Under the Electron binary transport a panel's pixels do not travel in the state: `Plot2D._encode_pixels` writes a "\x00bin:<adler>" change-token and the bytes ride a PLOTBIN frame. `Figure._push` materialises those tokens back to inline base64 only when `_binary_wire()` is false — but that gate reads a process-global env var, which is on in a host app even while the push being made is serialising a snapshot. `_sync_for_export` (added in 0.5.0 so snapshots capture widget positions) re-pushes every panel from inside `_repr_utils._widget_state`, the chokepoint every export goes through. So under a live wire every export wrote unresolved tokens into the panel traits — dangling references, since a snapshot has no PLOTBIN behind it — and, because `_push` rewrites `panel_<id>_json` unconditionally, it also undid any materialisation the caller had done first. The visible casualty was an `add_layer` overlay: `_layerBytes` in figure_esm.js bails on a token (`b64.charCodeAt(0) === 0`), so the layer silently did not draw, while the base image — encoded before the plot is attached to its Figure, so plain base64 and never a token — still did. `_push` grows a `resolve_pixels` keyword and `_sync_for_export` passes it: an export always ships real pixels. The live wire is untouched and keeps its token/PLOTBIN split, which the new tests pin from both sides.
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.
The bug
Under the Electron binary transport (
APL_BINARY_TRANSPORT=1) a panel's pixels do not travel in the state at all:Plot2D._encode_pixelswrites a"\x00bin:<adler>"change-token and the real bytes ride a PLOTBIN frame emitted by_electron._route_change.Figure._pushmaterialises those tokens back to inline base64 — but onlyif not _binary_wire(). That gate reads a process-global env var, which is on in a host app even while the push it is guarding is serialising a snapshot._sync_for_export()(added in 0.5.0 so exports capture widget positions where they are) re-pushes every panel from inside_repr_utils._widget_state— the one chokepointsave_html/to_html/figure_stateall go through. So on a live wire, every export wrote unresolved tokens into the panel traits. A snapshot has no PLOTBIN behind it, so those are dangling references and the pixels are simply lost. Worse, because_pushrewritespanel_<id>_jsonunconditionally, it also silently undid any materialisation the caller had performed beforehand.The visible casualty is an
add_layeroverlay._layerBytesinfigure_esm.jsbails on a token:so the layer silently does not draw — while the base image still does, because
imshowencodes it before the plot is attached to itsFigure(no_raw_pixelsside-table yet) and it is therefore plain base64, never a token. That asymmetry is what made this odd to track down in the wild: the figure looks normal, and only the overlay is missing.Bisected to 0.5.0. Same figure, same three steps:
Plot2D.resolve_pixel_tokens_widget_stateThe fix
_pushgrows aresolve_pixelskeyword;_sync_for_exportpasses it. An export always ships real pixels. The live wire is untouched and keeps its token/PLOTBIN split — resolving there would push megabytes of base64 through the comm on every scrub frame, which is the whole reason the transport exists.Tests
New
anyplotlib/tests/test_embed/test_export_pixel_tokens.py, 12 tests pinning both sides:layers[i].image_b64(the copy_layerBytesactually reads), and thelayer_<id>_b64geom key; no token survives anywhere in the state_sync_for_exportexistsset_data, and returns to tokens after an export6 of the 12 fail on
mainand all 12 pass here. Full suite green: 2007 passed, 6 skipped.