Skip to content

Fix signed fractional CFF coordinates - #557

Merged
JimBobSquarePants merged 2 commits into
SixLabors:mainfrom
Artromskiy:fix-cff-igrunok-outline
Sep 4, 2026
Merged

Fix signed fractional CFF coordinates#557
JimBobSquarePants merged 2 commits into
SixLabors:mainfrom
Artromskiy:fix-cff-igrunok-outline

Conversation

@Artromskiy

@Artromskiy Artromskiy commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practices demonstrated in the repository. These follow strict StyleCop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fix CFF Type 2 signed 16.16 parsing: the integer word is signed, while the fractional word is unsigned. This corrects the CFF outline coordinates produced for fonts such as Igrunok.

The change includes a raw glyph 54 regression test and a fixed-point reader test. The Igrunok font itself is not redistributed.

Validation performed:

  • SixLabors.Fonts Release build
  • Targeted CFF regression tests
  • Render probe for glyph 54 across historical CFF revisions
  • Comparison against CoreText and Skia outline coordinates

The first renderable CFF implementation already showed the mismatch; the fix corrects the fractional-word interpretation without changing the public API.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes parsing of CFF Type 2 signed 16.16 fixed-point coordinates by treating the integer word as signed and the fractional word as unsigned, correcting outline coordinates for affected fonts (eg. Igrunok) without changing the public API.

Changes:

  • Update SimpleBinaryReader.ReadFloatFixed1616() to interpret the fractional 16-bit word as unsigned.
  • Add regression tests covering fixed-point decoding and a raw charstring prefix reproducer for glyph 54 coordinate stability.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/SixLabors.Fonts.Tests/Tables/Cff/CffEvaluationEngineTests.cs Adds a fixed-point reader test and a raw Type 2 prefix regression test for CFF coordinate evaluation.
src/SixLabors.Fonts/Tables/Cff/SimpleBinaryReader.cs Corrects 16.16 fixed-point parsing to use an unsigned fractional word.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/SixLabors.Fonts.Tests/Tables/Cff/CffEvaluationEngineTests.cs Outdated
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81%. Comparing base (812fd6f) to head (daed664).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #557   +/-   ##
=====================================
  Coverage     81%     81%           
=====================================
  Files        406     406           
  Lines      35672   35672           
  Branches    5561    5561           
=====================================
+ Hits       29143   29147    +4     
+ Misses      5359    5356    -3     
+ Partials    1170    1169    -1     
Flag Coverage Δ
unittests 81% <100%> (+<1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Artromskiy

Copy link
Copy Markdown
Contributor Author

Are there any updates on this pull request?

@JimBobSquarePants JimBobSquarePants 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.

@Artromskiy Thank you for the fix. this is very much appreciated.

I'll merge it now ready for the next release. A nightly package will be published following merge.

@JimBobSquarePants
JimBobSquarePants merged commit c68b27d into SixLabors:main Sep 4, 2026
10 checks passed
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.

3 participants