Skip to content

[Text] Glyph Outline Part 4/13 - Batched glyph advance lookup - #21500

Closed
Gillibald wants to merge 9 commits into
AvaloniaUI:mainfrom
Gillibald:pr2e/metrics-batch-cleanup
Closed

[Text] Glyph Outline Part 4/13 - Batched glyph advance lookup#21500
Gillibald wants to merge 9 commits into
AvaloniaUI:mainfrom
Gillibald:pr2e/metrics-batch-cleanup

Conversation

@Gillibald

@Gillibald Gillibald commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Part 4 of 15 of the glyph-outline / variable-font workstream. Stacked on Part 3 (pr2b/glyph-metrics-bounds);

What does the pull request do?

Gives GlyphTypeface a vertical advance lookup to match the horizontal one:

public bool TryGetVerticalGlyphAdvance(ushort glyphIndex, out ushort advance);
public bool TryGetVerticalGlyphAdvances(ReadOnlySpan<ushort> glyphIndices, Span<ushort> advances);

Until now, a vertical-layout caller, such as CJK or Mongolian text, had no advance-only path and had to go through TryGetGlyphMetrics, which, after Part 3, reads the glyf bounding box for every glyph. That is pure waste when only the advance is wanted.

What is the updated/expected behavior with this PR?

  • Both overloads return false and write zero when the font has no vmtx table, which is the common case for Latin fonts, and when the glyph is outside the metrics table.
  • The batch overload requires the output span to be at least as long as the input; otherwise it returns false without writing anything.
  • Values follow the same numberOfLongVerMetrics and monospace-tail rules the horizontal path already applies to hmtx.
  • Variation deltas are not applied here. VVAR arrives in Part 11 (pr4g/vvar-advances) and wires into these same methods.

Nothing changes for existing horizontal-advance callers, and neither overload allocates.

Checklist

Breaking changes

None. The API is additive.

Obsoletions / Deprecations

None.

Fixed issues

None.

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 1317934 to 2dc0dc5 Compare June 5, 2026 04:12
@Gillibald Gillibald added enhancement area-perf area-textprocessing api-needs-review The PR adds new public APIs that should be reviewed. labels Jun 5, 2026
@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch 2 times, most recently from 6077802 to d8cb817 Compare June 5, 2026 07:18
@avaloniaui-bot

Copy link
Copy Markdown

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

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch 2 times, most recently from bfbb4c8 to 00ea272 Compare June 5, 2026 09:36
@avaloniaui-bot

Copy link
Copy Markdown

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

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch 2 times, most recently from c7564ca to 91b1a3a 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-cibuild0066139-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 91b1a3a to 08f132a 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-cibuild0066163-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 08f132a to beb955b 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-cibuild0066333-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch 2 times, most recently from c4d59ba to ea059d1 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-cibuild0066371-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from ea059d1 to 52f4eb2 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-cibuild0066788-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch 2 times, most recently from 73bd830 to 64eb86c Compare August 20, 2026 05:06
@MrJul

MrJul commented Aug 20, 2026

Copy link
Copy Markdown
Member

Notes from the API review meeting:

Make sure glyphIndex/glyphIndices names are used everywhere instead of glyphId/glyphIds.
In the proposed TryGetVerticalGlyphAdvance(s) methods, rename the parameter to match this rule.

@MrJul MrJul added api-needs-changes The new public APIs need some changes. and removed api-needs-review The PR adds new public APIs that should be reviewed. labels Aug 20, 2026
@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 64eb86c to af20a1f 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-cibuild0068587-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from af20a1f to db4bc12 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-cibuild0068653-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

miloush
miloush previously approved these changes Aug 28, 2026
@MrJul MrJul added api-approved The new public APIs have been approved. and removed api-needs-changes The new public APIs need some changes. labels Aug 29, 2026
@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 1acaf75 to b0b1e11 Compare August 31, 2026 07:51
@avaloniaui-bot

Copy link
Copy Markdown

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

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 pr2e/metrics-batch-cleanup branch 2 times, most recently from 57197e6 to 8e992d3 Compare September 7, 2026 07:07
Gillibald and others added 3 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.
@Gillibald
Gillibald force-pushed the pr2e/metrics-batch-cleanup branch from 8e992d3 to 87226fb 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-cibuild0069493-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul

MrJul commented Sep 7, 2026

Copy link
Copy Markdown
Member

Already included in #21501

@MrJul MrJul closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants