Skip to content

[Text] Glyph Outline Part 5/13 - Shaped buffer glyph indices - #21501

Merged
MrJul merged 10 commits into
AvaloniaUI:mainfrom
Gillibald:pr2d/shaped-buffer-glyph-indices
Sep 7, 2026
Merged

[Text] Glyph Outline Part 5/13 - Shaped buffer glyph indices#21501
MrJul merged 10 commits into
AvaloniaUI:mainfrom
Gillibald:pr2d/shaped-buffer-glyph-indices

Conversation

@Gillibald

@Gillibald Gillibald commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Part 5 of 15 of the glyph-outline / variable-font workstream. Stacked on Part 4 (pr2e/metrics-batch-cleanup);

What does the pull request do?

Exposes the post-shaping glyph indices of a run as one contiguous span:

public ReadOnlySpan<ushort> GlyphIndices { get; }

GlyphRunImpl and the batch readers added in Part 3 all want the same shape, every glyph index in this run as a single span. Extracting that from the existing ReadOnlyMemory<GlyphInfo> costs either a copy or a per-glyph field read, on the hot text path.

What is the updated/expected behavior with this PR?

  • ShapedBuffer keeps a parallel ushort[] of glyph indices alongside the existing GlyphInfo array, filled by the same loop that builds it, so the cost is one extra store per glyph rather than a second pass.
  • GlyphRunImpl reads that span when calling TryGetHorizontalGlyphAdvances and the bounds reader, replacing a materialisation it was doing per call.
  • Shaping output, GlyphInfo layout and existing GlyphInfo[i].GlyphIndex access are all unchanged. Callers that do not want the span pay nothing for it.

The backing field is settable only from the shaping path; the public surface is the read-only span.

Checklist

Breaking changes

None. The property is additive.

Obsoletions / Deprecations

None.

Fixed issues

None.

@Gillibald Gillibald added enhancement area-textprocessing api-needs-review The PR adds new public APIs that should be reviewed. labels Jun 4, 2026
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066032-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch 2 times, most recently from 68671b9 to 5a96610 Compare June 5, 2026 05:06
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066061-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from 5a96610 to 35594bc Compare June 5, 2026 08:50
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066088-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch 2 times, most recently from 4ec6206 to 0d5b7a1 Compare June 8, 2026 04:59
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066142-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from 0d5b7a1 to c4f34c2 Compare June 8, 2026 09:02
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066159-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from c4f34c2 to 580365f Compare June 12, 2026 13:37
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066335-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch 2 times, most recently from ec2b9b8 to 091ab0d Compare June 12, 2026 16:12
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066375-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from 091ab0d to 75d4b75 Compare June 23, 2026 09:47
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0066789-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch 2 times, most recently from 2b80803 to 5b624da Compare August 20, 2026 05:06
@MrJul MrJul added api-approved The new public APIs have been approved. and removed api-needs-review The PR adds new public APIs that should be reviewed. labels Aug 20, 2026
@MrJul

MrJul commented Aug 20, 2026

Copy link
Copy Markdown
Member

Notes from the API review meeting:
The API is accepted as is.

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from 5b624da to dcfd6f4 Compare August 20, 2026 10:10
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0068585-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from dcfd6f4 to 22b1747 Compare August 24, 2026 10:46
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0068654-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from 22b1747 to 779d97c Compare August 28, 2026 07:00
miloush
miloush previously approved these changes Aug 28, 2026
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0069167-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Comment thread src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyfTable.cs
Comment thread src/Avalonia.Base/Media/GlyphTypeface.cs Outdated
miloush
miloush previously approved these changes Sep 4, 2026
GlyphMetrics previously stored the advance in Width/Height, leaving no
field for the actual ink bounding box. Correct the contract:

- Expand GlyphMetrics with AdvanceWidth / AdvanceHeight (and the bitmap /
  vertical-layout members XOffset / YOffset / VerticalOriginX/Y, reserved
  for later population).
- Add GlyfTable.TryGetGlyphBounds: an allocation-free, header-only read of
  the glyph's control-point bounding box (xMin/yMin/xMax/yMax). Empty
  glyphs return true with zero bounds; out-of-range / short data return
  false. Composite glyphs use their header bbox without recursion.
- TryGetGlyphMetrics (single + batch) now sources advances into
  AdvanceWidth / AdvanceHeight and the bounding box into
  Width / Height / XBearing / YBearing via TryGetGlyphBounds. Side bearings
  fall back to hmtx/vmtx when the glyph has no outline.

The previously latent API had one test relying on the old behaviour
(Width == advance); it's updated to assert AdvanceWidth. New tests cover
TryGetGlyphBounds (range, empty, letter, descriptor parity) and the
corrected metrics (advance placement, ink width distinct from advance,
empty-glyph advance-without-ink, batch/single parity).
GlyphBoundsBenchmark (Avalonia.Benchmarks): a BenchmarkDotNet comparison of
the table-based bounds read against Skia's SKFont.GetGlyphWidths, in kernel
(font pre-created) and per-run variants across 1 / 16 / 256 glyphs, with
MemoryDiagnoser.
Adds a bounds-only batch reader so ink-bounds computation (the GlyphRunImpl
use case, which already has advances from shaping) doesn't pay for advance
lookup or per-glyph indirection:

- GlyphBounds: internal value type carrying the control-point box.
- GlyfTable.GetGlyphBounds / GlyphTypeface.TryGetGlyphBounds: fetch the glyf
  and loca spans once per batch and read offsets + headers directly via
  BinaryPrimitives — no per-glyph ReadOnlyMemory.Span conversion, no
  intermediate Memory.Slice, no nested call chain. LocaTable exposes RawData
  / IsShortFormat for this.
