Repository navigation
Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets - #298
Merged
Merged
Conversation
A MultiItemVariationStore delta set that ended before it filled its tuples (one per region of its subtable) added nothing: the reader checked the whole set first. HarfBuzz's MultiVarData::get_delta adds what it finds, region by region, so on a crafted font a component moved by its first values in HarfBuzz and not at all in sigilbuzz. The reader now walks such a set as HarfBuzz's TupleValues::fetcher_t does: a region's tuple stops where the stream does, and a run whose values do not fit the bytes left ends that region's tuple (or the lazy skip past regions whose scalar is zero) after its control byte, so the next region's tuple starts at the byte after it. It used to end the stream there. A Value condition now adds the summed deltas to its default value, as HarfBuzz's instancer returns them, instead of adding each region's delta to the default in turn; the two can round differently in f32. The sigilbuzz-subset round-trip test that encoded the old reading (each delta set holds one value for two translations) now expects HarfBuzz's: at axis 0 = 1 each set's value moves X and Y keeps its stored value, which also checks that every remapped variation index still reads its own set.
Face::glyph_outline_at_coords resolved a nested VARC composite by calling itself with the component's coords, so Varc::composite took them as the font's: a RESET_UNSPECIFIED_AXES component one or more composites down started from its parent's coords instead of the font's. A component naming its own glyph recursed until the 64-level cap and failed the glyph, where HarfBuzz draws that glyph's plain outline. A VARC glyph is now drawn by one walk (src/face/varc_draw.rs), as HarfBuzz's VARC::get_path_at walks it: - Every composite is resolved with composite_with_font_coords and the coords of the glyph drawn, so a reset starts from the font's coords however deep it sits. - A component whose glyph is its parent's draws that glyph from glyf or CFF at the component's coords. - Transforms compose top down, in hb_transform_t's order, and each leaf is drawn once through a sink that places its points (x0 + xx*x + xy*y, as HarfBuzz's transform_point sums them) instead of copying every nested outline into its parent's. - The leaves' tables (CFF2, CFF, or glyf with loca, hmtx, vmtx and, away from the default instance, gvar) are read once per walk, in the order the face reads them, instead of once per leaf. The component, op and depth caps are unchanged, except that a leaf's ops now count once rather than once per level they were copied through. GlyphOutlines draws a VARC glyph with the same walk and its cached VARC table, streaming into the sink, instead of resolving the composite once to test it and then asking the face.
The condition and delta budget from #293 was per composite_with_font_coords call, and one Face outline call makes up to 2,049 of them, so a glyph whose 2,048 components each reached heavy condition or delta work could spend 2,049 budgets: 0.5 s for a 200 KB font, 1.2 s for 64 nested levels of 31 components, 0.25 s for 2,047 components naming a glyph of 20,000 gated-off components (release build, HarfBuzz-style hostile fonts built for this change). A walk now holds one VarcMemo for every composite it resolves: - One work budget of 2^20 units and one condition-visit budget of 65,536. Besides the region scalars and delta values #293 charged, units now pay for each component record read and each axis value in it, each offset of an And or Or condition, each coord a component's coord vector holds, the axes a region scalar evaluates, and each coord vector the memo keeps. Every one of these was free or charged per call before, so the walk's total work is bounded by the budget however its components share children. - What depends only on coords, kept by coord vector for the whole walk: condition results, region scalars, and resolved composites (an Rc per glyph). A glyph reached again at the same coords, from any parent, costs nothing more. Out of work, the delta that ran out adds nothing, a condition that needed it fails, and no more components are read, so the composite being read and every later one ends there. A walk holds at most one budget's worth of coords and composites, so it stays small: the worst of the hostile fonts peaks at 4.1 MB of heap and every one of their glyphs draws in under 14 ms. Varc::composite_with_font_coords is one walk over one composite, with the same budget. Its per-composite cap of 2^20 coord values is gone: the coords are charged to the budget instead, so a record of 4096-axis components now stops at 255 of them rather than 256. Tests count the work: 600 components reaching a glyph at distinct coords whose condition and deltas cost 180,047 units spend the budget after six of them (one budget per composite let them spend 108 million units), and 600 reaching it at the same coords resolve it once, for 181,848 units.
Two more differences from HarfBuzz 14.5.0's VARC::get_path_at, found while checking the walk against it: - A cycle longer than a self-reference (A names B, B names A) recursed until the 64-level cap and failed the glyph. HarfBuzz walks the cycle until its hb_decycler_t, a tortoise moving at half the speed of the path, meets the glyph again: a covered glyph at position k of the path of glyphs being drawn is skipped when the glyph at position k / 2 is the same. The walk keeps that path and applies the same rule, so a two-glyph cycle draws its leaves twice over and stops, as in HarfBuzz. - A glyph the VARC coverage names past the end of the glyph records fell back to its glyf or CFF outline. HarfBuzz reads such a record as empty and draws nothing, at the top or as a component. Varc's composite and composite_with_font_coords now return no components for it (still None for an uncovered glyph, as documented).
hb_font_create sets a face's default variations, so a HarfBuzz font with fvar holds one coord per axis even at the default instance, and VARC applies deltas whenever the font has coords: a region that constrains no axis holds there, and its deltas apply. sigilbuzz reads coords that all round to zero as no coords at all, so the same glyph skipped those deltas (a component on such a region drew 100 units off in a crafted font). A font without fvar has no coords in HarfBuzz either, and still gets none. The walk now starts from one zero per fvar axis when the coords are empty, for Face::glyph_outline_at_coords and GlyphOutlines alike. The walk's work-count tests move with it (their font has three axes, so its coord vectors cost three units each).
tests/fixtures/varc_face.ttf has the axes of varc_parity.ttf and 41 glyphs the VARC coverage names: reset components two and three composites down, a reset in the middle of a chain and one whose axis values vary; components naming their own glyph (which carries a varied glyf outline of its own), at the top, one composite down, and with a reset; cycles of two and three glyphs; a covered glyph past the glyph records and a glyph using it; delta sets that end early, cut a run in the first, a middle or the last tuple, are empty, or cut an i32 run, for translations, axis values and a Value condition; conditions read inside nested composites, one gating a reset component; one glyph reached three times at the same coords and once elsewhere; and a region without axes, for a translation, a condition and a nested composite. tests/tools/build_varc_face_fixtures.py writes it byte by byte with the encoders of build_varc_morx_parity_fixtures.py, and tests/tools/varc_face_expected.py records HarfBuzz 14.5.0's outlines (uharfbuzz 0.56.2) at the eight locations varc_parity.expected uses. tests/varc_parity.rs now checks every glyph of both fonts through Face::glyph_outline_at_coords, GlyphOutlines::outline and GlyphOutlines::draw, and the parity font also through composite_with_font_coords. v_outer, the nested reset, no longer needs its own path. Before this branch the new font matched HarfBuzz on 14 to 22 of its 41 glyphs per location; now all 41 at all eight.
A subtable's region scalars were deduplicated through a BTreeMap, one insert per distinct region, which made them the costliest unit of the walk's budget. A crafted font whose glyph names, 2,047 times at distinct coords, a glyph varying through a subtable of 30,000 distinct regions took 16.8 s on main and 34 to 42 ms with the shared budget. The region slots and positions are now packed into one u32 each (both fit 16 bits) and sorted once, and each distinct region is evaluated as the sorted list reaches it. The font above now takes 11 to 13 ms per glyph, and every hostile font built for this change draws its slowest glyph in under 19 ms. The scalars and the axis count charged for them are unchanged; a new test checks that scalars keep the subtable's order with indexes out of order, repeated, and past the region list.
albedosehen
marked this pull request as ready for review
October 4, 2026 23:19
albedosehen
added a commit
that referenced
this pull request
Oct 5, 2026
Fix the VARC review findings of #298
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.
Closes #296
This merged at
e0ae353. The fixes from its independent review (an unparsableVARCfailing every glyph, rotation accuracy past 90 degrees, leaf work, record floods, and the cycle wording corrected below) are in #300.Finishes VARC through
Face(the follow-ups #293 left forsrc/face/outlines.rs): a VARC glyph is now drawn by one walk that resolves every composite in the font's coords, draws a self-referencing component's plain outline, shares one work budget across the whole glyph, and reads short delta sets as HarfBuzz 14.5.0 does. Checking that walk against HarfBuzz'sVARC::get_path_atturned up three more differences, fixed in their own commits: HarfBuzz's decycler, covered glyphs without a record, and VARC deltas at the default instance. Outlines of fonts without VARC are byte-identical to main through every path.Commits
TupleValues::fetcher_treads it (a run cut short ends that region's tuple after its control byte, and the next tuple starts at the byte after it). A Value condition adds the summed deltas to its default, in HarfBuzz's order. Thesigilbuzz-subsetround-trip test now expects HarfBuzz's reading.src/face/varc_draw.rs: every composite is resolved withcomposite_with_font_coordsand the font's coords, so a reset component starts from them however deep it sits; a component naming its parent's glyph draws that glyph fromglyf/CFF; transforms compose top down inhb_transform_t's order and each leaf is drawn once through a placing sink; leaf tables are read once per walk.GlyphOutlinesdraws VARC glyphs with the same walk and its cachedVarc.VarcMemoper walk: one 2^20-unit work budget and one 65,536-visit condition budget for every composite the glyph reaches, with component records, axis values, And/Or offsets, coord vectors and region axes now charged; condition results, region scalars and resolved composites kept by coord vector, so a glyph reached again at the same coords is resolved once.hb_decycler_t(a covered glyph at position k of the path is skipped when the glyph at k / 2 is the same), and a glyph the coverage names past the glyph records draws nothing.hb_font_creategives a font withfvarone coord per axis even at the default, so VARC deltas from a region that constrains no axis apply there; the walk now does the same.BTreeMap, the costliest unit of the budget (34 to 42 ms for a glyph over 30,000 distinct regions); slots and positions are now packed intou32s and sorted once.tests/fixtures/varc_face.ttf(41 VARC glyphs), its builder and HarfBuzz expected file;tests/varc_parity.rschecks both fonts through all three outline paths.Design
Face::glyph_outline_at_coordsandGlyphOutlines::drawhand a glyph the VARC coverage names toVarcDraw(src/face/varc_draw.rs), which walks it asVARC::get_path_atdoes:Varc::resolve(gid, coords, font_coords, memo), the font's coords fixed for the walk, soRESET_UNSPECIFIED_AXESstarts from them at any depth. Empty coords become one zero perfvaraxis.glyf/CFF at its coords. Apathof the glyphs being drawn applies HarfBuzz's decycler rule.hb_transform_t::multiply's sums) and draws each leaf once through a sink that maps its points (x0 + xx*x + xy*y) and counts its ops against the 2^20-op cap. Leaf tables (CFF2, CFF, or glyf/loca/hmtx/vmtx, and gvar for the first leaf away from the default) are read once per walk, in the face's order, errors included.VarcMemo(insrc/tables/varc.rs) holds the walk's work and visit budgets and, per coord vector (keyed byf32bits), condition results by table offset, region scalars by subtable, and composites by glyph (Rc). Keeping a coord vector costs one unit per value, so the memo is bounded by the budget. Out of work: the delta that ran out adds nothing, a condition that needed it fails, and the composite being read (and every later one) ends, as HarfBuzz stops when its budget goes negative.Varc::composite_with_font_coordsis one walk over one composite with the same budget.Malformedas before. A leaf's ops now count once instead of once per level they were copied through.Evidence, before and after
Outlines compared with HarfBuzz 14.5.0 (uharfbuzz 0.56.2,
draw_glyph_with_pen), same op sequence and every coordinate within 0.01 units, at the eight locations ofvarc_parity.expected. "Before" is main at 8b04190.varc_parity.ttf(#293's 14-glyph font):Face::glyph_outline_at_coordsv_outer, the nested reset, up to 31.5 units off)GlyphOutlines::outlineGlyphOutlines::drawv_nested)varc_face.ttf(new, 41 VARC glyphs), glyph-locations matching HarfBuzz throughFace(the twoGlyphOutlinespaths give the same counts):glyfoutline)Largest difference on matching glyphs after: 3.3e-5 units. HarfBuzz's own self-reference regression font,
test/api/fonts/varc-static-gvar.ttf(glyphanames itself with an axis value): before,Facefailed it with the depth-cap error; after, it matches HarfBuzz.Hostile fonts (built for this change with #293's encoders; release build, the slowest glyph of each at the default and at AAAA=100 through
Face::glyph_outline_at_coords,GlyphOutlineswithin the same range, on a laptop shared with other work, so ranges are over two runs; peak heap by a counting allocator):distinct_regions(526 KB): 2,047 components at distinct coords naming a glyph whose translation varies over 30,000 distinct regionsfanout_cond(200 KB): 2,047 components at distinct coords, each reaching a glyph gated by 20 shared And levels over a 60,000-mention Value condition and translated through the same setfanout_cond_same: the same, all at one coordsgated_flood(115 KB): 2,047 components naming a glyph of 20,000 gated-off componentsdeep_chain(197 KB): 64 nested glyphs of 31 components each at distinct coords over the conditionwide_coords(17 KB): 2,047 components with 4,096-axis coords naming a glyph of 200 moreself_fan: 2,047 components naming their own glyphcycle_fan: 40 x 40 components in a two-glyph cyclecondition_attack.ttf(182 KB)Every glyph of every one of these fonts draws in under 19 ms after, against up to 16.8 s before.
Work-count tests (no timing): in
src/face/varc_draw.rs, 600 components reaching a glyph at distinct coords whose condition and deltas cost 180,053 units there spend the shared 2^20 budget after six copies (one budget per composite let them spend 108 million units), visiting 246 condition tables; 600 reaching it at one coords resolve it once (183,057 units, 41 visits). Insrc/tables/varc/tests.rs, the #293 counts move with the new charges (the 20-level chain: 41 visits, 120,045 units), a memo test checks that a glyph at the same coords costs nothing more, and the run-cut-short reading is pinned byte by byte.Fonts without VARC: byte-identical to main. Every glyph (or every 2nd to 53rd for large fonts), at 1 to 3 settings each, through
glyph_outline,glyph_outline_at_coords,GlyphOutlines::outline,GlyphOutlines::drawandglyph_bounds_at_coords, floats printed exactly: 26 of 26 dumps identical. Fonts: Fraunces and Fraunces Italic, Hahmlet, Nabla (COLR), Noto Sans KR VF (CFF2 and glyf), Recursive, Roboto Flex, Source Serif 4 VF (CFF2), Arial, Segoe UI, Segoe UI Emoji (COLR), Calibri, Georgia, Consolas, Bahnschrift, Sitka VF, Gabriola, Ebrima, Malgun Gothic, and the fixtures Source Code Pro (CFF), Source Sans 3 VF (CFF2), CidCff1Synthetic (CID CFF), the Noto Sans KR CFF2 subset, the Hahmlet subset and Rubik VF.Verification
cargo fmt --all -- --check: clean.cargo +1.99.0 clippy --workspace --all-targets --all-features -- -D warnings, the same with--no-default-features, andcargo +1.99.0 clippy -p sigilbuzz --no-default-features -- -D warnings: clean.cargo test --workspace --all-features: 3,817 passed, 0 failed, 9 ignored, 200 test binaries.cargo build --no-default-features: ok.CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback cargo +stable generate-lockfile, thencargo +1.81 check --workspace --lib --no-default-featuresand--lib --bins --all-features): ok.cargo doc -p sigilbuzz --no-deps: no new warnings (one existing, fromglyph_points).rustlang/rust:nightly, cargo-fuzz,-O -a,-max_len=65536 -rss_limit_mb=2048 -timeout=10, no--privileged),faceandshape, 600 s each per round. Seeds: the VARC, morx, sbix and COLRv1 fixtures,varc_face.ttf, the hostile fonts under 64 KB and HarfBuzz'svarc-static-gvar.ttf, each also with zeroed and set control bytes. Three rounds: on the walk before the default-coords and scalar commits,face38,402 runs andshape250,148; on 6420629, 31,321 and 207,051; on the head e0ae353, 28,054 and 183,892. Peak RSS at most 618 MB. No crashes, timeouts or OOMs in any round.faceruns slowly on these seeds (30 to 60 a second) because it draws every glyph of the hostile fonts through every path.Not done
Malformed. HarfBuzz stops expanding at 64 levels and 16,384 edges and stops drawing when its 2^24-unit glyph budget (shared with the leaves) runs out, and returns what it drew. With the decycler, cycles of up to 32 glyphs no longer reach these caps; longer ones still reach the 64-level cap first (corrected after review; documented in Fix the VARC review findings of #298 #300).Face::varcerrors), where HarfBuzz 14.5.0's sanitizer drops the table and drawsglyf. Correction: HarfBuzz'stest/api/fonts/varc-6868.ttfand thevarc-ac0*fonts are in the MultiItemVariationStore layout HarfBuzz adopted after 14.5.0 (bd079077d2), not an older one; the copies at tag 14.5.0 parse in sigilbuzz.glyfheader box for aglyffont and the control box of the VARC outline for a CFF font. The outline the extents read for CFF fonts does get every fix here.varc-short-cff2.otf(HarfBuzz test font): HarfBuzz 14.5.0 draws nothing for its glyphs because its CFF2 charstring count differs frommaxp; sigilbuzz draws them. A CFF2 difference, not VARC.fvar(itsvarc-static-deltastest); 14.5.0 does not, and neither does sigilbuzz.GlyphOutlinesdraws VARC leaves from tables its walk reads once per glyph, not from the tables it keeps for all glyphs (its CFF2 region scalars are for one coord vector, and leaves have their own).Face::glyph_outline_at_coordsstill parsesVARConce per call.CHANGELOG lines
Changed:
Face::glyph_outline_at_coordsandGlyphOutlinesdraw a VARC glyph in one walk that shares one work budget, 2^20 units, across every composite the glyph reaches, and resolve a glyph reached again at the same coords once (Finish VARC through Face: nested reset, self-reference, a shared work budget, short delta sets #296, Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets #298). The budget was per composite, and a glyph can reach 2,049 of them, so a crafted glyph took up to 17 s; the worst crafted glyph now draws in under 19 ms and holds 4 MB at most. Component records, axis values, And and Or condition offsets, coord vectors and region axes now count toward the budget too. Past it, the composite being read and every later one ends, as in HarfBuzz.Varc::composite_with_font_coordscharges the same work to its 2^20 units, so a composite's coord vectors count there instead of against a separate cap of 2^20 values, and it gives no components (rather thanNone) for a glyph the coverage names past the end of the glyph records, which HarfBuzz draws as nothing (Finish VARC through Face: nested reset, self-reference, a shared work budget, short delta sets #296, Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets #298).MultiVarStore::resolve_deltasadds the values a delta set holds when it ends before filling its tuples, as HarfBuzz does, instead of nothing; a run cut short ends the tuple it is in after its control byte (Finish VARC through Face: nested reset, self-reference, a shared work budget, short delta sets #296, Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets #298).Fixed:
RESET_UNSPECIFIED_AXEScomponent one or more composites down starts from the font's coords throughFace::glyph_outline_at_coordsandGlyphOutlines, as in HarfBuzz; it started from its parent's (Finish VARC through Face: nested reset, self-reference, a shared work budget, short delta sets #296, Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets #298).glyfor CFF outline, as HarfBuzz does, instead of failing the glyph at the 64-level depth cap; longer cycles of up to 32 glyphs stop where HarfBuzz's decycler stops them (Finish VARC through Face: nested reset, self-reference, a shared work budget, short delta sets #296, Finish VARC through Face: nested reset, self-reference, one work budget, short delta sets #298).