Skip to content

[skera] Preserve fractional layout condition thresholds - #2279

Closed
behdad wants to merge 16 commits into
skera-alternate-feature-paramsfrom
skera-layout-condition-scaling
Closed

behdad wants to merge 16 commits into
skera-alternate-feature-paramsfrom
skera-layout-condition-scaling

Conversation

@behdad

@behdad behdad commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Partial instancing must preserve a variable-value condition’s sign boundary before integer rounding. A condition could previously become constant when its residual deltas rounded to zero, or move its threshold when its folded default was fractional.

Follow HarfBuzz’s condition instancing algorithm: test unrounded residual deltas, positively rescale the expression to preserve fractional values, and give it a private store row when scaling or a large default requires one. Keep shared positioning rows unchanged. Cache matching condition rows and use a neutral region for defaults outside the signed 16-bit field.

Validation: 219 unit tests and focused BASE, conditions, feature-parameter, and layout integration tests pass. The new regression fails on the previous implementation. Boundary tests cover shared rows, fractional defaults, positive/negative quarter-unit residuals, and large defaults; 180 fresh HarfBuzz partial-then-full cases match GSUB/GPOS output. Clippy, rustfmt, and the CI typo checker pass.

Readers use read-fonts directly. write-fonts handles font assembly and owned table rebuilding; Skera retains its direct serializers and repacker. There is no separate generated Skera table tree.

@behdad
behdad added this pull request to stack #2267 October 10, 2026 05:26
@behdad
behdad force-pushed the skera-layout-condition-scaling branch from 9ed9e1d to e439bc7 Compare October 10, 2026 18:08
@behdad
behdad requested a review from qxliu76 October 10, 2026 18:10
@behdad
behdad force-pushed the skera-layout-condition-scaling branch from e439bc7 to 2f1ab92 Compare October 12, 2026 03:19
@behdad
behdad removed this pull request from stack #2267 October 12, 2026 03:28
@behdad
behdad added this pull request to stack #2335 October 12, 2026 03:29
@behdad
behdad force-pushed the skera-layout-condition-scaling branch 3 times, most recently from 906c63e to d933abf Compare October 12, 2026 22:04
@behdad
behdad force-pushed the skera-layout-condition-scaling branch from d933abf to 98e8823 Compare October 14, 2026 23:11
behdad added 13 commits October 15, 2026 14:52
Resolve integration conflicts without dropping validated parent fixes.

Assisted-by: OpenAI Codex
This tree is unchanged; the replacement evaluator already handles missing
variation rows. Preserve the repaired parent ancestry.

Assisted-by: OpenAI Codex
Preserve reviewed parent ancestry and each PR's existing feature changes.

Assisted-by: OpenAI Codex
Preserve the independently validated TrueType changes and all feature code.

Assisted-by: OpenAI Codex
Keep empty record counts and allocation bounds at each PR boundary.

Assisted-by: OpenAI Codex
Move the existing #2338 correction into the width-reduction PR.

Assisted-by: OpenAI Codex
Reject unsupported versions and oversized arrays before ownership.
Keep current HarfBuzz references with complete feature parameters.

Assisted-by: OpenAI Codex
@behdad

behdad commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Self-review findings before further fixes:

  • The private condition rows are the right boundary: positive rescaling must not change positioning values that share the original VariationIndex. Caching by source index plus default value and keeping unrounded residuals are justified. The existing fractional and large-default cases exercise those choices.
  • There is a scalar-precision inconsistency to fix. A valid synthetic 20-axis font has one positive region and a format-2 condition with default 0 and delta 1. Pinning 19 axes at 1/256 makes the source float32 region product underflow to zero for every remaining location. Full Skera and HarfBuzz instancing correctly select the default substitution. Skera partial-then-full instancing instead rescales the double-precision residual and selects the alternate substitution; HarfBuzz keeps the default. The shared store plan must honor a zero float32 pinned-axis scalar before rescaling conditions. I will add a public composition regression and preserve double precision for the surviving transforms.
  • Further probes with nonzero subnormal products show that positive rescaling can expose float32 underflow thresholds in both tools. This is a separate precision boundary from the all-locations-zero bug above and needs an explicit treatment rather than silently claiming exact composition for arbitrary float underflow.
  • FontTools currently ignores unsupported variable-value conditions and can emit the wrong substitution even for a simple valid one-axis full instance. Its current main still has that behavior. I am preparing an upstream PR to reject unsupported condition formats before mutating the font; this does not claim to implement the new condition formats.

The earlier complete feature-parameter comparisons and ownership guards remain part of this branch. This review is not complete until the new regression, propagation and checks pass.

Honor float32 pinned-axis products before carrying double-precision
residuals into condition scaling. Drop regions that reach zero. Reject
partial condition instancing when nonzero subnormal pinned scalars
would lose their underflow threshold after positive rescaling.

Add public full/composed regressions for zero scalars, nonzero defaults,
and explicit subnormal rejection. Both regressions fail before their
fixes. All 534 Skera tests, Clippy, MSRV, formatting and typo checks pass.

Assisted-by: OpenAI Codex
@behdad

behdad commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

The scalar fixes are now pushed through the stack: regions whose pinned float32 product reaches zero stay inactive, and partial conditions with nonzero subnormal pinned products are rejected instead of being rescaled across their underflow boundary. Full instancing remains supported. Both public regressions fail before their respective fixes. This PR passes 534 Skera tests, Clippy and MSRV; the final combined tip also passes the three partial-condition regressions on actual i686.

Upstream fixes are open in HarfBuzz #6441 and FontTools #4348. HarfBuzz's full Meson suite passes 272 tests with one skip; all FontTools CI checks are green. The remaining stack audit continues.

@behdad

behdad commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by the rebuilt, consolidated instancing stack in #2416–#2423. MATH and name-table overrides remain separately covered by #2415 and #2411.

@behdad behdad closed this Oct 28, 2026
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.

1 participant