Skip to content

[ttx_diff.py] only normalize contour order when contours perfectly match between fontc vs fontmake#1654

Merged
anthrotype merged 2 commits intomainfrom
no-reorder-contours-unless-matching
Sep 25, 2025
Merged

[ttx_diff.py] only normalize contour order when contours perfectly match between fontc vs fontmake#1654
anthrotype merged 2 commits intomainfrom
no-reorder-contours-unless-matching

Conversation

@anthrotype
Copy link
Member

@anthrotype anthrotype commented Sep 25, 2025

Fixes part of #1653

Previously, ttx_diff.py would normalize contour order within each compiler's output independently, which could create misleading diffs when the contours actually differed (e.g., different starting points).

This change compares contours between fontc and fontmake first, and only normalizes the order of whole contours when the set of contors serialized as xml strings is identical. When contours differ, they are left in their original order to make the actual differences more apparent.

The ttx diff for Sirivennela-Regular.ufo now looks more sane and makes the different starting points more evident:

--- /Users/clupo/oss/fontc/build/default/fontc.glyf.ttx	2025-09-25 11:14:20
+++ /Users/clupo/oss/fontc/build/default/fontmake.glyf.ttx	2025-09-25 11:14:20
@@ -3206,6 +3206,7 @@

     <TTGlyph name="ellipsis" xMin="12" yMin="0" xMax="388" yMax="66">
       <contour>
+        <pt x="342" y="0" on="0"/>
         <pt x="322" y="20" on="0"/>
         <pt x="322" y="46" on="0"/>
         <pt x="342" y="66" on="0"/>
@@ -3213,9 +3214,9 @@
         <pt x="388" y="46" on="0"/>
         <pt x="388" y="20" on="0"/>
         <pt x="368" y="0" on="0"/>
-        <pt x="342" y="0" on="0"/>
       </contour>
       <contour>
+        <pt x="188" y="0" on="0"/>
         <pt x="169" y="20" on="0"/>
         <pt x="169" y="46" on="0"/>
         <pt x="188" y="66" on="0"/>
@@ -3223,9 +3224,9 @@
         <pt x="235" y="46" on="0"/>
         <pt x="235" y="20" on="0"/>
         <pt x="216" y="0" on="0"/>
-        <pt x="188" y="0" on="0"/>
       </contour>
       <contour>
+        <pt x="32" y="0" on="0"/>
         <pt x="12" y="20" on="0"/>
         <pt x="12" y="46" on="0"/>
         <pt x="32" y="66" on="0"/>
@@ -3233,7 +3234,6 @@
         <pt x="78" y="46" on="0"/>
         <pt x="78" y="20" on="0"/>
         <pt x="58" y="0" on="0"/>
-        <pt x="32" y="0" on="0"/>
       </contour>
       <instructions/>
     </TTGlyph>

I also exposed the --keep-direction flag to ttx_diff.py, so it gets forwarded to fontc and fontmake. I used underscore instead of hyphen because absl.flags wants that.

JMM

with underscore, not hyphen, because absl.flags is particular
Previously, ttx_diff.py would normalize contour order within each
compiler's output independently, which could create misleading diffs
when the contours actually differed (e.g., different starting points
for all-offcurve contours).

This change compares contours between fontc and fontmake first, and
only normalizes the order when the contours are identical (as sets).
When contours differ, they are left in their original order to make
the actual differences more apparent.

Fixes part of #1653
@anthrotype anthrotype force-pushed the no-reorder-contours-unless-matching branch from f55714e to 33af8fc Compare September 25, 2025 11:36
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

One concern inline... boy do I wish we had tests for this code 😅

@anthrotype anthrotype added this pull request to the merge queue Sep 25, 2025
Merged via the queue into main with commit 55c2620 Sep 25, 2025
12 checks passed
@anthrotype anthrotype deleted the no-reorder-contours-unless-matching branch September 25, 2025 15:49
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