Enforce configurable height and pixel limits for tablet documents

Script.org

@@ -245,10 +245,15 @@ OU
     Le mode =--refaire= exige un fichier =refaire.json= et écrit dans
     =BRnot=.
 
-    Les documents sont limités à 32 000 pixels de haut par défaut, comme
-    pour =BGnot=. Pour modifier cette limite commune, définir
-    =ANNOTATION_MAX_HEIGHT_PX = 32000= dans =config.py= avec la hauteur
-    souhaitée (entier strictement positif, en pixels).
+    Les documents sont limités à 33 000 pixels de haut et 67 108 864 pixels
+    de surface (largeur maximale × hauteur totale), comme pour =BGnot=.
+    Pour modifier ces limites communes, définir
+    =ANNOTATION_MAX_HEIGHT_PX = 33000= et
+    =ANNOTATION_MAX_PIXELS = 67108864= dans =config.py= (entiers strictement
+    positifs). La limite de surface vient des observations sur DS02VA : les
+    quatre groupes dépassant 2^26 pixels sont flous dans Squid Android.
+    Cela représente 256 Mio pour un bitmap à quatre octets par pixel ;
+    ce seuil est une hypothèse empirique, pas une limite documentée par Squid.
     Le réglage s'applique aux prochaines générations ; utiliser
     =--overwrite= pour régénérer les documents déjà présents.
     Les grandes copies sont découpées entre les labels en =Part1=, =Part2=,
@@ -263,10 +268,12 @@ OU
     réponses par question.
     Enregistrées dans =BGnot=
 
-    Les groupes visent la hauteur maximale =ANNOTATION_MAX_HEIGHT_PX=
-    (32 000 pixels par défaut). Si le bloc
-    d'un seul élève dépasse cette limite, il reste entier dans un groupe
-    dédié, même lorsqu'il ne contient qu'une seule réponse.
+    Les groupes respectent les deux limites =ANNOTATION_MAX_HEIGHT_PX=
+    et =ANNOTATION_MAX_PIXELS= ci-dessus, en comptant les marges blanches
+    nécessaires pour aligner les réponses sur la largeur maximale du groupe.
+    Le bloc d'un élève reste entier s'il respecte ces limites ; sinon il est
+    séparé entre deux réponses. Une réponse individuelle dépassant l'une
+    des limites produit une erreur explicite.
 
     _Needs_ : label_groups file (made automatically by this function),
     qui dit quelles questions regrouper.

copienator/annotation_documents.py

@@ -1,12 +1,19 @@
 """Locate tablet documents, including the parts of a large checked copy."""
 from pathlib import Path
 
-from copienator.configuration import ANNOTATION_MAX_HEIGHT_PX
+from copienator.configuration import ANNOTATION_MAX_HEIGHT_PX, ANNOTATION_MAX_PIXELS
 from copienator.json_io import read_json
 
 if type(ANNOTATION_MAX_HEIGHT_PX) is not int or ANNOTATION_MAX_HEIGHT_PX <= 0:
     raise ValueError("ANNOTATION_MAX_HEIGHT_PX must be a positive integer (pixels)")
 MAX_HEIGHT_PX = ANNOTATION_MAX_HEIGHT_PX
+if type(ANNOTATION_MAX_PIXELS) is not int or ANNOTATION_MAX_PIXELS <= 0:
+    raise ValueError("ANNOTATION_MAX_PIXELS must be a positive integer (pixels)")
+MAX_PIXELS = ANNOTATION_MAX_PIXELS
+
+
+def document_fits(width: int, height: int, max_height: int, max_pixels: int) -> bool:
+    return height <= max_height and width * height <= max_pixels
 
 
 def copy_documents(directory: Path) -> list[Path]:

copienator/commands/annotating_by_label.py

@@ -23,7 +23,7 @@ from copienator import (
     workspace_from_args,
 )
 from copienator.annotation_data import load_annotation_data
-from copienator.annotation_documents import MAX_HEIGHT_PX
+from copienator.annotation_documents import MAX_HEIGHT_PX, MAX_PIXELS, document_fits
 from copienator.commands import annotating, annotating_with_checks
 from copienator.commands.import_annotations import clear_import_directory
 from copienator.filesystem import staged_directory
@@ -64,10 +64,11 @@ def save_batch(batch, prefix, group_id, output_root: Path) -> None:
     print(f"Generating group PDF: {prefix} G{group_id} ({len(batch)} elements)")
     max_width = max(item[2].width for item in batch)
     total_height = sum(item[2].height for item in batch)
-    if total_height > MAX_HEIGHT_PX and len({item[0] for item in batch}) > 1:
+    if not document_fits(max_width, total_height, MAX_HEIGHT_PX, MAX_PIXELS):
         raise CliError(
-            f"{prefix} G{group_id} would be {total_height} px high; "
-            f"only a single student's block may exceed {MAX_HEIGHT_PX} px."
+            f"{prefix} G{group_id} would be {max_width} × {total_height} px "
+            f"({max_width * total_height} pixels); document limits are "
+            f"{MAX_HEIGHT_PX} px high and {MAX_PIXELS} pixels."
         )
     concatenated = Image.new("RGB", (max_width, total_height), "white")
     draw = ImageDraw.Draw(concatenated)
