Skip to content

fix(glyph): keep shared Three storage alive when a draw retires - #317

Open
krispya wants to merge 4 commits into
mainfrom
fix/three-shared-storage-disposal
Open

krispya wants to merge 4 commits into
mainfrom
fix/three-shared-storage-disposal

Conversation

@krispya

@krispya krispya commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Removing a TextGroup on Three r186 could destroy storage still used by a sibling. Retired draw geometries now detach shared attributes that remain owned by the root; replaced generations retain Three's geometry release path.

This also fixes final-owner disposal after a nonempty → empty publication, including a rendered draw replaced by a never-rendered draw. On published Three r185/r186, a private compatibility adapter registers the actual renderer's attribute manager and calls its canonical delete path when the attribute's owner disposes it. It covers material-only placement storage and detached Glyphs storage, supports multiple renderers, and uses weak references. Repeated frames perform one mesh-local WeakSet lookup. Public API, ABI and peer ranges are unchanged.

Validation of runtime head 391aebd920870afd17529d610974902cd7407b53 (runtime fix adbdc371, followed by documentation formatting) and regression-harness head 4027390b516d0af1dfafe8c56bd99d682482aeb8:

  • Full Glyph check and benchmark check pass; 1,028 package/integration tests and 124 focused Three integration tests pass.
  • Frozen packed consumer, through Portless, passes Chromium WebGPU and WebGL2 on Three 0.185.1 and 0.186.1. The persistent-renderer lifecycle returns storage from 0 → 10 → 2 → 10 → 2 → 0; initial and revived readback match exactly. The authenticated original package leaks 18 and 10 storage attributes respectively in the same proof.
  • Two renderers sharing storage, renderer disposal before Glyph disposal, and r186 compileAsync replacement all pass. Fresh scoped source review found no material issue.
  • Direct sibling-removal regression on the same published Three 0.186.1/WebGPU renderer: the authenticated base package fails its post-removal submit with a destroyed-buffer validation error and leaves the A+B framebuffer (5,493 lit pixels), while the PR runtime retains B and matches the B-only RGBA reference (2,721 lit pixels) with zero errors. WebGL2 control matches B on both packages. The named benchmark:three-storage-lifetime --scenario=sibling harness and source-bound browser proof are committed on this PR.
  • Exact-head CI run 39135968931 passes all five jobs, including full correctness, size and performance reporting. Its six smoke workloads are neutral (p50 changes −2.3% to +2.1%; no significant regression detected). The report flags noisy samples and has roughly 5–8% resolution, so it does not exclude smaller effects or cover the full suite.
  • Exact-head CI run 39149071610 on 4027390b passes all five jobs, including benchmark application/script types, browser conformance, packed-consumer verification, size and performance reporting.
  • Packed archive grows 3,304 bytes locally. CI delivery reporting measures the affected Three runtime bundles at +480–521 gzip bytes, with every affected surface below 0.5% growth. Local full delivery reporting was blocked by an absent demo build; CI completed it.

Local browser-tested package SHA-256: 926aee67b36ea35e0814a5b2f59576e56ec6a40b5155be0ad5962fba635c65be. CI package SHA-256: 742bcba06fc86545db2ad577063ccf8c92c602000af9a3ca2e9844975aae2b93. All runtime JavaScript, declarations and package manifest are byte-identical between them; the five Wasm members differ, so the local proof is not claimed as an exact-byte CI-package replay. CI's independent correctness job passed. The named regression workflow is benchmark:three-storage-lifetime. The compatibility bridge is limited to verified revisions 185/186; later Three revisions use attribute disposal directly. This PR remains unmerged.

Three r186 geometry disposal destroys the GPU buffer of every attribute on
the geometry. Each draw geometry also carries the root's shared Codec
buffers, transform table, and detached glyph storage, so removing one
TextGroup destroyed buffers a sibling draw's bind group still referenced
and every later submit failed WebGPU validation.

Retired draws now detach shared storage that outlives the publication
before disposal. Storage retiring in the same publication stays attached,
so Three still releases it.

Validation: two new three-v1 tests fail before and pass after; the full
Glyph check passes (1,028 package and integration tests). A packed build
on three 0.186.1 removes the label with a clean console (66 validation
errors in 60 frames before); three 0.185.1 behaves identically before and
after.
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Docs report 📚

Every changed package is attested at this head. A reviewer verifies each claim after merge.

Package Concept Attestation Status
@pmndrs/glyph-benchmarks packages/benchmarks.md · not edited Added named Three sibling-storage packed-browser proof. Targeted Oxfmt, benchmark app/scripts TypeScript, Oxlint and gi… ✅
@pmndrs/glyph packages/glyph.md · edited Registered published Three r185/r186 renderer attribute managers for final-owner storage disposal; full Glyph and bench… ✅

✅ attested · ⚠️ stale · ❌ missing

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

size-limit report 📦

Surface gzip Surface gzip
Core JS 91.2 kB (0%) Inter font · Bitmap 558.2 kB (0%)
Shaper Wasm 541.6 kB (0%) Inter font · MTSDF 8.17 MB (0%)
Three.js adapter JS 142.9 kB (+0.34% Size increase) Inter font · Slug 618.3 kB (0%)
React adapter JS 142.7 kB (+0.37% Size increase) Font Awesome icons · Bitmap 449.9 kB (0%)
Vue adapter JS 142.6 kB (+0.36% Size increase) Font Awesome icons · MTSDF 8.76 MB (0%)
R3F hello-world app JS 747.4 kB (+0.05% Size increase) Font Awesome icons · Slug 657.9 kB (0%)
Tres playground app JS 571.7 kB (+0.08% Size increase) Bitmap baker JS 4.9 kB (0%)
Font validator JS 144.8 kB (0%) Bitmap baker Wasm 231.1 kB (0%)
Runtime bake host JS 6.1 kB (0%) MTSDF baker JS 5.6 kB (0%)
Runtime bake Worker JS 13.5 kB (0%) MTSDF baker Wasm 215.5 kB (0%)
Font baker JS 2.6 kB (0%) Slug baker JS 4.5 kB (0%)
Font baker Wasm 389.0 kB (0%) Slug baker Wasm 183.7 kB (0%)

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Package performance: smoke suite

0 faster · 0 slower · 6 neutral · 0 skipped

Baseline 0.1.0-canary-4e25a96a-20261015 → candidate 0.1.0 (742bcba0)

Latency distribution across 6 comparable workload p50s. Each workload counts once; these are not frame times.

statistic baseline candidate
Median workload p50 3.885ms 3.745ms
Workloads ≤1 ms 1 1
Workloads ≤2 ms 2 2
Workloads ≤3 ms 2 2
Workloads >3 ms 4 4

Largest candidate workloads (including neutral results):

workload baseline candidate Δ time
measure after text change 6.25ms 6.18ms -0.070ms
publish after text change 5.07ms 4.91ms -0.160ms
relayout after font-size change 4.47ms 4.30ms -0.170ms
publish after color change 3.30ms 3.19ms -0.110ms
reflow after width change 1.74ms 1.61ms -0.130ms
                                   faster ◀       ┊       ▶ slower    Δ p50      p  baseline   candidate
  measure after text change                   ├──●┼┤                  -1.0%   .200  █▆▅▅▂▃▂▃▂▁ █▅▇▄▁▂▁▂▂▁
  publish after color change               ├────●─┼──┤                -3.1%   .343  ▅█▅▆▂▂▁▁▁▁ ▇█▄▄▃▂▁▁▁▁
  publish after text change              ├──────●─┤                   -3.2%   .114  █▆█▇▄▃▂▂▂▂ ▅█▃▆▂▃▂▁▁▁
  relayout after font-size change          ├───●──┼──┤                -3.9%   .343  █▆▄▄▂▂▁▁▂▁ █▆▄▅▂▂▁▁▁▁
  measure 100 unchanged labels       ├───────●────┼────────┤          -7.0%   .486  ▄▅▇██▃▅▁▁▂ ▅▄█▇▄▂▃▃▁▁
  reflow after width change           ├─────●─────┼────┤              -7.4%   .114  ▆▅▇▃▅█▄▅▃▁ ▆█▇▂▃▇▄▃▂▂
                                   └──────────────┴──────────────┘
                                   -20%           0           +20%

All 6 compared benches are neutral.

6 neutral, 0 skipped
status bench baseline candidate Δ time Δ p50 p 95% CI
neutral measure after text change 6.25ms 6.18ms -0.070ms -1.0% .200 -4.8..+0.8%
neutral publish after text change 5.07ms 4.91ms -0.160ms -3.2% .114 -12.0..+0.4%
neutral reflow after width change 1.74ms 1.61ms -0.130ms -7.4% .114 -16.1..+6.3%
neutral publish after color change 3.30ms 3.19ms -0.110ms -3.1% .343 -9.4..+4.1%
neutral relayout after font-size change 4.47ms 4.30ms -0.170ms -3.9% .343 -9.5..+4.6%
neutral measure 100 unchanged labels 62.69µs 58.32µs -0.004ms -7.0% .486 -17.0..+11.6%

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.

2 participants