[Text] Glyph Outline Part 2/13 - GlyphTypeface.GetGlyphOutline implementation - #21406
Conversation
There was a problem hiding this comment.
Pull request overview
Adds foundational font-table parsing support for extracting per-glyph vector outlines and exposes it via a new GlyphTypeface.GetGlyphOutline(...) public API, enabling future features like glyph path export and COLR outline layer rendering.
Changes:
- Add
GlyphTypeface.GetGlyphOutline(...)and cache theglyftable for outline extraction. - Implement on-demand parsing of TrueType
loca/glyf(simple + composite glyphs) with recursion cycle protection. - Add supporting utilities/types (object pooling, decycler, cmap dictionary view, variation settings scaffolding).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Avalonia.Base/Utilities/ObjectPool.cs | Adds a thread-safe object pool used for reusing traversal helpers. |
| src/Avalonia.Base/Media/IGlyphDrawing.cs | Introduces a glyph drawing abstraction (currently has build-breaking unused usings). |
| src/Avalonia.Base/Media/GlyphTypeface.cs | Adds GetGlyphOutline API and caches glyf parsing/render interface access. |
| src/Avalonia.Base/Media/GlyphDrawingType.cs | Adds an enum describing glyph render formats (outline/COLR/SVG/bitmap). |
| src/Avalonia.Base/Media/FontVariationSettings.cs | Adds a record for future variation/color/bitmap selection parameters. |
| src/Avalonia.Base/Media/Fonts/Tables/LocaTable.cs | Adds loca parsing for glyph-to-glyf offset lookup (has unused using). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/SimpleGlyph.cs | Implements simple-glyph decoding (flags + delta coords) (has unused using + minor comment mismatch). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyphFlag.cs | Defines TrueType simple glyph point flags. |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyphDescriptor.cs | Parses glyph headers and dispatches to simple/composite readers (has unused usings). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyphDecycler.cs | Adds pooled cycle-detection for composite glyph recursion. |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyphComponent.cs | Defines composite component metadata (flags, args, transform). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyfTable.cs | Core glyf reader + geometry builder (has unused usings; composite anchor attachment currently unimplemented; debug logging in hot path). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/CompositeGlyph.cs | Implements composite glyph component parsing (has unused using). |
| src/Avalonia.Base/Media/Fonts/Tables/Glyf/CompositeFlags.cs | Defines composite glyph flags. |
| src/Avalonia.Base/Media/Fonts/Tables/Decycler.cs | Adds generic cycle/depth guard used by glyph traversal. |
| src/Avalonia.Base/Media/Fonts/Tables/Cmap/CharacterToGlyphMapDictionary.cs | Adds a read-only dictionary wrapper for cmap mappings. |
| src/Avalonia.Base/Media/Fonts/Tables/Cmap/CharacterToGlyphMap.cs | Exposes cmap mappings via AsReadOnlyDictionary(). |
Comments suppressed due to low confidence (1)
src/Avalonia.Base/Media/Fonts/Tables/Glyf/GlyfTable.cs:353
- Composite glyphs where
ARGS_ARE_XY_VALUESis not set (i.e. Arg1/Arg2 are point indices for anchor-point attachment) are currently not positioned: CreateComponentTransform leaves tx/ty as 0 in that case. This will render many composite glyphs incorrectly (accents/marks). Consider implementing point-based attachment translation per the TrueType spec (matching component point Arg2 to parent point Arg1).
double tx = 0, ty = 0;
if ((flags & CompositeFlags.ArgsAreXYValues) != 0)
{
tx = component.Arg1;
ty = component.Arg2;
}
|
You can test this PR using the following package version. |
f32b53c to
34106bc
Compare
|
You can test this PR using the following package version. |
0426f22 to
aac247a
Compare
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
Fonts are untrusted input, but several pre-existing table parsers throw out of the GlyphTypeface constructor on hostile data, denying the whole font instead of degrading the affected (cosmetic) table. - name: wrap NameTable.Load in try/catch so a malformed table falls back to a default family name (same outcome as an absent name), and bounds-check NameRecord.GetValue's (offset,length) slice — the record array is validated at load but each record's storage slice is read later during construction. - post: wrap PostTable.Load so a malformed cosmetic-hint table degrades to defaults rather than denying the font. - cmap format 12/13: clamp the declared length and group count to the buffer, computing the group span in long to avoid the nGroups*12 int overflow a hostile count would wrap to a negative slice length. - cmap format 4 selection: score a Windows Symbol subtable worse than any Unicode subtable so ASCII resolves regardless of subtable order, while a Symbol-only font still selects its only subtable. maxp/cmap remain fatal by design. Behavioural hardening only; public API unchanged. Backportable independently of the glyph-outline stack.
9dbea89 to
9f69367
Compare
Format 12/13 group contents are attacker-controlled: TryGetRange used to hand the raw uint32 end straight to per-codepoint consumers, so a group with endCharCode 0x7FFFFFFF turned range enumeration into an unbounded (for int.MaxValue, non-terminating) loop. Clamp ranges to the Unicode range and map inverted/out-of-range groups to empty ranges so later groups still enumerate. Give the format 4 constructor the same treatment its format 12 sibling already received: clamp the declared length and segment count to what the buffer actually holds instead of slicing unchecked (debug asserts replaced by the clamps). In BigEndianBinaryReader, bounds-check array reads before allocating (a hostile length is rejected instead of attempting the allocation), make EnsureAvailable overflow-proof, and clamp ReadBytes against negative counts.
Introduces shared utility types that subsequent PRs will use to implement GetGlyphOutline (glyf table) and GetGlyphDrawing (COLR v0/v1) on GlyphTypeface: - ObjectPool<T>: thread-safe object pool used by Decyclers - Decycler<T> / CycleGuard<T> / DecyclerException: generic cycle-detection and depth-limiting utility for recursive font-table traversal (composite glyphs, paint graphs) - FontVariationSettings: parameter type for the upcoming GetGlyphOutline / GetGlyphDrawing overloads - IGlyphDrawing / GlyphDrawingType: return-type contract for color glyph drawings (outline, color layers, SVG, bitmap) - CharacterToGlyphMapDictionary: lightweight, allocation-free IReadOnlyDictionary<int, ushort> view over the cmap, plus CharacterToGlyphMap.AsReadOnlyDictionary() to expose it No behavior change; types are not yet consumed in this PR.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GetGlyphOutline(ushort, Matrix) returned a mutable StreamGeometry (an AvaloniaObject) with the transform baked in — neither cacheable (every pen position is a distinct geometry) nor safe to share. Reshape to GetGlyphOutline(ushort) returning design-unit-space geometry as an immutable IGeometryImpl (a small sealed wrapper that exposes only the read-only surface and can't be re-opened), so a single instance is safe to cache and share on the future glyph-run hot path. Callers apply scale/position via IGeometryImpl.WithTransform or a drawing-context transform, and draw through the existing DrawGeometry(IGeometryImpl) overload.
Round-trip, table patch/replace/remove/truncate, and asset-load tests for the synthetic-font harness over Inter-Regular and InterVariable.
numPoints is derived from the last contour endpoint alone, so a glyph whose endPtsOfContours array is not strictly increasing sized its point buffers too small and sent every consumer walk indexing past them - an IndexOutOfRangeException on each outline build of that glyph. Validate the endpoints up front and return the default (empty) glyph instead.
Review feedback on the glyph-outline API: the ushort values these members take are glyph indices, and the platform already speaks that language (GlyphRun.GlyphIndices), so the parameters say what they are instead of the looser glyphId/glyphIds. Applies to GetGlyphOutline and the advance/metrics members this series reworks, plus the tests exercising them. Purely a naming change - no call site uses named arguments.
0d3133b to
556b5df
Compare
miloush
left a comment
There was a problem hiding this comment.
only some potential bound checks, otherwise LGTM
The off-curve-start branch of the glyph walker drops one quadratic per extra point in runs of consecutive off-curve points and ignores an on-curve last point when picking the figure start. Pins the correct TrueType decomposition; the on-curve-start case is pinned as-is.
Both geometry emitters carried the same on-curve/off-curve branch pair, and the off-curve branch had two defects: it advanced past both points of an off-curve pair (dropping one quadratic per extra point in longer off-curve runs) and always started at the implied midpoint even when the last point of the contour is on-curve. One shared walker now does the TrueType decomposition with a pending-control state machine and an explicit close, and both the simple-glyph path and the point-matching composite path route every contour through it.
The counts in a simple glyph header and in maxp are untrusted: a body shorter than they promise currently throws out of SimpleGlyph.Create, and a short loca table reports a glyph count it cannot index.
SimpleGlyph.Create bounds-checks the endpoint array, the declared instruction run and every flag/coordinate read, returning default (and the rented buffers) instead of throwing when the body is shorter than its untrusted counts promise. LocaTable clamps its glyph count to the entries the table data actually covers, replacing an empty validation block that only claimed to handle short tables.
The OpenType spec assigns bit 6 (0x40) OVERLAP_SIMPLE; only bit 7 is reserved.
# Conflicts: # tests/Avalonia.Base.UnitTests/Media/GlyphTypefaceTests.cs
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
Part 2 of 13 of the glyph-outline / variable-font workstream. Stacked on Part 1 (#21405); please merge that one first.
What does the pull request do?
Adds a public API on
GlyphTypefaceto retrieve a glyph's vector outline, parsed directly from the font'sglyfandlocatables:The outline comes back in the font's design-unit space (Y-up, no transform applied) as an immutable, cacheable geometry. This is the foundation for higher-level features that need per-glyph geometry — custom path effects, exporting glyph paths, driving an
IGlyphRunImplfrom outlines on backends without native glyph-run primitives, and (in the color-glyph PR) drawing the outline layers of COLR v0 glyphs.What is the updated/expected behavior with this PR?
GetGlyphOutline(glyphIndex)returns the vector outline ofglyphIndexin font design units (Y-up). Callers position it themselves — scale byemSize / DesignEmHeight, flip Y, translate to the pen origin — viaIGeometryImpl.WithTransform(matrix)or a drawing-context transform, and draw it through the publicDrawGeometry(IBrush?, IPen?, IGeometryImpl)overload.Returns
nullwhen:glyftable (e.g. CFF / CFF2 — out of scope for this PR), orVariable-font axis configuration is intentionally not a parameter here — it lives on the
GlyphTypefaceinstance itself (alongside font weight / style / stretch / simulations), resolved through the font collection in the variable-font sub-stack (Parts 6–11).How was the solution implemented (if it's not obvious)?
IGeometryImpl, not aGeometry. The result is a thin read-only wrapper (ImmutableGeometryImpl) over the built platform geometry.IGlyphRunImplfallback for backends without native glyph rasterization) needs, so the API is committed to it now rather than reshaping a public signature later.glyf/locareader is split into focused pieces per concern: table loader, simple-glyph decoder, composite-glyph traversal.return null.locaoffsets — nothing is materialized up front. A pooled per-call decycler and a pooled resolved-outline buffer keep the hot path allocation-free.GetGlyphOutlinedoesn't pay a service-locator lookup per call.loca/glyfparsing; render tests rasterize representative glyphs through Skia and image-diff them — Latin, a recursive composite (Á), a CJK ideograph, the variable-default instance, and a purpose-builtPointMatch.ttfwhosePis assembled by point matching. No production font encodes point matching, so a synthetic fixture is the only way to exercise that build path end-to-end; a broken implementation would drop the matched component at the origin, which the image diff catches.Checklist
Breaking changes
None. New public API; existing behavior is unchanged.
Obsoletions / Deprecations
None.
Fixed issues