Skip to content

Polar audit round: GPU buffer leak, mobile chrome, wedge vertex cost, CodSpeed coverage - #380

Merged
Alek99 merged 1 commit into
alek/polar-axesfrom
claude/wind-rose-polar-fixes-vrohh2
Jul 30, 2026
Merged

Polar audit round: GPU buffer leak, mobile chrome, wedge vertex cost, CodSpeed coverage#380
Alek99 merged 1 commit into
alek/polar-axesfrom
claude/wind-rose-polar-fixes-vrohh2

Conversation

@Alek99

@Alek99 Alek99 commented Jul 30, 2026

Copy link
Copy Markdown
Member

Stacked onto #370 (alek/polar-axes) — merge here and the fixes land on that PR.

Nine reported failures across the polar surface and the responsive chrome it shares with every other chart, plus the CodSpeed gap that let one of them ship.

P1 — repeated data updates leaked GPU buffers

_destroyTraceResources walked a hand-kept list of geometry buffer names, so every rebuilt trace — a state-driven data update, an append that could not patch in place, an animated spec swap — orphaned its style, direct-RGBA colour, stroke, corner-radius, LOD-blend and dashed-line-length buffers.

All three teardown paths (trace, drill window, sample overlay) now read one shared TRACE_GPU_BUFFERS list in the leaf module. tests/test_trace_buffer_lifecycle.py pins that every <name>Buf field assigned anywhere in js/src appears in it, so adding a channel buffer cannot silently reintroduce the leak.

P1/P2 — mobile Wind Rose chrome

  • Title clipped ~10 px. The browser wraps a long title (white-space: pre-line) while layout measured one line, so the first line went above the canvas. Titles now wrap at one shared width — the canvas minus the authored horizontal gutters, resolved before the title band because the measured left gutter depends on the plot height — in all three renderers, and the browser caps the title element at that same width so the DOM cannot wrap into more lines than were reserved. Single-line titles are byte-identical.
  • Legend over the disc. A disc inscribed in its rect leaves no free corner, so a default upper right legend covered the north-east sectors and part of the outer radial label. A polar figure with a legend now reserves a gutter off the canvas edge before the disc is fitted — _POLAR_LEGEND_ROOM (96 px) on the side loc names, or _POLAR_LEGEND_BAND (64 px) beneath the disc at compact widths — and the legend places and bounds itself in that box. An authored anchor (still resolved against the plot) or four-tuple padding reserves nothing.

P2 — 50k polar bars fell off a cliff at 194 vertices per wedge

Subdivision is now span-proportional and recorded as a formula:

segments(span) = clamp(ceil(96 · |span| / turn), 2, 96)

Sagitta is quadratic in the per-segment angle, so holding span / n fixed holds the flattening bound: a 16-sector wind-rose sector needs six segments (14 vertices) rather than the full-turn worst case of 96 (194). A full turn still uses 96, so nothing gets coarser.

Not the view-adaptive count §28 would need a recording for — the input is the authored angular width (the scalar width on the compact bar path, the widest x1 − x0 in the trace on the four-edge path, measured once at build and cached), so zoom, resize and export cannot change it.

P2 — polar axis options that shipped and were ignored

xy.theta_axis/xy.r_axis now refuse minor_tick_values, minor_style, tick_label_min_gap, tick_label_anchor and the collision spellings of tick_label_strategy (auto/hide/rotate/stagger/preserve), each naming the control that does work. off and none stay honoured.

Refused on the documented surface rather than at payload build, deliberately: xy.pyplot's polar projection assembles its axis from a property bag it does not own (every Axes carries an rcParam minor_style; minorticks_on() and tick_params(ha=) add more), so refusing there would turn projection="polar" into an error over defaults nobody authored. The adapter drops them instead — what all three renderers already do with the values — and that drop is recorded in spec/matplotlib/compat.md.

Two are now honoured instead: theta_axis(format=".0f°") wins over the built-in degree/radian text (the angular branch used to run first and overwrite it), and r_axis(margin=) keeps the outer pad the centre-origin default otherwise drops.

P2 — radial time axes

A time radius was pinned to the centre-origin default, i.e. epoch zero, so twelve consecutive days out of ~1.7e12 ms occupied 0.0006 % of the radius and read as a solid disc edge. A time radius now keeps its ordinary padded extent. Signed-radius behaviour (a position on a range that includes it, never a mirrored direction) is normative in the spec and stated in the r_axis docstring.

P2 — compact colorbars were unlabelled

_positionColorbar hid every tick and the title, leaving a bare gradient. It now keeps the two extreme tick labels and the scale title, and reserves the room they need; only the interior ladder and the text-free minor ticks drop.

P2 — redundant frames

  • Data animations rebuilt the entire tick-label DOM 60×/s while the axes stood still. They now use the view animation's 80 ms cadence and force one settled rebuild when the transition ends.
  • A DPR change (browser zoom) ran a synchronous resize and left the ResizeObserver's queued one, laying out and painting twice. It now coalesces into the single resize frame.
  • The same DPR change left per-instance stroke widths and corner radii at the old scale, because those are baked in device pixels. They are rescaled in place before the frame draws.

P2 — xy.pie_chart

Listed in the generated chart-factory inventory with the other polar compositions (it was the one missing). A zero-width bar is now legal and draws nothing, like line_width=0, so the hand-rolled pie/gauge recipes no longer die at exactly 0 % with "bar width must be positive" — a message about the author's code from a value that came out of their data. Negative and non-finite widths are still refused.

CodSpeed coverage for the polar surface

CodSpeed reported "103 untouched benchmarks" on #370 — for a change that rewrote wedge geometry in three renderers. That blind spot is why the 50k-bar cliff above was found by hand. benchmarks/test_codspeed_polar.py adds six rows: payload prep for a polar line / wind rose / pie (the three materially different validation and emit paths), SVG vs native-PNG export of the same rose (bracketing the arc-flattening term, so a regression back to a flat subdivision count is a step change rather than a bug report), and a polar heatmap's bounded inverse raster.

The report's other item — 2 benchmarks skipped, baseline reused — is orphan rows in the dashboard, not skipped tests: the suite collects exactly 103 rows and 103 were measured, so those two have no code behind them and need archiving there by hand. test_codspeed_row_count_matches_the_methodology_spec now gates the collected total against spec/benchmarks/methodology.md §8 (which was itself one row stale), so a renamed or deleted benchmark fails CI instead of becoming a permanent "skipped" row.

Not in this PR

docs/charts/pie-chart.md and docs/charts/radial-bar-chart.md live on #371's branch, not on this base, so the two doc-page items from the audit — adding xy.pie_chart() to the Pie prose, and the contrast failures in the Pie/Radial Bar examples (gray text at 2.63:1, adjacent purples at 1.33–1.43:1) — belong there. The underlying 0 %-recipe failure those pages hit is fixed here.

Verification

Ruff check and format are clean across the worktree, and pre-commit runs nothing else outside docs/ (only codespell, and the sole docs change is a Python list).

The sandbox has no network egress to PyPI, npm or crates.io, so pytest, cargo test and node js/build.mjs could not be executed here — the native core and the client bundle cannot be built. What was verified directly:

  • every source-text assertion in the new test modules re-checked against the current js/src (33 literals, 0 failing);
  • config.polar_bar_segments exercised standalone: 96 at a full turn, 6 at 22.5°, floor 2, and a constant per-segment angle across 360/180/90/45/22.5°;
  • _textblock.wrap_lines/measure(max_width=) exercised standalone: the reported title wraps to two lines at a 380 px canvas (block height 33.6 px inside a 41.6 px reserved band), single-line titles keep the historical 30 px band, hard newlines are preserved, and an over-long word overflows on its own line;
  • the refusal and pyplot-strip logic exercised standalone over all six keyword cases plus the off/none spellings that must survive;
  • the numeric-format grammar checked against _NUMBER_SPEC (.0f°90°);
  • the CodSpeed row-count arithmetic and AST collection (103 → 109 with the new module, matching the spec);
  • every existing polar_wedge_points caller in the suite passes an explicit steps=, so the new per-wedge default cannot move their assertions.

