Skip to content

fix non-determinism in FeatureParamsCharacterVariants nameIDs#1572

Merged
anthrotype merged 1 commit into
mainfrom
fix-feature-params-char-vars
Jul 23, 2025
Merged

fix non-determinism in FeatureParamsCharacterVariants nameIDs#1572
anthrotype merged 1 commit into
mainfrom
fix-feature-params-char-vars

Conversation

@anthrotype

@anthrotype anthrotype commented Jul 22, 2025

Copy link
Copy Markdown
Member

Part of fixing #1568

We were iterating over HashMap's items while incrementally assigning new nameIDs for the Feature Parameters Character Variants, but the iteration order changes each time, so best to use a BTreeMap in this case

for (tag, cv_params) in self.character_variants.iter() {
let params = cv_params.build(name_builder);
result.insert(*tag, FeatureParams::CharacterVariant(params));
}

After applying this patch, running the following command multiple times produces the same GSUB table:

python3 resources/scripts/ttx_diff.py 'https://github.com/calcom/font?b833fb1129#sources/Cal-Sans.glyphs'

JMM

Part of fixing #1568

We were iterating over a HashMap items while incrementally assigning nameIDs for Feature Parameters Character Variants, but the iteration order changes each time, best to use a BTreeMap in this case
@anthrotype anthrotype force-pushed the fix-feature-params-char-vars branch from 00c6112 to 086b5ec Compare July 22, 2025 16:26
@anthrotype anthrotype requested a review from cmyr July 22, 2025 16:26

@cmyr cmyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, good call :)

@anthrotype anthrotype added this pull request to the merge queue Jul 23, 2025
Merged via the queue into main with commit b208b62 Jul 23, 2025
12 checks passed
@anthrotype anthrotype deleted the fix-feature-params-char-vars branch July 23, 2025 08:24
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.

2 participants