Skip to content

[skera] Preserve alternate feature parameters and their names - #2278

Closed
behdad wants to merge 17 commits into
skera-layout-constant-devicesfrom
skera-alternate-feature-params
Closed

behdad wants to merge 17 commits into
skera-layout-constant-devicesfrom
skera-alternate-feature-params

Conversation

@behdad

@behdad behdad commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Alternate feature tables inherit their parameter format from the original feature tag. Skera previously resolved them with a placeholder tag, which made ordinary subsetting fail and instancing silently discard valid parameters. Carry the original tag through GSUB/GPOS subsetting and ownership, and retain names referenced by retained alternate features.

Tests cover size, ss01, and cv01 parameters in both tables, feature-index remapping, full and partial instancing, axis ranges, and partial-then-full instancing. All eight complete HarfBuzz table references now include alternate parameters; ordinary and partial references were refreshed after the upstream HarfBuzz fixes. The original parameter regressions fail on the preceding implementation.

Self-review also found that feature-variation ownership accepted unsupported major versions and could panic while accessing an oversized declared record array on 32-bit hosts. Check the version and array extent before generated range access. The regression fails before the fix and passes on native x86_64 and actual i686. The general generated-reader overflow is addressed separately in #2392.

Validation at this PR: all 530 Skera tests with all features, the i686 malformed-header regression, Clippy with warnings denied, Rust 1.85 all-target/all-feature checks, formatting, and typo checks pass. Downstream propagation checks compilation and the header/feature-parameter regressions at each affected boundary; the final refactored tip also runs the full Skera suite.

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:18
@behdad
behdad force-pushed the skera-alternate-feature-params branch from 546afe3 to e3dca79 Compare October 10, 2026 18:08
@behdad
behdad requested a review from qxliu76 October 10, 2026 18:10
@behdad
behdad force-pushed the skera-alternate-feature-params branch from e3dca79 to 066756a 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-alternate-feature-params branch from 066756a to 8ec9c1b Compare October 12, 2026 20:41
@behdad
behdad force-pushed the skera-alternate-feature-params branch from 8ec9c1b to 0d425dc Compare October 12, 2026 20:53
@behdad
behdad force-pushed the skera-alternate-feature-params branch from 0d425dc to 578e389 Compare October 12, 2026 22:04
@behdad
behdad force-pushed the skera-alternate-feature-params branch from 578e389 to 8d888f1 Compare October 14, 2026 23:11
behdad added 15 commits October 15, 2026 14:52
Propagate reviewed fixes and repaired ancestry through #2278.

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
@behdad

behdad commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Self-review findings before fixes:

  • Passing the original feature tag when resolving alternate tables is the right model. I checked feature-index remapping, size/ssXX/cvXX name closure, and the OFF 2026 section 6.2.11 interaction between legacy alternate parameters and lookup variations. The implementation applies the parameter substitution first and then constructs the lookup set, using the current feature's lookups when ADD_DEFAULT_LOOKUPS is set, as specified.
  • The owned FeatureVariations conversion currently accepts an unsupported major version and serializes it as a supported version. The new regression fails before the correction. This should be rejected at the ownership boundary.
  • Its legacy array getter is called before validating the encoded count's byte extent. On 32-bit hosts, the generated range calculation can overflow for a tiny malformed table with a very large count. I am adding a bounds check before invoking that getter and checking the case on a 32-bit target.
  • The differential test deliberately clears alternate parameters because its older HarfBuzz reference build dropped them. That upstream bug is already fixed by merged HarfBuzz #6349 / #6350, and FontTools #4278. I will regenerate the references and compare the complete parameter-bearing layout tables instead of clearing those fields. There is no need to file duplicate upstream PRs.

…ameters

Reject unsupported major versions and oversized legacy record arrays before
calling generated range getters. The malformed count overflow is reproduced
on i686; the ownership guard and valid empty arrays pass on i686 and x86_64.

Refresh the eight HarfBuzz references after the upstream alternate-parameter
fixes and compare full parameter-bearing tables without clearing fields.

All 530 Skera tests, Clippy, Rust 1.85 checks, formatting, and typos pass.

Assisted-by: OpenAI Codex
@behdad

behdad commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Self-review fixes are now validated and propagated through the stack. Feature-variation ownership rejects unsupported major versions and oversized record arrays before generated range access. The malformed-header regression reproduces the original acceptance bug on x86_64 and the original overflow panic on actual i686; both pass after the guard. The general generator/read-fonts fix is in #2392.

The feature-parameter comparisons now check complete alternate parameters against all eight current HarfBuzz references. The previous normalization that stripped those parameters has been removed; the relevant HarfBuzz and FontTools parameter fixes already landed, so no duplicate upstream PR is needed.

This PR passes all 530 Skera tests, Clippy with warnings denied, Rust 1.85 all-target/all-feature checks, formatting, and typo checks. Every affected downstream boundary compiles and passes the ownership/parameter regressions. The final refactored tip passes 627 Skera tests, Clippy, MSRV, formatting, typo checks, and the i686 header regression. All affected branches were pushed atomically after validation. The design document records the ownership boundary and validation scope.

@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