CI is the first place the full suite runs.

Nine reported failures across the polar surface and the responsive chrome
it shares with every other chart.

**P1 — repeated data updates leaked GPU buffers.** Trace teardown walked a
hand-kept list of geometry buffer names, so every rebuilt trace (a
state-driven update, an append that could not patch in place, an animated
spec swap) orphaned its style, direct-RGBA colour, stroke, corner-radius,
LOD-blend and dashed-line-length buffers. All three teardown paths now read
one shared TRACE_GPU_BUFFERS list, pinned against the build paths by a test
so a new channel cannot reintroduce the leak.

**P1/P2 — mobile Wind Rose chrome.** The browser wraps a long title while
layout measured one line, so a compact title lost ~10 px off the canvas top.
Titles now wrap at one shared width in all three renderers and the browser
caps the element at that same width; single-line titles are byte-identical.
A polar legend now reserves a gutter beside the disc — 96 px on the loc's
side, or a 64 px band beneath at compact widths — instead of covering the
north-east sectors and the outer radial label. An authored anchor or
four-tuple padding still wins.

**P2 — wedge vertex cost.** Subdivision is span-proportional:
clamp(ceil(96 * |span| / turn), 2, 96) over the *authored* angular width, in
every renderer. Sagitta is quadratic in the per-segment angle, so this holds
the flattening bound while a 16-sector wind-rose bar costs 14 vertices
instead of 194; a full turn still uses 96.

**P2 — inert polar axis options.** theta_axis/r_axis refuse minor_tick_values,
minor_style, tick_label_min_gap, tick_label_anchor and the collision
spellings of tick_label_strategy, each naming the control that does work.
pyplot's projection="polar" drops the same keywords instead, because every
matplotlib Axes carries an rcParam minor style it never authored; recorded in
the compat spec. theta_axis(format=) now wins over the built-in degree text,
and an r_axis(margin=) is honoured instead of discarded.

**P2 — radial time axes.** A time radius autoranges from its data instead of
epoch zero, which had squeezed every modern instant into a hairline ring at
the rim. The signed-radius contract (a position, never a mirrored direction)
is now normative in the spec and in the r_axis docstring.

**P2 — compact colorbars.** They keep their two extreme tick labels and the
scale title; only the interior ladder and the text-free minor ticks drop.
Hiding everything left an unlabelled gradient.

**P2 — redundant frames.** Data animations use the view animation's 80 ms
label cadence instead of rebuilding the whole tick-label DOM every frame, and
force one settled rebuild at the end. A DPR change coalesces into a single
resize frame and rescales the per-instance widths and corner radii that are
baked in device pixels.

**P2 — pie_chart.** Listed in the generated chart-factory API reference with
the other polar compositions. A zero-width bar is now legal and draws
nothing, like line_width=0, so a 0% progress ring or an empty aggregated
category no longer dies with "bar width must be positive".
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5810af06-e11e-4b89-b466-a60ea78cb2a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@codspeed-hq

codspeed-hq Bot commented Jul 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 103 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing claude/wind-rose-polar-fixes-vrohh2 (fee7007) with alek/polar-axes (a3e28bf)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Alek99
Alek99 merged commit 7449360 into alek/polar-axes Jul 30, 2026
35 of 39 checks passed
@Alek99 Alek99 changed the title Polar audit round: GPU buffer leak, mobile chrome, and wedge vertex cost Polar audit round: GPU buffer leak, mobile chrome, wedge vertex cost, CodSpeed coverage Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant