perf(core): allocate the label atlas only for multi-label annotations - #460
Merged
Conversation
tsenoner
force-pushed
the
fix/render-cliff-456
branch
from
August 17, 2026 09:33
df83816 to
041fb8f
Compare
tsenoner
force-pushed
the
fix/lazy-label-atlas
branch
from
August 17, 2026 09:33
51d51c6 to
d30b431
Compare
tsenoner
force-pushed
the
fix/render-cliff-456
branch
from
August 17, 2026 10:20
041fb8f to
8d4557a
Compare
tsenoner
force-pushed
the
fix/lazy-label-atlas
branch
from
August 17, 2026 10:20
d30b431 to
1dbb7d5
Compare
tsenoner
force-pushed
the
fix/render-cliff-456
branch
from
August 17, 2026 10:43
8d4557a to
94955c1
Compare
tsenoner
force-pushed
the
fix/lazy-label-atlas
branch
from
August 17, 2026 10:43
1dbb7d5 to
5e7c817
Compare
The pie-chart colour atlas is 32 of the 76 bytes per point resident on the GPU — 42% — and for a single-label annotation every one of them is dead weight. Nothing samples it: the shader's pie branch needs labelCount > 1.5, and fillLabelColorTexels returns immediately for a one-colour point. At Swiss-Prot scale that is 17.5 MiB of CPU and 17.5 MiB of GPU held for a feature the view is not using, plus a full-surface upload on every restage. The gate is pull-based and computed inside createStyleGetters, over the same `data` binding the colour getters close over. That makes staleness structurally impossible in the direction that matters: getColors returns the deduped values for that annotation, so getColors(p).length >= 2 implies the storage is multi-valued, implies the gate is true. It can only ever over-report. That covers every transition a push model would have had to enumerate — annotation switch, projection switch, dataset swap, EAT overlay, isolation, legend hide/show — because all of them go through a getter rebuild. The predicate is memoized on a WeakMap keyed by the storage object: the dense form is O(N) and the getters rebuild on a legend hide, a selection, a projection switch. Sound because no producer mutates an AnnotationData in place. The renderer tracks the transition itself rather than trusting a caller to invalidate. A change in multi-label-ness need not move the style signature, which samples four points' colours, but it changes every point's staged slice count — and `_refreshSelectedAnnotationValues` nulls the getter cache without calling invalidateStyleCache, so a caller-driven scheme would have a real hole. The getter is required on WebGLStyleGetters but called optionally, defaulting to true: omitting it over-allocates, which wastes memory, where under-reporting would silently drop pie segments. Only the first failure direction is acceptable. Refs #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFuj7M2Ls1hpJXKaX2uJp
Merges the "resources for an unused feature are not allocated" requirement into renderer-capability-limits. Refs #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFuj7M2Ls1hpJXKaX2uJp
tsenoner
force-pushed
the
fix/lazy-label-atlas
branch
from
August 17, 2026 14:38
5e7c817 to
ddbd3f2
Compare
… atlas Review follow-up on the lazy label atlas. The export inherited its atlas decision from `this.atlas`, which records only what the last completed render staged. Since the atlas is now released for a single-label annotation, that field is legitimately null while a multi-label annotation is selected — before the first populate, on an empty render, and in the window between an annotation switch and the next frame. Nothing forces a render before an export, so an export taken in that window staged two colours per point through the live getters and then told the shader there was no atlas: dominant colours where the user saw pie markers. `exportLabelStride` now asks the same authority the export stages its colours from, while a live plan still wins so a device-forced fidelity reduction is never exceeded. `isMultilabel` is read once per render pass and latched into `labelAtlasActive`; `syncLabelAtlas` allocates against that latch rather than re-asking the getter, so the gate and the re-stage it triggers cannot disagree. The `labelAtlasDisabled` guard is split back out, which drops a provably dead release branch. `WebGLStyleGetters` was declared with `isMultilabel` required, but six renderer suites kept their own copy of the getters stub without it — real TS2741 errors that `type-check` cannot see, because `packages/core/tsconfig.json` excludes `**/*.test.ts`. All six now use the shared `styleGetters` fixture, which is what that module exists for. The optional call with its `?? true` default stays, per the change's task 2.6: under-reporting silently drops pie segments, and the type is not enforced for exactly the files that kept getting this wrong. Tests: the gate is pinned at `createStyleGetters`, where the storage-shaped vs colour-shaped decision is actually made, including that hiding a value leaves it true; one session now drives both atlas release paths — the multi-label gate and the capacity hysteresis — and asserts that re-entry plans against current capacity rather than resurrecting the released plan; and two shrink tests that had gone vacuous under a single-label fixture are back on multi-label colours. Refs #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvrjuoBnKueAUnT1LwxtV9
…test filters Cleanup pass over the lazy-label-atlas change. No behaviour change except the export ordering below. `exportLabelStride` asked `this.atlas` before the gate, so it was wrong in the mirror of the window it was written for: after a switch to a single-label annotation with no frame rendered yet, the stale plan won and the export built and uploaded a capacity-sized atlas (~18 MB at 573K points, plus a `fillLabelColorTexels` pass over every point) of texels the shader never samples, because every `labelCount` is 1. Asking the gate first makes the single-label early-out reach the export too; a live plan still wins once the answer is yes, so a figure still cannot exceed the fidelity the screen showed. The gate's release repeated `disableLabelAtlas`'s three statements verbatim. Both reasons to release now run one `releaseLabelAtlas`, so a fourth release step cannot reach the failure path and miss the gate path — which is the one that fires on every annotation switch. `isMultilabelAnnotation` in the legend's sync controller was the one consumer the memo did not reach; it runs from `_renderColorPicker`, i.e. on every legend re-render while the picker is open. The memo itself stays: `conversion.ts` picks `Int32Array` only when a column is single-valued AND carries neither scores nor evidence, so a single-label EAT or predicted column lands in dense storage, where `.some()` cannot early-exit and walks all 573K inner arrays. Tests: `texImageSizes`/`atlasAllocations`/`realAtlasAllocations` move to the shared fixture and the atlas-shrink assertion uses them, so it is about the atlas rather than about whichever texture happened to be allocated last — the gamma pipeline's canvas-sized one is in that list too. `makeSwitchableRenderer` spreads the shared stub instead of hand-listing a seventh copy of `WebGLStyleGetters`. The `no-clamp` device-limit literal returns to its local helper. One accessor replaces two in the export suite, dropping five `as unknown as` double casts. The memoization test asserted three `true`s and passed identically with the memo deleted; it now mutates storage in place — which production never does — purely to observe that the second call does not re-read. Its sibling claiming to prove the storage-shaped property had no hidden-value input at that layer and could not prove it; the real version lives in `style-getters.test.ts`, where hiding is an actual input. Refs #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvrjuoBnKueAUnT1LwxtV9
`defer-label-atlas-allocation` gave "the live renderer has no label atlas" a second cause and left the spec describing only the first. While the device-limit case was the only way to reach that state, the scenario's condition determined its outcome: no atlas on screen meant dominant colours, and an export that did the same matched what the user saw. Releasing the atlas for a single-label annotation added a second cause with the opposite correct answer — `this.atlas` is also null when a multi-label annotation is selected but no frame has been staged yet, and there the export must allocate one. The code was corrected in the same PR; this archives the matching spec delta, so the living spec stops describing the pre-#457 world. Narrows the existing scenario to the device-limit case it was written for, states the sourcing rule normatively, and adds the two cases the gate created. No code change. Refs #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvrjuoBnKueAUnT1LwxtV9
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.
Third and last of the #456/#457 series. Stacked on #459, which is stacked on #458. Merge order: #458 → #459 → this.
What this is
The pie-chart colour atlas is 32 of the 76 bytes per point resident on the GPU — 42% — and for a single-label annotation every one of them is dead weight. Nothing samples it: the shader's pie branch needs
v_labelCount > 1.5, andfillLabelColorTexelsreturns immediately for a one-colour point.At Swiss-Prot scale (573,649 proteins) that is 17.5 MiB of CPU and 17.5 MiB of GPU held for a feature the view is not using — plus a full-surface upload on every restage, i.e. on every legend recolour.
This is small because #458 did the hard part: it made "no atlas" a first-class, tested state — nullable backing array, staging that skips texels, a 1×1 placeholder keeping the sampler complete, and a shader gate that makes the pie branch unreachable. That was built for the device-limit failure path. This change adds a second, deliberate reason to enter it, so a revert cannot regress to corruption.
How the gate stays honest
Pull-based, computed inside
createStyleGettersover the samedatabinding the colour getters close over.That makes staleness structurally impossible in the direction that matters.
getColorsreturns[...new Set(values)]for the selected annotation, sogetColors(p).length >= 2implies the storage is multi-valued, implies the gate is true. It can only ever over-report — allocate when it needn't — never under-report and drop a segment.And it covers, in one place, every transition a push-based scheme would have had to enumerate individually: annotation switch, projection switch, dataset swap, the EAT overlay, isolation, legend hide/show. All of them go through a getter rebuild, and
datais in the cache key.Two details worth flagging for review:
WeakMapkeyed by the storage object. The dense form isdata.some(v => v.length > 1)— O(N), 573K at Swiss-Prot scale — and the getters rebuild on a legend hide, a selection, a projection switch. Sound because no producer mutates anAnnotationDatain place; every one returns fresh storage._refreshSelectedAnnotationValuesnulls the style-getter cache without callinginvalidateStyleCache, so a caller-driven scheme would have a real hole.One asymmetry, on purpose
isMultilabelis required onWebGLStyleGettersso TypeScript consumers cannot omit it, but the renderer calls it optionally and defaults totrue. A consumer that omits the getter over-allocates, which costs memory; one that under-reports silently drops pie segments — a wrong picture presented as data. Only the first failure direction is acceptable.The storage-vs-colour distinction
The gate answers from stored values, never from rendered colours. A colour-shaped test would read false the moment hidden values collapse every point to one colour — and would then release the atlas exactly one un-hide before it is needed again.
point-visibilitycarries this as a normative requirement (added by #458); this change implements it, and there is a test that pins it.Verification
pnpm test2,293 green ·pnpm test:e2e124 green ·pnpm precommit+pnpm format:checkgreen.Four new renderer tests: a single-label annotation allocates nothing beyond the placeholder and never issues a
texSubImage2D; single → multi → single allocates exactly once and releases; the transition re-stages even though the signature cannot see it; staying multi-label refreshes in place rather than reallocating. Plus memo tests covering per-object (not per-content) caching and the storage-shaped property.The assertions filter atlas allocations from framebuffer ones — the gamma pipeline allocates its own canvas-sized texture, which caught me out first time round.
No user-visible change on a multi-label annotation
Pixels are identical; only the unused case stops paying.
Please merge or rebase, not squash (repo-wide
allow_squash_merge: false).🤖 Generated with Claude Code
https://claude.ai/code/session_01USFuj7M2Ls1hpJXKaX2uJp