Stop the PNG exporter from deleting text it cannot draw - #316
Merged
Conversation
The native rasterizer bakes its glyphs into src/font.rs because the Rust core avoids FreeType. A character outside that atlas got no glyph AND no advance — so it did not render as a box, it vanished. `Zürich` exported as `Zrich`, a `format="€,.0f"` axis lost its symbol on every tick, and a CJK label exported as blank space, all while the same figure's SVG rendered them correctly. Two changes: - Extend the atlas to the Latin-1 Supplement and Latin Extended-A letters — every language written in the Latin script — plus non-ASCII currency. The generator takes ranges rather than a hand-typed string so the coverage is auditable. 205 -> 424 glyphs, ~48 KB of baked coverage, +4% on the core dylib. - Substitute U+FFFD for anything still unbaked (CJK, Cyrillic, emoji) instead of dropping it. §28 asks that a decision the engine makes on the user's behalf be visible, and a replacement box is the visible form of "this renderer cannot draw that". Zero-width and control characters stay dropped — they have nothing to show. Currency is not incidental: `y_axis(format="€,.0f")` shipped two commits ago, and its symbol was being deleted from every tick of the exported PNG. spec/api/styling.md documented the silent-skip behavior precisely, so it is updated rather than left contradicting the code.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Member
Author
|
Fixed in fdfac0d (on The regression was real and it was mine, from the base branch rather than this one: the probe that decides whether A category column fails the colour test on its very first value, so the probe now gates on entry zero: Local timings at the benchmark's 100k rows, median of 12 runs:
The probe itself went from 7.9 ms to 30 µs on a 500k-row category column. Pinned with a test rather than a timing assertion — an 🤖 Addressed by Claude Code |
Alek99
added a commit
that referenced
this pull request
Jul 26, 2026
* Make customization survive the trip to a file
An audit of xy's customizability across eleven dimensions found one theme
behind most of it: options the WebGL client honors, the static exporters
silently drop. A user styles a chart in a notebook, exports it for a report,
and gets something different — with no warning, and no way to notice until
someone reads the report.
Fixed, each verified against the live client:
- The categorical palette cycled per *trace*, not per series. A box is four
traces and a stem is two, so four box series under a four-color
xy.theme(palette=...) all wore palette[0], and eight box series drew two
colors out of the built-in eight. Adding an outlier to one series repainted
a different one. Marks now take one slot per logical series, and an explicit
color= takes none, matching matplotlib's property cycle.
- x_axis/y_axis format= reached the browser and nothing else: "$,.0f" read
$1,000,000 on screen and 1.0e6 in the PNG. The exporters now share the
client's grammar, and the left gutter grows for labels that need the room.
- A categorical color= channel produced no legend in any export — one row
bearing the trace name and one color, beside marks painted in three.
- text= on hline/vline/bands/threshold/arrow was dropped by the exporters, and
xy.marker() drew nothing at all. Placement is now one helper shared by both,
ported from the client's own label pass.
- Log-axis decades below 1.0 all rendered "0": precision came from the tick
step, which means nothing on a multiplicative axis.
- color=["#ff0000", "#00ff00"] was factorized into categories, sorted
alphabetically, and repainted from the palette. Written-out colors now paint
themselves; named colors stay categorical, since a column of "red"/"green"
is ordinary data.
Added: xy.theme(palette={category: color}) pins colors to labels rather than
positions, so a category keeps its color across marks and across facet panels
where a positional cycle silently recolors whatever is left.
* Add the before/after evidence sheet for the export-parity fixes
* Gate the literal-color probe before it materializes the column
CodSpeed caught a 68.8% regression on
`test_first_payload_scatter_categorical_color`. The cause is in this branch:
the probe that decides whether `color=` is a list of CSS colors ran
`arr.tolist()` unconditionally, ahead of `_factorize_categories` — which goes
to real trouble to identify equal records in Rust precisely so it never creates
N Python objects. So every categorical scatter paid for a full materialization
it did not need. Measured at ~39% of the payload build for a 500k-row column.
A category column fails the test on its very first value, so gate on entry
zero: `["setosa", ...]` now costs O(1) and only an array that already looks
like paint pays for the full scan. Requiring entry zero to match is exactly as
strict as the `all(...)` that follows, which already demands every entry be a
color string, so no input changes classification.
Local timings at the benchmark's 100k rows, median of 12:
origin/main 2.00 ms
before (probe alone was 7.9 ms at 500k)
after 2.03 ms
Pinned with a test rather than a timing assertion: an ndarray subclass whose
`tolist` raises.
* Stop the PNG exporter from deleting text it cannot draw (#316)
The native rasterizer bakes its glyphs into src/font.rs because the Rust core
avoids FreeType. A character outside that atlas got no glyph AND no advance —
so it did not render as a box, it vanished. `Zürich` exported as `Zrich`, a
`format="€,.0f"` axis lost its symbol on every tick, and a CJK label exported
as blank space, all while the same figure's SVG rendered them correctly.
Two changes:
- Extend the atlas to the Latin-1 Supplement and Latin Extended-A letters —
every language written in the Latin script — plus non-ASCII currency. The
generator takes ranges rather than a hand-typed string so the coverage is
auditable. 205 -> 424 glyphs, ~48 KB of baked coverage, +4% on the core dylib.
- Substitute U+FFFD for anything still unbaked (CJK, Cyrillic, emoji) instead
of dropping it. §28 asks that a decision the engine makes on the user's
behalf be visible, and a replacement box is the visible form of "this
renderer cannot draw that". Zero-width and control characters stay dropped —
they have nothing to show.
Currency is not incidental: `y_axis(format="€,.0f")` shipped two commits ago,
and its symbol was being deleted from every tick of the exported PNG.
spec/api/styling.md documented the silent-skip behavior precisely, so it is
updated rather than left contradicting the code.
* Test the numeric core of a formatted label, not the whole label
Review catch, and a genuine crash. The sub-unit log fallback asked "did this
spec collapse the value to zero?" by comparing the FORMATTED label against
zero — but the grammar allows prefixes and suffixes, so a `format="$,.0f"`
axis renders a sub-unit decade as `"$0"`.
float("$0") raises ValueError and takes the whole render down
Number("$0") is NaN, so `=== 0` is false and the collapsed label ships
Two different wrong answers out of the one layer whose job is to keep the
browser and the exporters identical. `_collapsed_to_zero` / `collapsedToZero`
now strip everything that is not part of the number — affixes and group
separators alike — before the comparison, in both runtimes.
Also from review: a `{category: color}` palette that pins all eight built-in
colors leaves nothing distinct for an unmapped category, so it necessarily
reuses one that is already spoken for. That is unavoidable with eight
defaults, but it should not be silent — the unmapped-category warning now says
when the fallback repeats a pinned color.
* Draw unicode spaces as spaces, and stop over-claiming Latin coverage
Second review round, three real catches:
- Substituting U+FFFD for every unbaked codepoint boxed the non-ASCII SPACES
too. Locale-aware number formatting emits NBSP (U+00A0) and narrow NBSP
(U+202F) as group separators, so those arrive through ordinary tick labels —
and whitespace is drawn by its advance, not its shape, so a box is a worse
answer than the space it stands for. Whitespace now maps to the ASCII space
glyph; control and zero-width characters stay dropped; everything else still
boxes.
- "Every language written in the Latin script" was an over-claim. Latin-1
Supplement plus Latin Extended-A carries Western, Central and Northern
European orthographies, but Vietnamese and Romanian's ș/ț live in later
blocks and still box. Both spec statements now say which blocks, and which
languages that leaves out.
- rust-engine.md §2.1 still documented 205 glyphs and silent deletion — its own
text called a substitution glyph with a real advance "the minimum honest
fix", which is what shipped, so the section is rewritten to describe it. The
remaining gap it names (a warning at the Python export boundary) is recorded
as still open rather than quietly dropped.
Also spelled the invisible test characters as escapes: a literal U+200B or
U+FFFD in the source can be stripped by an editor or a copy-paste, silently
reducing the assertion to `_title_png("X") == _title_png("X")`.
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
1 participant
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.
Stacked on #315 (base:
alek/cust-audit-2).The native rasterizer bakes its glyphs into
src/font.rsbecause the Rust coredeliberately avoids FreeType. A character outside that atlas got no glyph and
no advance — so it did not render as a box, it vanished.
Left is
to_png()before:Résumé→Rsum,Zürich→Zrich,Köln→Kln, the CJK tick is blank, and the€is gone from every y tick.The same figure's
to_svg()renders all of it correctly.The currency case is not incidental —
y_axis(format="€,.0f")shipped in #315,and its symbol was being deleted from every tick of the exported PNG.
Changes
written in the Latin script) plus non-ASCII currency.
scripts/gen_font.pynow takes codepoint ranges rather than a hand-typed string, so the coverage
is auditable. 205 → 424 glyphs.
instead of dropping it. §28 asks that a decision the engine makes on the
user's behalf be visible, and a replacement box is the visible form of "this
renderer cannot draw that". Zero-width and control characters stay dropped;
they have nothing to show.
Cost
~48 KB of baked coverage: the core dylib goes 1,170,160 → 1,219,744 bytes
(+4.2%). Measured, not estimated.
CJK and emoji still need
engine=xy.Engine.chromiumfor real glyphs — theatlas approach cannot cover them, and this PR does not pretend otherwise. It
makes the limit visible instead of silent.
Verification
the Python tests compare
to_png()bytes for"X<char>"against"X".(My first probe instrumented
_Cmd.textand wrongly showed everythingpassing — the display list carries the full string either way.)
clean;
tyat the 13-diagnostic baseline;abi_smokepasses.spec/api/styling.mddocumented the silent-skip behavior precisely, so it isupdated rather than left contradicting the code;
spec/design/rust-engine.mdglyph counts refreshed.
Related, not in this PR
tick_label_angleat any angle other than ±90° is also discarded by thisexporter — labels are drawn horizontally and overflow the canvas, because the
display-list text command carries two rotation flag bits rather than an
angle. That is a wire-format change, so it gets its own PR.