Fix CCITT b1 selection so vertical modes cannot move backwards - #4
Merged
soadzoor merged 1 commit intoSep 17, 2026
Merged
Conversation
decodeTwoDimensionalRow asked findReferenceTransition for the first changing element at or after a0, but T.4 4.2.1.3.2 defines b1 as the first changing element on the reference line strictly to the right of a0 and of opposite colour. When the reference row changed colour exactly at a0, b1 came back as a0 itself and any V_L(n) placed a1 before a0, which decodeTwoDimensionalRow reports as "CCITT vertical mode places a changing element outside the row." The search cannot simply become strict: T.4 4.2.1.3.1 starts every coding line on an imaginary changing element just before column 0, so a0 is -1 until the first mode is decoded. Without that, a reference element on column 0 would be skipped instead. Measured on the one-page PDF attached to issue soadzoor#2, decoding failed on row 20 of 264 with a0 = 168, b1 = 168 and V_L(1). With b1 taken strictly to the right of a0, the same stream decodes in full: 264 rows, all 4819 encoded bytes consumed, terminated by EOFB, no damaged rows. The new case in testGroup4ModesAndEofb sits next to the existing "all six nonzero vertical modes" block, whose comment already notes that it keeps every b1 strictly ahead of a0. It reproduces the same failure on eight columns. Refs soadzoor#2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 8, 2026
Owner
|
Great job, thank you for the explanations/details as well! |
soadzoor
added a commit
that referenced
this pull request
Sep 17, 2026
- 0.1.30 - Fix: use native ESM paths in Vite config - Prefer usable PDF output with bounded raster fallback and approximation warnings - Strengthen CFF BaseFontBlend regression coverage - Merge pull request #10 from sebgoubier/fix/cff-basefontblend-is-not-mm - Clarify stitching bounds tolerance and expand regression tests - Merge pull request #9 from sebgoubier/fix/stitching-bounds-plateau - Feat(pdf): Add support for ICC color profiles (qcms, lcms, and alternate) with fallback - Fix(pdf): recover Flate EOL padding without losing decoded output - Feat(pdf): synthesize cloudy borders for Square annotations - Feat(pdf): support underline borders on Square annotations - Feat(pdf): synthesize Square annotations with validated geometry - Test: reject invalid CIDSystemInfo strings - Merge pull request #5 from sebgoubier/fix/cid-system-info-nul-padding - Fix(tests): handle file URLs correctly on Windows - Merge pull request #4 from sebgoubier/fix/ccitt-b1-strictly-right-of-a0 - Stop rejecting BaseFontBlend as a Multiple Master CFF font - Treat repeated stitching Bounds as the empty subdomain they describe - Trim fixed-width NUL padding in /CIDSystemInfo strings - Fix CCITT b1 selection so vertical modes cannot move backwards Source-Revision: 5d761c9
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
2 participants
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.
What this fixes
The first of the four failures reported in #2:
CCITT vertical mode places a changing element outside the row.decodeTwoDimensionalRowasksfindReferenceTransitionfor the first changing element at or after a0. T.4 4.2.1.3.2 defines b1 as the first changing element on the reference line strictly to the right of a0 and of opposite colour. When the reference row changes colour exactly at a0, b1 comes back as a0 itself, and any V_L(n) then places a1 before a0 — which the decoder refuses, correctly.So the throw is the symptom, not the defect: the stream was conformant, b1 was not.
The search cannot simply become strict, though. T.4 4.2.1.3.1 starts every coding line on an imaginary changing element situated just before column 0, so a0 is -1 until the row's first mode is decoded. Without modelling that, a reference element on column 0 would now be skipped instead. The patch does both.
Evidence
Measured on the one-page PDF attached to #2 (957 columns, 264 rows,
K -1,ImageMask):The failure before the fix:
a0 = 168,b1 = 168,V_L(1)→a1 = 167. The reference row's changing elements are[168, 169, 170, ...]; with b1 taken strictly to the right of a0, b1 is 170 and a1 is 169, which advances.Consuming exactly the whole stream and landing on its EOFB is what says the stream was conformant rather than merely tolerable — and the decoded image is clean, with no row-level artefacts.
node PDFtoHEP.json that PDF now completes in 2 s.Test
New case in
testGroup4ModesAndEofb, placed right after the "all six nonzero vertical modes" block — whose comment already states that it keeps every b1 strictly ahead of a0. The situation was therefore avoided by the fixtures rather than covered by them.Eight columns, same signature as the real file (
a0 = 3,b1 = 3,V_L(1)). It fails onmainwith the exact production error and passes with the patch.Checks
npm run test:fileontest-native-ccitt.mjs,test-native-ccitt-images.mjs,test-pdf-session-ccitt-images.mjs— all passnpm test—tsc --noEmitclean, 36 / 37 fast files passnpm run test:integration— 41 / 41npm run test:unit— 66 / 68The two failing files under
fast/unit(test-text-lod-core.mjs,test-room-segment-extractor.mjs) also fail onmainwithout this change. They are Windows-only: a POSIX-style path reaches Node as/C:/dev/...and resolves toC:\C:\dev\.... Unrelated to this patch — happy to open a separate issue if that is useful to you.Scope
Only the CCITT item of #2. The
/Squareannotation appearance, ICCBased and CID font failures are untouched; the first of those is a missing feature rather than a defect, and the policy question it raises is yours to settle.Refs #2