@@ -281,6 +282,26 @@ def _existing_group_state(
 def split_batches(rendered):
     batches = []
     heights = []
+    widths = []
+
+    def fits(width, height):
+        return document_fits(width, height, MAX_HEIGHT_PX, MAX_PIXELS)
+
+    def add_block(items):
+        width = max(item[2].width for item in items)
+        height = sum(item[2].height for item in items)
+        # Reuse space, accounting for padding to the widest answer in a group.
+        for batch_index in range(len(batches)):
+            combined_width = max(widths[batch_index], width)
+            combined_height = heights[batch_index] + height
+            if fits(combined_width, combined_height):
+                batches[batch_index].extend(items)
+                widths[batch_index] = combined_width
+                heights[batch_index] = combined_height
+                return
+        batches.append(list(items))
+        widths.append(width)
+        heights.append(height)
 
     index = 0
     while index < len(rendered):
@@ -291,22 +312,31 @@ def split_batches(rendered):
         student_items = rendered[index:end]
         student_height = sum(item[2].height for item in student_items)
 
-        if student_height > MAX_HEIGHT_PX:
-            # Oversized blocks remain intact in a group of their own.
-            batches.append(list(student_items))
-            heights.append(student_height)
-            index = end
-            continue
-
-        # Reuse space in earlier groups, keeping each student's block intact.
-        for batch_index, height in enumerate(heights):
-            if height + student_height <= MAX_HEIGHT_PX:
-                batches[batch_index].extend(student_items)
-                heights[batch_index] += student_height
-                break
+        student_width = max(item[2].width for item in student_items)
+        if fits(student_width, student_height):
+            add_block(student_items)
         else:
-            batches.append(list(student_items))
-            heights.append(student_height)
+            # Split oversized student blocks only between complete answers.
+            block = []
+            width = height = 0
+            for item in student_items:
+                image = item[2]
+                if not fits(image.width, image.height):
+                    raise CliError(
+                        f"Copie{student_id} ({item[1]}) is {image.width} × "
+                        f"{image.height} px ({image.width * image.height} pixels); "
+                        f"a single answer cannot be split. Document limits are "
+                        f"{MAX_HEIGHT_PX} px high and {MAX_PIXELS} pixels."
+                    )
+                if block and not fits(max(width, image.width), height + image.height):
+                    add_block(block)
+                    block = []
+                    width = height = 0
+                block.append(item)
+                width = max(width, image.width)
+                height += image.height
+            if block:
+                add_block(block)
         index = end
 
     return batches

copienator/commands/annotating_with_checks.py

@@ -26,7 +26,7 @@ from copienator import (
     workspace_from_target,
 )
 from copienator.annotation_data import load_annotation_data
-from copienator.annotation_documents import MAX_HEIGHT_PX, copy_documents
+from copienator.annotation_documents import MAX_HEIGHT_PX, MAX_PIXELS, copy_documents, document_fits
 from copienator.commands import annotating
 from copienator.commands.import_annotations import clear_import_directory
 from copienator.filesystem import staged_directory
@@ -212,18 +212,23 @@ def _render_student(
     batches = []
     start = 0
     height = 0
+    width = 0
     for index, image in enumerate(label_images):
-        if image.height > MAX_HEIGHT_PX:
+        if not document_fits(image.width, image.height, MAX_HEIGHT_PX, MAX_PIXELS):
             raise CliError(
                 f"Copie{student_id} ({bnote_entries[index]['label']}) is "
-                f"{image.height} px high; a single answer cannot be split and "
-                f"the maximum document height is {MAX_HEIGHT_PX} px."
+                f"{image.width} × {image.height} px ({image.width * image.height} pixels); "
+                f"a single answer cannot be split. Document limits are "
+                f"{MAX_HEIGHT_PX} px high and {MAX_PIXELS} pixels."
             )
-        if height + image.height > MAX_HEIGHT_PX:
+        if not document_fits(max(width, image.width), height + image.height,
+                             MAX_HEIGHT_PX, MAX_PIXELS):
             batches.append((start, index))
             start = index
             height = 0
+            width = 0
         height += image.height
+        width = max(width, image.width)
     batches.append((start, len(label_images)))
 
     with staged_directory(output_dir) as staging:
@@ -251,6 +256,12 @@ def _save_document(
 ) -> None:
     max_width = max(image.width for image in label_images)
     total_height = sum(image.height for image in label_images)
+    if not document_fits(max_width, total_height, MAX_HEIGHT_PX, MAX_PIXELS):
+        raise CliError(
+            f"Document would be {max_width} × {total_height} px "
+            f"({max_width * total_height} pixels); document limits are "
+            f"{MAX_HEIGHT_PX} px high and {MAX_PIXELS} pixels."
+        )
     concatenated = Image.new("RGB", (max_width, total_height), "white")
     checkbox_map: list[dict[str, Any]] = []
     current_y = 0

copienator/configuration.py

@@ -41,7 +41,8 @@ THINKING_LEVEL_PRO = "high"
 ALWAYS_CROP = False
 ANNOTATION_MARGIN_LEFT = 300
 ANNOTATION_LOW_MARGIN_LEFT = 400
-ANNOTATION_MAX_HEIGHT_PX = 32000
+ANNOTATION_MAX_HEIGHT_PX = 33000
+ANNOTATION_MAX_PIXELS = 67_108_864
 SHOW_PERSONAL_STEPS = False
 RETURN_JPEG_ENABLED = True
 RETURN_PDF_ENABLED = True

default_config.py

@@ -11,8 +11,14 @@ ANNOTATION_MARGIN_LEFT = 300
 ANNOTATION_LOW_MARGIN_LEFT = 400
 
 # Hauteur maximale des documents Bnot et des groupes BGnot, en pixels.
-# BGnot conserve entier un bloc d'élève qui dépasse cette limite.
-ANNOTATION_MAX_HEIGHT_PX = 32000
+# Les blocs d'élèves trop grands sont séparés entre deux réponses.
+ANNOTATION_MAX_HEIGHT_PX = 33000
+
+# Surface maximale : largeur maximale du document × hauteur totale, en pixels.
+# Sur DS02VA, les quatre groupes dépassant 2**26 pixels deviennent flous dans
+# Squid Android. Cela correspond à 256 Mio pour un bitmap à 4 octets/pixel.
+# Seuil déduit des observations, pas une limite documentée par Squid.
+ANNOTATION_MAX_PIXELS = 67_108_864
 
 # Fichiers à inclure dans A Rendre (les sources d'annotations sont conservées).
 RETURN_JPEG_ENABLED = True

docs/guide_utilisateur_gui.md

@@ -399,6 +399,16 @@ Choisir le mode :
    Les groupes de label sont ceux qui ont été générés dans l'analyse
    de l'énoncé. Il est possible de les modifier à la main ici.
 
+Les modes Bnot et BGnot respectent deux limites configurables dans `config.py` :
+`ANNOTATION_MAX_HEIGHT_PX = 33000` et `ANNOTATION_MAX_PIXELS = 67108864`.
+La surface compte la largeur maximale du document multipliée par sa hauteur
+totale. Le seuil de 2²⁶ pixels vient des quatre groupes DS02VA devenus flous
+dans Squid Android au-delà de cette surface ; il correspond à 256 Mio pour un
+bitmap à quatre octets par pixel, sans être une limite documentée par Squid.
+Les documents sont séparés entre les réponses pour respecter les deux limites.
+En BGnot, les réponses d'un élève restent ensemble lorsqu'elles tiennent dans
+un document. Une réponse individuelle trop grande produit une erreur explicite.
+
 **Écraser les sorties** remplace les documents existants. **Mode
 refaire** ne concerne qu’une reprise sélectionnée ; utilisez de
 préférence le parcours dédié décrit plus bas.

tests/test_annotation_height_config.py

@@ -8,7 +8,7 @@ import unittest
 
 
 class AnnotationHeightConfigTests(unittest.TestCase):
-    def load_generators(self, setting):
+    def load_generators(self, setting, attribute="MAX_HEIGHT_PX"):
         with tempfile.TemporaryDirectory() as directory:
             config = Path(directory) / "config.py"
             config.write_text("from default_config import *\n" + setting, encoding="utf-8")
@@ -16,7 +16,7 @@ class AnnotationHeightConfigTests(unittest.TestCase):
                 [sys.executable, "-c",
                  "from copienator.commands import annotating_by_label as grouped; "
                  "from copienator.commands import annotating_with_checks as checks; "
-                 "print(grouped.MAX_HEIGHT_PX, checks.MAX_HEIGHT_PX)"],
+                 f"print(grouped.{attribute}, checks.{attribute})"],
                 cwd=Path(__file__).resolve().parents[1],
                 env={**os.environ, "COPIENATOR_CONFIG": str(config)},
                 capture_output=True, text=True, check=False,
@@ -30,7 +30,7 @@ class AnnotationHeightConfigTests(unittest.TestCase):
     def test_older_configuration_keeps_current_default(self):
         result = self.load_generators("del ANNOTATION_MAX_HEIGHT_PX\n")
         self.assertEqual(result.returncode, 0, result.stderr)
-        self.assertEqual(result.stdout.strip(), "32000 32000")
+        self.assertEqual(result.stdout.strip(), "33000 33000")
 
     def test_invalid_heights_are_rejected(self):
         for value in ("0", "-1", "1.5", "True", "'32000'"):
@@ -38,3 +38,20 @@ class AnnotationHeightConfigTests(unittest.TestCase):
                 result = self.load_generators(f"ANNOTATION_MAX_HEIGHT_PX = {value}\n")
                 self.assertNotEqual(result.returncode, 0)
                 self.assertIn("ANNOTATION_MAX_HEIGHT_PX must be a positive integer", result.stderr)
+
+    def test_custom_pixel_limit_reaches_both_generators(self):
+        result = self.load_generators("ANNOTATION_MAX_PIXELS = 50000000\n", "MAX_PIXELS")
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(result.stdout.strip(), "50000000 50000000")
+
+    def test_older_configuration_keeps_default_pixel_limit(self):
+        result = self.load_generators("del ANNOTATION_MAX_PIXELS\n", "MAX_PIXELS")
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(result.stdout.strip(), "67108864 67108864")
+
+    def test_invalid_pixel_limits_are_rejected(self):
+        for value in ("0", "-1", "1.5", "True", "'67108864'"):
+            with self.subTest(value=value):
+                result = self.load_generators(f"ANNOTATION_MAX_PIXELS = {value}\n")
+                self.assertNotEqual(result.returncode, 0)
+                self.assertIn("ANNOTATION_MAX_PIXELS must be a positive integer", result.stderr)

tests/test_checked_documents.py

@@ -54,7 +54,7 @@ class CheckedDocumentTests(unittest.TestCase):
                                           overwrite=overwrite, output_mode='checks')
 
     def test_splits_on_labels_and_resets_geometry_at_each_part(self):
-        self.assertEqual(MAX_HEIGHT_PX, 32000)
+        self.assertEqual(MAX_HEIGHT_PX, 33000)
         self.assertEqual(self.generate(), 'success')
         parts = copy_documents(self.output)
         self.assertEqual([part.name for part in parts], ['Part1', 'Part2'])
@@ -75,6 +75,30 @@ class CheckedDocumentTests(unittest.TestCase):
         self.assertTrue((self.output / 'Concat.pdf').is_file())
         self.assertFalse((self.output / 'Part1').exists())
 
+    def test_pixel_limit_splits_documents_at_exact_boundary(self):
+        with patch.object(checks, 'MAX_PIXELS', 6000):
+            self.assertEqual(self.generate(), 'success')
+        parts = copy_documents(self.output)
+        self.assertEqual([read_json(p / 'bnote.json')['height'] for p in parts], [60, 40, 60])
+
+    def test_pixel_limit_accounts_for_width_growth_and_preserves_previous_copy(self):
+        original = self.compose
+        def wider(base, label, *args, **kwargs):
+            image, header = original(base, label, *args, **kwargs)
+            if label == 'Ex 2':
+                image = image.resize((200, image.height))
+            return image, header
+        with patch.object(self, 'compose', side_effect=wider), patch.object(checks, 'MAX_PIXELS', 12000):
+            self.assertEqual(self.generate(), 'success')
+        parts = copy_documents(self.output)
+        self.assertEqual([read_json(p / 'bnote.json')['width'] for p in parts], [100, 200, 100])
+        sentinel = self.output / 'sentinel'
+        sentinel.write_text('previous')
+        with patch.object(checks, 'MAX_PIXELS', 5999):
+            with self.assertRaisesRegex(CliError, 'Copie01.*Ex 1'):
+                self.generate(overwrite=True)
+        self.assertEqual(sentinel.read_text(), 'previous')
+
     def test_single_oversized_answer_and_save_failure_preserve_previous_copy(self):
         self.generate()
         sentinel = self.output / 'sentinel'

tests/test_gui_core.py

@@ -2330,7 +2330,7 @@ class StandardCliTests(unittest.TestCase):
             all(sum(item[2].height for item in batch) <= 100 for batch in batches)
         )
 
-    def test_grouped_batching_never_splits_one_student(self) -> None:
+    def test_grouped_batching_splits_oversized_student_between_answers(self) -> None:
         module = self.modules["annotating_by_label"]
         image = Image.new("RGB", (10, 60), "white")
         rendered = [
@@ -2340,27 +2340,44 @@ class StandardCliTests(unittest.TestCase):
         ]
         with patch.object(module, "MAX_HEIGHT_PX", 100):
             batches = module.split_batches(rendered)
-        self.assertEqual(batches, [rendered[:2], rendered[2:]])
+        self.assertEqual(batches, [[rendered[0]], [rendered[1]], [rendered[2]]])
 
-    def test_grouped_oversized_answer_is_saved_as_its_own_group(self) -> None:
+    def test_grouped_oversized_answer_is_rejected(self) -> None:
+        module = self.modules["annotating_by_label"]
+        rendered = [("02", "Ex 1", Image.new("RGB", (10, 120)), 0, [])]
+        with tempfile.TemporaryDirectory() as directory, patch.object(module, "MAX_HEIGHT_PX", 100):
+            with self.assertRaisesRegex(CliError, "Copie02.*Ex 1"):
+                module.split_batches(rendered)
+            with self.assertRaises(CliError):
+                module.save_batch(rendered, "Ex 1", 1, Path(directory))
+            self.assertFalse((Path(directory) / "Ex 1 G1").exists())
+
+    def test_grouped_pixels_include_padding_to_the_widest_answer(self) -> None:
         module = self.modules["annotating_by_label"]
         rendered = [
-            ("01", "Ex 1", Image.new("RGB", (10, 40)), 0, []),
-            ("02", "Ex 1", Image.new("RGB", (10, 120)), 0, []),
-            ("03", "Ex 1", Image.new("RGB", (10, 50)), 0, []),
+            ("01", "Ex 1", Image.new("RGB", (10, 60)), 0, []),
+            ("02", "Ex 1", Image.new("RGB", (30, 20)), 0, []),
+            ("03", "Ex 1", Image.new("RGB", (10, 40)), 0, []),
         ]
-        with tempfile.TemporaryDirectory() as directory, patch.object(module, "MAX_HEIGHT_PX", 100):
+        with patch.object(module, "MAX_HEIGHT_PX", 100), patch.object(module, "MAX_PIXELS", 2000):
             batches = module.split_batches(rendered)
             self.assertEqual(batches, [[rendered[0], rendered[2]], [rendered[1]]])
-            for index, batch in enumerate(batches, start=1):
+            with tempfile.TemporaryDirectory() as directory:
+                for index, batch in enumerate(batches):
                     module.save_batch(batch, "Ex 1", index, Path(directory))
-            metadata = read_json(Path(directory) / "Ex 1 G2/bnote.json")
-            self.assertEqual(metadata["height"], 120)
-            self.assertEqual([item["id"] for item in metadata["images"]], ["02"])
-            self.assertTrue((Path(directory) / "Ex 1 G2/Concat.pdf").is_file())
                 with self.assertRaises(CliError):
                     module.save_batch(rendered[:2], "Ex 1", 3, Path(directory))
 
+    def test_grouped_pixel_limit_splits_student_and_rejects_single_answer(self) -> None:
+        module = self.modules["annotating_by_label"]
+        image = Image.new("RGB", (30, 40))
+        rendered = [("01", label, image, 0, []) for label in ("Ex 1", "Ex 2")]
+        with patch.object(module, "MAX_HEIGHT_PX", 100), patch.object(module, "MAX_PIXELS", 1200):
+            self.assertEqual(module.split_batches(rendered), [[rendered[0]], [rendered[1]]])
+        with patch.object(module, "MAX_PIXELS", 1199):
+            with self.assertRaisesRegex(CliError, "single answer cannot be split"):
+                module.split_batches(rendered)
+
     def test_grouped_batching_reuses_space_in_earlier_groups(self) -> None:
         module = self.modules["annotating_by_label"]
         rendered = [
@@ -2378,9 +2395,9 @@ class StandardCliTests(unittest.TestCase):
                          [100, 100])
         self.assertEqual(batches[0][-2:], rendered[-2:])
 
-    def test_grouped_annotation_height_limit_is_32000_pixels(self) -> None:
+    def test_grouped_annotation_height_limit_is_33000_pixels(self) -> None:
         module = self.modules["annotating_by_label"]
-        self.assertEqual(module.MAX_HEIGHT_PX, 32000)
+        self.assertEqual(module.MAX_HEIGHT_PX, 33000)
 
 
 class WorkflowTests(unittest.TestCase):