fix: decode 16.16 Fixed with /65536 instead of /65535#857
Open
spokodev wants to merge 1 commit into
Open
Conversation
getFixed divided the 16-bit fractional part by 65535, but a 16.16 Fixed value's fraction is a binary fraction over 2^16 = 65536 (the sibling parseF2Dot14 correctly uses 2^14). The encode path scales by 65536, so a parse -> write -> parse round-trip drifted and parsed values disagreed with fonttools. This affects post.italicAngle, head.fontRevision, fvar axis min/default/ max and instance coordinates, and STAT values: e.g. Arial Narrow Italic italicAngle parses as -9.699992 instead of the correct -9.699997. The error is up to 1/65536 per value. Update the STAT parse fixtures, which asserted the /65535 values.
Member
|
I'll take a look at this PR in two days, I have family in town.
…On Tue, Jun 23, 2026, 12:28 PM spokodev ***@***.***> wrote:
getFixed (src/parse.mjs) decodes a 16.16 Fixed value's fractional part
with / 65535:
function getFixed(dataView, offset) {
const decimal = dataView.getInt16(offset, false);
const fraction = dataView.getUint16(offset + 2, false);
return decimal + fraction / 65535; // should be / 65536}
Per the OpenType *Data Types* spec, Fixed is a "32-bit signed fixed-point
number (16.16)" — the 16-bit fraction is a binary fraction over *2^16 =
65536*, not 65535. The sibling parseF2Dot14 in the same file correctly
divides by 16384 = 2^14, which isolates this as an off-by-one in getFixed.
Effect
getFixed backs post.italicAngle, head.fontRevision, the fvar axis
min/default/max + instance coordinates, and the STAT table values. They
are all off by up to 1/65536 (~1.5e-5), and they do *not round-trip*: the
encode path scales by 65536 (src/types.mjs, src/tables/post.mjs,
src/tables/fvar.mjs), so parse → write → parse drifts.
Concrete real fonts (macOS system fonts):
Font post.italicAngle (current) correct (/65536)
Arial Narrow Italic -9.699992370489051 -9.699996948242188
Times New Roman Italic -16.332982375829708 -16.332992553710938
The magnitude is small — it does not cross a glyph-metric/rendering
threshold — so this is a spec-conformance / round-trip-consistency fix, not
a rendering change.
Fix
getFixed: / 65535 → / 65536. The two STAT parse fixtures (
test/tables/stat.spec.mjs) asserted the old /65535 values (e.g. raw Fixed 01
23 45 67 = 291 + 17767/65535 = 291.2711070…); updated to the spec value 291
+ 17767/65536 = 291.2711029…. They fail on the current code and pass with
the fix; the full suite (342) and lint are green.
------------------------------
You can view, comment on, or merge this pull request online at:
#857
Commit Summary
- e7427d0
<e7427d0>
fix: decode 16.16 Fixed with /65536 instead of /65535
File Changes
(2 files <https://github.com/opentypejs/opentype.js/pull/857/files>)
- *M* src/parse.mjs
<https://github.com/opentypejs/opentype.js/pull/857/files#diff-8eb0d25152194e2ea8d11cee221a2ec8621f50617b23f9b8b2702f9ce08dc3fe>
(2)
- *M* test/tables/stat.spec.mjs
<https://github.com/opentypejs/opentype.js/pull/857/files#diff-c08669a4dd0dbfb744d6bf3326a6c8a8bf9747a25e8f042e3907c65a957bbbc7>
(4)
Patch Links:
- https://github.com/opentypejs/opentype.js/pull/857.patch
- https://github.com/opentypejs/opentype.js/pull/857.diff
—
Reply to this email directly, view it on GitHub
<#857?email_source=notifications&email_token=AACB45S7QKP6HUVTG7P6LML5BLK67A5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTSMRSGU3TGMJTHCTHEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVRTG633UMVZF6Y3MNFRWW>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACB45SZ33PICEZUPW576LL5BLK67AVCNFSNUABEKJSXA33TNF2G64TZHMYTGMJVGAYDKNR3JFZXG5LFHM2DOMRYHEZDKMZUGWQXMAQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
getFixed(src/parse.mjs) decodes a 16.16Fixedvalue's fractional part with/ 65535:Per the OpenType Data Types spec,
Fixedis a "32-bit signed fixed-point number (16.16)" — the 16-bit fraction is a binary fraction over 2^16 = 65536, not 65535. The siblingparseF2Dot14in the same file correctly divides by16384 = 2^14, which isolates this as an off-by-one ingetFixed.Effect
getFixedbackspost.italicAngle,head.fontRevision, thefvaraxis min/default/max + instance coordinates, and theSTATtable values. They are all off by up to1/65536(~1.5e-5), and they do not round-trip: the encode path scales by65536(src/types.mjs,src/tables/post.mjs,src/tables/fvar.mjs), soparse → write → parsedrifts.Concrete real fonts (macOS system fonts):
post.italicAngle(current)/65536)-9.699992370489051-9.699996948242188-16.332982375829708-16.332992553710938The magnitude is small — it does not cross a glyph-metric/rendering threshold — so this is a spec-conformance / round-trip-consistency fix, not a rendering change.
Fix
getFixed:/ 65535→/ 65536. The twoSTATparse fixtures (test/tables/stat.spec.mjs) asserted the old/65535values (e.g. raw Fixed01 23 45 67=291 + 17767/65535 = 291.2711070…); updated to the spec value291 + 17767/65536 = 291.2711029…. They fail on the current code and pass with the fix; the full suite (342) and lint are green.