Skip to content

Stop an Arabic form's context walk at the edge of the run (#204) - #219

Merged
jakejackson1 merged 5 commits into
gravitypdffrom
recover/204-arabic-context-walk
Sep 17, 2026
Merged

jakejackson1 merged 5 commits into
gravitypdffrom
recover/204-arabic-context-walk

Conversation

@jakejackson1

Copy link
Copy Markdown
Member

Closes #204.

This carries PR #210's change to gravitypdf. #210 was merged into its stacked base branch after that branch had already been squash-merged into gravitypdf, so its commits never reached gravitypdf and #204 stayed open. The commits are #210's own, replayed with git rebase --onto and patch-for-patch identical per git range-diff. See #210 for the full description, measurements and review.

This branch also contains #193's recovery PR, and is merged after them.

🤖 Generated with Claude Code

jakejackson1 and others added 5 commits September 17, 2026 12:48
…x turning up in the class (#193)

GDEF's classes are kept as " 00300| 00301", and every reader asked whether a glyph was in one
with strpos(). GlyphString::of() writes a plane 16 character six digits wide, and a six-digit
glyph holds two five-digit ones: U+100300 is 10030 followed by a 0, and a 1 followed by 00300.
Where a class named U+100300, U+0300 and U+10030 both read as members.

The stored formats do not change. GlyphString gains set(), a list of glyphs keyed by glyph, and
inList(), which finds a glyph only where no hex digit touches it on either side:

- LookupFlag::skips() looks each class up in a set, built once per class.
- Otl builds a set of the marks once per font, and its twenty GlyphClassMarks tests become
  isMark(). The LookupFlag it built per run is kept per font with it. LineBreaking is handed
  the set.
- The mark to base, ligature and mark attachment subtables found the base in a "|"-joined
  Coverage string and took its index as the strpos() offset over six; they look both up in a
  map of hex to Coverage Index.
- `finals`, Shaper\Arabic's backtrack, lookahead and ignore strings, the parser's test of a
  nested lookup's first glyph against a rule position, and OtlDump::positionHolds() go through
  inList().

No font TTFontFile parses can reach this today. It reads no cmap entry at or past U+30000 and
maps a glyph only such an entry reaches into the Private Use Area, so every glyph in a class is
five digits wide. Regenerating every golden master from cold moves none of them.

NotoSans-PlaneSixteenMark-Synthetic shows it all the same. It is Noto Sans 2.007 (OFL 1.1) cut
down in fontTools 4.59.2 to space, A, grave, asciitilde, gravecomb and acutecomb, with u10030
(drawn as A, mapped to U+10030) and u100300 (drawn as gravecomb, mapped to U+100300) added, its
layout tables replaced and name IDs 1, 4 and 6 renamed. GDEF classes gravecomb and u10030 as
bases, acutecomb and u100300 as marks; a 'ccmp' lookup setting IgnoreMarks substitutes grave for
gravecomb, and a 'mark' lookup attaches acutecomb to u10030. Handed the classes as the font
states them, the shaper drew A U+0300 unsubstituted and left the mark in U+10030 U+0301
unattached. It now draws `A grave` and attaches the mark at x -339 from the base's end, as
hb-shape 14.3.1 does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mark attachment Coverage straight from the table (#193)

Otl built the marks into a set of its own beside the LookupFlag that built the same set on the
first IgnoreMarks lookup; LookupFlag::marks() hands out that one, and Otl keeps just the
LookupFlag per font. marksOutsideFilteringSet() reads its mark glyph set through
GlyphString::set(). coverageIndexByHex() was the last reader of _getCoverage(), whose hex list
was cached only to be turned into the index, so the index is built from the table and the list
goes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shaper\Arabic::glyphs() matches a joining form's backtrack (prel) and
lookahead (postl) by walking over the glyphs the lookup ignores, and
nothing stopped that walk at the edge of the run. Where a word starts or
ends in ignored glyphs, $chars[...] past the edge is null. On PHP 7 that
was a notice per glyph read before the loop gave up; from PHP 8
GlyphString::inList() searches the ignore pattern for "", finds it, and
the loop never returns. A word ending in BEH FATHA, where beh's final or
isolated form has a lookahead, was enough.

Both walks now test isset() before reading the ignore pattern. The check
after the walk already refused a position the run does not hold, which is
what HarfBuzz's match_backtrack()/match_lookahead() do when skippy_iter
runs out of glyphs, so no other result changes.

NotoSansArabic-ContextEdge-Synthetic is NotoSansArabic-Joining-Subset
(Noto Sans Arabic 2.012, OFL 1.1) rebuilt in fontTools 4.59.2: a FATHA
(uni064E, dotbelowar's outline, GDEF mark class, U+064E) is added; GSUB
is replaced by 'init' and 'fina' Chaining Context Substitutions (Type 6
Format 3, IgnoreMarks) - beh takes uni066E.init after uni08AD and
uni066E.fina before it - and a single 'medi'; 'ccmp' is dropped, since
the shaper resolves joining after it and a beh it takes apart joins
nothing (#209); name IDs 1, 4 and 6 are renamed. hb-shape 14.3.1 draws what
ArabicContextEdgeTest expects for BEH BEH FATHA, FATHA BEH BEH, and the
same with LOW ALEF past the mark.

Regenerating fontcache, otldump, shaping and subset from cold adds the
new font's four fixtures and moves no existing one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the walk's comment (#204)

The child-process test spawned PHP once per data-provider case. The
fixture now takes all the runs at once and prints each run's [hex, form]
pairs by name, so the suite pays one start-up and one time limit, and the
test no longer re-reads the child's run into pairs itself. Removing
either bound still fails it on "Maximum execution time of 5 seconds
exceeded".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jakejackson1 jakejackson1 added bug Something isn't working create-upstream-pr labels Sep 17, 2026
@jakejackson1
jakejackson1 merged commit abbf2e1 into gravitypdf Sep 17, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working create-upstream-pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An Arabic form's context walked past the end of the run over ignored glyphs never returns on PHP 8

1 participant