Merge pull request #12 from atlasia-ma/fix/coverage-presentation-forms

fix(fonts): check coverage against the glyphs that are actually drawn

Author: hBouanane

CHANGELOG.md

@@ -4,6 +4,30 @@ All notable changes to this project are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.2] - 2026-08-13
+
+### Fixed
+
+- **Font coverage was checked against the wrong string, so Arabic could render as tofu.**
+  Coverage was judged on the *logical* text, but without Raqm the renderer draws Arabic
+  *presentation forms* (U+FE70-FEFF). A modern OpenType face such as Fustat or Mada covers
+  the base Arabic block while carrying no presentation-form glyphs at all — it joins
+  letters via GSUB instead — so the probe reported 100% coverage and every glyph then
+  rendered as an empty box, with the label still claiming the text. `FontPool` now shapes
+  before probing, using the same shaper the renderer will use. Raqm builds are unaffected
+  and correctly keep judging the logical form.
+- **The coverage probe missed table cells and list items.** A table block's own text is
+  empty; its content lives in the cells. An invoice whose prose was four words therefore
+  chose a font on the strength of those four words and drew its entire table as tofu.
+  Added `DocumentContent.all_text`, which includes cells and list items, and the pipeline
+  now probes against it.
+- **The font fallback silently discarded the guarantee.** When no face covered a document,
+  the pipeline fell back to the *entire* pool and could hand the document a face that
+  cannot draw its script at all. It now falls back to the single best-covering face.
+
+Found by rendering sample documents and looking at them — every automated check passed
+while the images were visibly broken.
+
 ## [1.0.1] - 2026-08-13
 
 ### Fixed
@@ -157,5 +181,6 @@ page before it reaches the dataset.
   were unset, which made every sample fail for configs that omit them.
 - Whitespace-only input no longer produces a zero-sized canvas.
 
+[1.0.2]: https://github.com/atlasia-ma/OCRSmith/releases/tag/v1.0.2
 [1.0.1]: https://github.com/atlasia-ma/OCRSmith/releases/tag/v1.0.1
 [1.0.0]: https://github.com/atlasia-ma/OCRSmith/releases/tag/v1.0.0

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ocrsmith"
-version = "1.0.1"
+version = "1.0.2"
 description = "Synthetic document and OCR dataset forge for Arabic, Darija and Latin scripts."
 readme = "README.md"
 authors = [

src/ocrsmith/__init__.py

@@ -29,4 +29,4 @@
     "run_generation",
 ]
 
-__version__ = "1.0.1"
+__version__ = "1.0.2"

src/ocrsmith/core/documents/content.py

@@ -53,6 +53,24 @@ class DocumentContent:
     def text(self) -> str:
         return "\n".join(block.text for block in self.blocks if block.text)
 
+    @property
+    def all_text(self) -> str:
+        """Every character the document will draw, including table cells and list items.
+
+        `text` reads block text only, and a table block's text is empty — the content
+        lives in its cells. Font coverage must be judged against *this*, or an invoice
+        whose prose is four words picks a face on the strength of those four words and
+        then renders its entire table as tofu.
+        """
+        parts: list[str] = []
+        for block in self.blocks:
+            if block.text:
+                parts.append(block.text)
+            parts.extend(block.items)
+            if block.table is not None:
+                parts.extend(cell.text for cell in block.table.cells if cell.text)
+        return "\n".join(parts)
+
     def __len__(self) -> int:
         return len(self.blocks)
 

src/ocrsmith/core/fonts.py

@@ -19,6 +19,7 @@
 from PIL.ImageFont import FreeTypeFont
 
 from ..text.coverage import fonts_supporting, supports_text
+from ..text.shaping import TextShaper, resolve_shaper
 
 __all__ = ["FontPool", "clear_font_cache", "discover_fonts", "load_font"]
 
@@ -86,11 +87,16 @@ def __init__(
         include: Sequence[str] = (),
         exclude: Sequence[str] = (),
         require_full_coverage: bool = True,
+        shaper: TextShaper | None = None,
     ):
         self.faces = discover_fonts(paths, include=include, exclude=exclude)
         if not self.faces:
             raise ValueError(f"No font files found under {list(paths)!r}")
         self.require_full_coverage = require_full_coverage
+        # Coverage must be judged against the characters that will actually be drawn,
+        # which is the shaper's business — so the pool needs the same shaper the renderer
+        # will use.
+        self.shaper = shaper or resolve_shaper()
         self._coverage_cache: dict[tuple[str, str], bool] = {}
 
     def __len__(self) -> int:
@@ -124,11 +130,17 @@ def covers(self, path: str | Path, text: str) -> bool:
             self._coverage_cache[key] = cached
         return cached
 
-    @staticmethod
-    def _probe(text: str) -> str:
-        """The distinct characters of `text`, which is all coverage depends on.
+    def _probe(self, text: str) -> str:
+        """The distinct characters that will actually be *drawn* for `text`.
 
-        Collapsing to a character set turns a per-document question into a per-alphabet
-        one, so the coverage cache actually hits.
+        Shaping happens first, and this is the whole point. Without Raqm the renderer
+        draws Arabic presentation forms (U+FE70-FEFF), not the base letters — and a modern
+        OpenType face such as Fustat or Mada covers the base block while carrying no
+        presentation-form glyphs at all, because it joins letters via GSUB instead. Probing
+        the logical string reports 100% coverage for such a font and every glyph then
+        renders as tofu, with the label still claiming the text.
+
+        Collapsing to a character set afterwards turns a per-document question into a
+        per-alphabet one, so the coverage cache actually hits.
         """
-        return "".join(sorted(set(text)))
+        return "".join(sorted(set(self.shaper.shape(text).visual)))

src/ocrsmith/pipeline/factory.py

@@ -159,7 +159,16 @@ def _page_spec(self, page: PageConfig, direction: Direction | None, rng: random.
         )
 
     def _typography(self, content, rng: random.Random):
-        faces = self.fonts.supporting(content.text[:2000]) or self.fonts.faces
+        # Probe against everything the page will draw - table cells and list items
+        # included - and against the *shaped* form, which is what actually reaches the
+        # rasteriser.
+        probe = content.all_text[:4000]
+        faces = self.fonts.supporting(probe)
+        if not faces:
+            # Falling back to the whole pool would hand the document a face that cannot
+            # draw its script at all, which is the exact failure coverage exists to
+            # prevent. Fall back to the best face instead, and only that one.
+            faces = (self.fonts.choose(probe, rng),)
         sampler = TypographySampler(faces, body_size_range=tuple(self.config.fonts.size_range))
         return sampler.sample(rng)
 

tests/test_font_pool.py

@@ -0,0 +1,92 @@
+"""Contract for coverage-aware font selection.
+
+The bug these pin: coverage was checked against the *logical* string while the renderer
+drew *presentation forms*. Without Raqm, arabic-reshaper turns `أمينة` into U+FE94 U+FEE8
+… and a modern OpenType face like Fustat or Mada covers the base Arabic block while
+carrying no presentation-form glyphs at all — it joins via GSUB instead. Probing the
+logical string reported 100% coverage for such a face, and every glyph then rendered as an
+empty box while the label still claimed the text.
+"""
+
+from pathlib import Path
+
+import pytest
+
+from ocrsmith.core.fonts import FontPool
+from ocrsmith.text.shaping import ReshaperBidiShaper, TransparentShaper
+
+FONT_DIR = Path(__file__).resolve().parents[1] / "assets" / "fonts"
+
+# Covers base Arabic *and* presentation forms.
+FULL_FORMS = "NotoSansArabic-Regular.ttf"
+# Covers base Arabic but has no presentation forms.
+GSUB_ONLY = "Fustat-Medium.ttf"
+
+ARABIC = "أمينة كتب الدرس"
+
+pytestmark = pytest.mark.skipif(not (FONT_DIR / GSUB_ONLY).exists(), reason="bundled fonts unavailable")
+
+
+@pytest.fixture
+def pool():
+    return FontPool(
+        [FONT_DIR / FULL_FORMS, FONT_DIR / GSUB_ONLY],
+        shaper=ReshaperBidiShaper(),
+    )
+
+
+class TestPresentationFormCoverage:
+    def test_a_font_without_presentation_forms_is_rejected(self, pool):
+        eligible = {path.name for path in pool.supporting(ARABIC)}
+
+        assert FULL_FORMS in eligible
+        assert GSUB_ONLY not in eligible, "font would render tofu under the reshaper backend"
+
+    def test_choose_never_returns_the_unusable_font(self, pool):
+        import random
+
+        for seed in range(20):
+            assert pool.choose(ARABIC, random.Random(seed)).name == FULL_FORMS
+
+    def test_latin_is_unaffected(self, pool):
+        eligible = {path.name for path in pool.supporting("hello world")}
+
+        assert eligible == {FULL_FORMS, GSUB_ONLY}
+
+    def test_a_raqm_backend_judges_the_logical_form_instead(self):
+        # With Raqm, HarfBuzz applies GSUB and never emits presentation forms, so a
+        # GSUB-only face is perfectly usable and must not be excluded.
+        pool = FontPool(
+            [FONT_DIR / FULL_FORMS, FONT_DIR / GSUB_ONLY],
+            shaper=TransparentShaper(),
+        )
+
+        eligible = {path.name for path in pool.supporting(ARABIC)}
+
+        assert eligible == {FULL_FORMS, GSUB_ONLY}
+
+    def test_disabling_the_requirement_still_returns_everything(self):
+        pool = FontPool(
+            [FONT_DIR / FULL_FORMS, FONT_DIR / GSUB_ONLY],
+            require_full_coverage=False,
+            shaper=ReshaperBidiShaper(),
+        )
+
+        assert len(pool.supporting(ARABIC)) == 2
+
+
+class TestProbeCompleteness:
+    def test_table_and_list_text_reach_the_coverage_probe(self):
+        from ocrsmith.core.documents import DocumentBuilder
+
+        content = (
+            DocumentBuilder().paragraph("short").list(["ONE", "TWO"]).table([["HEAD"], ["CELL"]]).build()
+        )
+
+        probe = content.all_text
+
+        # `text` alone misses both, which is how an invoice picked a font on the strength
+        # of four words of prose and then drew its whole table as tofu.
+        assert "CELL" in probe and "HEAD" in probe
+        assert "ONE" in probe and "TWO" in probe
+        assert "CELL" not in content.text