Merge pull request #11 from atlasia-ma/perf/shaping-cache

perf(text): stop paying for arabic-reshaper's broken ligature cache

Author: hBouanane

CHANGELOG.md

@@ -4,6 +4,32 @@ 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.1] - 2026-08-13
+
+### Fixed
+
+- **Shaping was 68% of generation time.** arabic-reshaper 3.0.0 guards its ligature-regex
+  cache with `hasattr(self, '__ligatures_re')`, but writes the cache to
+  `self.__ligatures_re` — which Python mangles to `_ArabicReshaper__ligatures_re` inside
+  the class body, while the string literal passed to `hasattr` is not mangled. The guard
+  therefore checks a name that is never set, and every call rebuilt the regex, re-reading
+  ~290 configparser entries. Laying out one page called it ~1,800 times.
+
+  Two mitigations, both contained to `ocrsmith.text.shaping`: results are cached (shaping
+  is a pure function of the string), and the reshaper's cache is warmed once so the
+  library's own guard fires from the second call. 2,000 distinct strings: 26s -> 0.36s.
+
+### Changed
+
+- Wrapping now measures a line the way the renderer draws it — summing word advances plus
+  space advances — instead of measuring the whole candidate line as one shaped run. This
+  is a correctness improvement as well as a speed one: the two could previously disagree
+  about where a line ends. It also keys the measurement cache on *words*, which repeat,
+  rather than on line prefixes, which never do.
+
+Test suite runtime: 95s -> 20s. Generation throughput on a mixed Arabic corpus improved
+~1.6x end to end; the remaining cost is genuine rasterisation and degradation work.
+
 ## [1.0.0] - 2026-08-12
 
 The first release of OCRSmith as a document forge rather than a line-image generator.
@@ -131,4 +157,5 @@ 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.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.0"
+version = "1.0.1"
 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.0"
+__version__ = "1.0.1"

src/ocrsmith/core/documents/flow.py

@@ -284,7 +284,7 @@ def _place_text(
         lines = [
             wrapped
             for source in text.split("\n")
-            for wrapped in wrap_paragraph(source, metrics.advance, column.width)
+            for wrapped in wrap_paragraph(source, metrics.line_advance, column.width)
         ]
         if not lines:
             return (None, role.space_after, None)

src/ocrsmith/core/documents/table_renderer.py

@@ -121,7 +121,7 @@ def _column_widths(
         for cell in table.cells:
             if cell.col_span > 1:
                 continue  # spanning cells do not constrain a single column
-            natural[cell.col] = max(natural[cell.col], metrics.advance(cell.text) + padding)
+            natural[cell.col] = max(natural[cell.col], metrics.line_advance(cell.text) + padding)
 
         total = sum(natural)
         if total <= max_width or total <= 0:
@@ -138,7 +138,7 @@ def _wrap_cells(
         for cell in table.cells:
             span_width = sum(col_widths[cell.col : cell.col + cell.col_span])
             available = max(1.0, span_width - inner_padding)
-            wrapped[(cell.row, cell.col)] = wrap_paragraph(cell.text, metrics.advance, available)
+            wrapped[(cell.row, cell.col)] = wrap_paragraph(cell.text, metrics.line_advance, available)
         return wrapped
 
     def _row_heights(

src/ocrsmith/core/rendering/metrics.py

@@ -81,6 +81,20 @@ def extent(self, text: str) -> TextExtent:
     def advance(self, text: str) -> float:
         return self._extent(text).advance
 
+    def line_advance(self, text: str) -> float:
+        """Width of `text` as the renderer will actually lay it out.
+
+        The renderer positions words individually and adds a space advance between them,
+        so measuring the whole string as one shaped run could disagree with what gets
+        drawn. Summing per word also means the measurement cache is keyed on *words*,
+        which repeat, rather than on line prefixes, which never do — wrapping a paragraph
+        asks for "a", "a b", "a b c" … and none of those would ever hit.
+        """
+        words = text.split()
+        if not words:
+            return 0.0
+        return sum(self.advance(word) for word in words) + self.space_advance * (len(words) - 1)
+
     @property
     def space_advance(self) -> float:
         return self._extent(" ").advance

src/ocrsmith/core/rendering/text_renderer.py

@@ -126,7 +126,7 @@ def render(
         metrics = metrics_for(font, self.shaper)
         base_direction = direction or detect_direction(text)
 
-        line_texts = list(wrap_text(text.split("\n"), metrics.advance, max_width))
+        line_texts = list(wrap_text(text.split("\n"), metrics.line_advance, max_width))
         line_height = metrics.line_height(style.line_spacing)
         line_texts, dropped = fit_lines(line_texts, line_height, max_height)
 

src/ocrsmith/text/shaping.py

@@ -65,6 +65,20 @@ def shape(self, text: str) -> ShapedText:
         return ShapedText(logical=text, visual=text, direction=detect_direction(text))
 
 
+#: Shaping is a pure function of the string, so results are cached. This is not a
+#: micro-optimisation: arabic-reshaper 3.0.0 guards its ligature-regex cache with
+#: `hasattr(self, '__ligatures_re')`, and because that string literal is not name-mangled
+#: the guard never fires — so every single call rebuilds the regex, re-reading ~290
+#: configparser entries. Laying out one page called it ~1_800 times, which measured as 68%
+#: of total generation time. Caching here sidesteps it without patching their library.
+_SHAPE_CACHE_SIZE = 200_000
+
+
+@lru_cache(maxsize=_SHAPE_CACHE_SIZE)
+def _visual_form(text: str) -> str:
+    return _bidi_display(_reshape(text))
+
+
 class ReshaperBidiShaper:
     """Substitute Arabic presentation forms and reorder runs to visual order."""
 
@@ -74,8 +88,7 @@ def shape(self, text: str) -> ShapedText:
         direction = detect_direction(text)
         if not text:
             return ShapedText(logical=text, visual=text, direction=direction)
-        visual = _bidi_display(_reshape(text))
-        return ShapedText(logical=text, visual=visual, direction=direction)
+        return ShapedText(logical=text, visual=_visual_form(text), direction=direction)
 
 
 _BACKENDS: dict[str, type] = {
@@ -118,9 +131,26 @@ def resolve_shaper(backend: str = "auto") -> TextShaper:
 
 @lru_cache(maxsize=1)
 def _reshaper():
-    import arabic_reshaper
+    """A reshaper whose ligature-regex cache actually works.
+
+    arabic-reshaper 3.0.0 guards that cache with `hasattr(self, '__ligatures_re')`, but
+    writes it to `self.__ligatures_re` — which, inside the class body, Python mangles to
+    `_ArabicReshaper__ligatures_re`. The string passed to `hasattr` is *not* mangled, so
+    the guard checks a name that is never set and the regex is rebuilt on every call,
+    re-reading around 290 configparser entries each time.
+
+    Warming the property once and then setting the unmangled name makes the guard fire
+    from the second call onwards. Reaching into a third-party private attribute is not
+    something to do lightly; it is contained to this adapter, and the alternative is
+    paying that cost on every word of every page.
+    """
+    from arabic_reshaper import ArabicReshaper
 
-    return arabic_reshaper.reshape
+    reshaper = ArabicReshaper()
+    reshaper._ligatures_re  # noqa: B018 - builds and caches under the mangled name
+    if not hasattr(reshaper, "__ligatures_re"):
+        object.__setattr__(reshaper, "__ligatures_re", True)
+    return reshaper.reshape
 
 
 @lru_cache(maxsize=1)

tests/test_rendering_text_block.py

@@ -236,3 +236,40 @@ def test_translated_annotation_moves_with_the_paste(self, renderer, font):
         original = rendered.lines[0].words[0].bbox
         assert moved[0].words[0].bbox.x0 == original.x0 + 100
         assert moved[0].words[0].bbox.y0 == original.y0 + 50
+
+
+class TestLineMeasurement:
+    """Wrapping must measure a line the way the renderer draws it.
+
+    The renderer places words individually and adds a space advance between them. If
+    wrapping measured the line as one shaped run instead, the two could disagree about
+    where a line ends — and the disagreement would show up as text overflowing its column.
+    """
+
+    def test_line_advance_is_the_sum_of_word_advances_and_gaps(self, font):
+        from ocrsmith.core.rendering.metrics import metrics_for
+
+        metrics = metrics_for(font)
+        words = ["mad", "rasa", "kbira"]
+
+        expected = sum(metrics.advance(w) for w in words) + metrics.space_advance * 2
+
+        assert metrics.line_advance(" ".join(words)) == pytest.approx(expected)
+
+    def test_a_single_word_measures_as_itself(self, font):
+        from ocrsmith.core.rendering.metrics import metrics_for
+
+        metrics = metrics_for(font)
+
+        assert metrics.line_advance("مرحبا") == pytest.approx(metrics.advance("مرحبا"))
+
+    def test_empty_text_has_no_width(self, font):
+        from ocrsmith.core.rendering.metrics import metrics_for
+
+        assert metrics_for(font).line_advance("   ") == 0.0
+
+    def test_wrapped_lines_fit_the_column_they_were_measured_against(self, renderer, font):
+        rendered = renderer.render(ARABIC + " " + LATIN, font, max_width=240)
+
+        for line in rendered.lines:
+            assert line.bbox.width <= 240 + 8  # canvas bleed

tests/test_text_shaping.py

@@ -133,3 +133,41 @@ def test_unknown_backend_is_rejected(self):
     def test_any_shaper_keeps_the_label_intact(self):
         for backend in ("raqm", "reshaper"):
             assert resolve_shaper(backend).shape(HELLO_AR).logical == HELLO_AR
+
+
+class TestShapingCache:
+    """Shaping is cached because arabic-reshaper is pathologically slow per call.
+
+    Caching a pure function is only safe if it really is pure, so these pin the
+    behaviour the cache must not change.
+    """
+
+    def test_repeated_shaping_is_stable(self):
+        shaper = ReshaperBidiShaper()
+
+        first = shaper.shape(HELLO_AR)
+        second = shaper.shape(HELLO_AR)
+
+        assert first == second
+
+    def test_the_cache_does_not_bleed_between_inputs(self):
+        shaper = ReshaperBidiShaper()
+
+        one = shaper.shape("مرحبا").visual
+        two = shaper.shape("بالعالم").visual
+
+        assert one != two
+        assert shaper.shape("مرحبا").visual == one
+
+    def test_lam_alef_still_forms_a_ligature(self):
+        # The reshaper's ligature table is what the cache-warming workaround touches, so
+        # a lost ligature is the failure mode to watch for.
+        shaped = ReshaperBidiShaper().shape("لا")
+
+        assert len(shaped.visual) == 1
+        assert shaped.logical == "لا"
+
+    def test_a_shaped_word_keeps_its_character_count_otherwise(self):
+        shaped = ReshaperBidiShaper().shape("بالعالم")
+
+        assert len(shaped.visual) == len("بالعالم")