Skip to content

fix: contract right-to-left text emitted in logical order - #356

Open
wittjeff wants to merge 1 commit into
docling-project:mainfrom
wittjeff:fix/cursor-advance-char-spacing
Open

wittjeff wants to merge 1 commit into
docling-project:mainfrom
wittjeff:fix/cursor-advance-char-spacing

Conversation

@wittjeff

Copy link
Copy Markdown
Contributor

Fixes the regression reported in docling-project/docling#4018 (reopened): with 7.21.0 the Hebrew sample heb_test_doc-prog.pdf comes out as א ל ו ה י ם … — every character a separate word and line cell. 7.20.0 was fine.

Cause

Bisected between 7.20.0 and 7.21.0: #328 alone reproduces the 7.20.0 output exactly; the change comes from #351's geometry-driven contractor. Character geometry is identical in all builds.

The sample is Quartz output that writes Hebrew in logical order under a left-to-right text matrix: -1.0762 Tc (≈ −17 pt) plus a per-glyph TJ offset puts each glyph immediately left of the previous one. The ink boxes abut exactly, but the cursor runs backwards.

  • forward_gap() measures previous advance end → next origin along the writing axis, which here is ≈ −2 glyph widths. continues_line() requires gap >= -layout_scale, so every pair starts a new line run.
  • append_cell() always prepends right-to-left text, which is correct for visual-order streams but reverses logical-order ones once they do merge (םיהולא).

Change

  • forward_gap(): if both cells are right-to-left and the next origin lies behind the previous one along the writing axis, return the gap from the next cell's advance end back to the previous origin. Left-to-right text and math cursor rewinds are not affected (they never have both cells RTL).
  • append_cell(): append instead of prepend only when the next cell extends clearly before the aggregate along the bbox axis (both extents, 25 % tolerance). Overlapping cells such as combining marks keep the prepend default — an earlier centre-based test swapped fathatan and alef in 4160141760688927471-1.pdf.
  • New tests/test_unit_rtl_logical_order.py: synthetic logical-order TJ stream at two Tc values (fails on main) plus a visual-order control (passes on both).

Rejected alternative: adding Tc/Tw to the text advance endpoint in text.h. It changes tracked left-to-right text everywhere and still splits words on the TJ offsets.

Results

  • heb_test_doc-prog.pdf: word cells identical to 7.20.0 (91 words); line cells without the stray whitespace-only lines 7.20.0 produced.

  • Unit tests pass locally.

  • test_regression_threaded_parse against the pinned groundtruth: main fails 3 pages (2508.13113v2.pdf p2/9/17, locally — possibly environment). This branch changes 25 further pages, all Arabic/Persian. Per page, word-level single-letter RTL fragments never increase and often drop sharply compared with the current groundtruth, e.g.:

    page 7.20.0 7.21.0 (groundtruth) this PR
    b766d7d18edf92b1_0002 153 626 233
    069af5ebf89bc7b3_0030 50 131 60
    ead2c552b489aac1_0004 17 32 22
    ec62f8dc0265e965_0018 24 26 18

    Several pages keep identical words but produce fewer line cells, because backward-running RTL runs are no longer cut at every glyph pair (e.g. 994ed640a9c152e5_0001: 1489 → 1210 lines, joining runs of Persian digits). Please review those line merges. The groundtruth for these pages would need regenerating if you accept the change; I have not touched tests/constants.py.

Not addressed

right_to_left_04.pdf: #351 also splits the Arabic heading "المقدمة" (with tatweel) into ا|لم|ـ|ق|ـ|دمـ|ــ|ة; 7.20.0 kept it as one word. This PR only merges ـ|ق|ـ there; the tatweel handling is a separate issue.

🤖 Generated with Claude Code

Some generators (macOS Quartz output, e.g. docling#4018) write Hebrew and
Arabic in logical order under a left-to-right text matrix: each glyph is
placed immediately left of the previous one by a large negative Tc plus
per-glyph TJ offsets. The painted glyphs abut, but the PDF cursor runs
backwards.

The geometry-driven contractor from docling-project#351 measures the gap from the
previous cursor advance end to the next origin. For these streams that is
about minus two glyph widths, so continues_line() rejects every pair and
each character becomes its own word and line (regression from 7.20.0).
append_cell() also always prepends right-to-left text, which assumes
visual-order streams and would reverse these words once merged.

- forward_gap(): when both cells are right-to-left and the next origin
  lies behind the previous one, measure from the next cell's advance end
  back to the previous origin.
- append_cell(): append right-to-left text only when the next cell lies
  clearly before the aggregate along the bbox axis; overlapping cells
  (combining marks) keep the prepend default.

The reporter's page now yields the same word cells as 7.20.0. Including
Tc/Tw in the text advance was considered and rejected: it changes
tracked left-to-right text globally and still splits words on the TJ
offsets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @wittjeff, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@wittjeff
wittjeff marked this pull request as ready for review September 24, 2026 05:48

This branch has not been deployed

No deployments
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.

1 participant