- Batch TryGetGlyphMetrics refactored onto the same span-cached reader.

Measured against Skia's SKFont.GetGlyphWidths (bounds-only, in-process
BenchmarkDotNet): the table path is faster at every run length (0.06× / 0.32×
/ 0.66× at 1 / 16 / 256 glyphs) and allocation-free, where Skia allocates
104 B/run for the SKFont. The earlier advance+bounds path via
TryGetGlyphMetrics was ~2.2× slower than Skia at 256 glyphs; the bounds-only
span-cached path is ~1.5× faster.

The benchmark is updated to compare bounds-only on both sides.
The glyf bounds readers (TryGetGlyphBounds / GetGlyphBounds) read the
header int16s via BinaryPrimitives. The base pr2 commit trimmed this
using as unused there, but it is needed once pr2b adds the bounds code —
without it GlyfTable does not compile on this branch.
GlyphBounds.Width/Height and both TryGetGlyphMetrics overloads computed
the ink extent as (xMax - xMin) / (yMax - yMin). A malformed glyf header
where xMax < xMin (or yMax < yMin) produced a negative value that, when
narrowed to the ushort GlyphMetrics.Width/Height, wrapped to a huge
positive extent instead of a safe zero.

Clamp the extent to non-negative in GlyphBounds (the single source of
truth for both the single and batch metric paths) and route the single
TryGetGlyphMetrics path through GlyphBounds so both behave identically.
For int16 coordinates the maximum extent is 65535 = ushort.MaxValue, so
the clamped value always fits without overflow. Adds GlyphBoundsTests.
Batch TryGetGlyphMetrics unconditionally sized the hMetrics, vMetrics and
bounds temporary buffers to glyphIds.Length, even when the font has no
hmtx, no vmtx, or no glyf table. For CFF/CFF2 fonts (no glyf) the bounds
buffer is never read, and most fonts have no vmtx, so a large batch on
such a font heap-allocated a per-glyph array that was pure waste.

Size each buffer to zero — a free, empty stackalloc — when its source
table is absent, so it only materialises (and only spills to the heap
past 256 glyphs) when it will actually be filled and read.
@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch 2 times, most recently from cf5c0ca to d28eee4 Compare September 7, 2026 07:07
Gillibald and others added 4 commits September 7, 2026 09:15
The short loca format stores every offset divided by two, which is why the
batch reader doubles what it reads. LocaTable's single-glyph path already
carries that note; the inline decode in the batch path did not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The glyf check does not vary across the run, so it belongs above the loop
rather than inside it. Splitting it also lets the bounds buffer live only
in the branch that reads it, which drops the zero-length allocation that
CFF / CFF2 fonts used to make just to satisfy the shared loop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirror the existing TryGetHorizontalGlyphAdvance / ...Advances pair with
vertical equivalents. Until now a vertical-layout caller (CJK, Mongolian)
had no advance-only path on GlyphTypeface and had to go through
TryGetGlyphMetrics, which after PR2b reads the glyf bounding box per
glyph — pure waste when only the advance is needed.

VerticalMetricsTable already exposes the per-glyph and batch readers
internally (used by TryGetGlyphMetrics(batch)); this is a thin wrapper
over them.

Tests cover the Latin (no vmtx → false) and CJK (vmtx → positive
advances) cases and batch/single parity using Inter and MiSans-Normal.

Audit note: HorizontalMetricsTable and VerticalMetricsTable batch readers
already span-cache (each fetches _data.Span once per call, reusing a
BigEndianBinaryReader across glyphs). No metrics-table refactor is
needed — the speculative Part 2 of the plan collapses to nothing.
Consumers that need a ReadOnlySpan<ushort> of glyph IDs (the upcoming
GlyphTypeface.TryGetGlyphBounds batch path, SKFont.GetGlyphWidths, atlas
lookups...) previously had to allocate a parallel ushort[] and project
GlyphInfo.GlyphIndex into it. Carry that array on the ShapedBuffer instead:

- ShapedBuffer rents a parallel ushort[] from ArrayPool alongside the
  existing GlyphInfo[] rental, exposed as
  `public ReadOnlySpan<ushort> GlyphIndices`.
- The indexer setter syncs both, so HarfBuzz shaping and post-shape
  mutators (InterWordJustification rewrites the same GlyphIndex) keep
  the parallel view in lockstep with no caller change.
- Split / WithBidiLevel slice the indices alongside the GlyphInfos at
  the same offsets — both share the parent's pooled arrays.
- Dispose returns both rentals.

Consumer wiring:
- GlyphRunImpl: copies ShapedBuffer.GlyphIndices once via CopyTo when
  the source is a ShapedBuffer, dropping the per-glyph extract from the
  position loop.
- TextFormatterImpl.CreateEmptyTextLine: switched to the pooled public
  ctor + indexer (was passing a heap-allocated GlyphInfo[] to the
  internal slice ctor, which now requires a parallel ushort slice).

Tests cover indexer sync, overwrite, ascending split alignment,
empty-leading split, and dispose clearing the view.
@Gillibald
Gillibald force-pushed the pr2d/shaped-buffer-glyph-indices branch from d28eee4 to 129a672 Compare September 7, 2026 07:30
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0069495-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul
MrJul enabled auto-merge September 7, 2026 10:44
@MrJul
MrJul added this pull request to the merge queue Sep 7, 2026
Merged via the queue into AvaloniaUI:main with commit 79ff4ff Sep 7, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-approved The new public APIs have been approved. area-textprocessing enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants