From a38c1ff4b31a2dd1899caeadbe81eb3ba22233e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= Date: Mon, 26 Oct 2026 15:09:05 +0100 Subject: [PATCH] Enforce configurable height and pixel limits for tablet documents --- Script.org | 23 ++++--- copienator/annotation_documents.py | 9 ++- copienator/commands/annotating_by_label.py | 68 +++++++++++++------ copienator/commands/annotating_with_checks.py | 21 ++++-- copienator/configuration.py | 3 +- default_config.py | 10 ++- docs/guide_utilisateur_gui.md | 10 +++ tests/test_annotation_height_config.py | 23 ++++++- tests/test_checked_documents.py | 26 ++++++- tests/test_gui_core.py | 51 +++++++++----- 10 files changed, 187 insertions(+), 57 deletions(-) diff --git a/Script.org b/Script.org index 6ea2470..cbb5597 100644 --- a/Script.org +++ b/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. diff --git a/copienator/annotation_documents.py b/copienator/annotation_documents.py index 13faa27..b5e5f22 100644 --- a/copienator/annotation_documents.py +++ b/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]: diff --git a/copienator/commands/annotating_by_label.py b/copienator/commands/annotating_by_label.py index e7745ee..24dec40 100644 --- a/copienator/commands/annotating_by_label.py +++ b/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 diff --git a/copienator/commands/annotating_with_checks.py b/copienator/commands/annotating_with_checks.py index 25efd29..9f7ae39 100644 --- a/copienator/commands/annotating_with_checks.py +++ b/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 diff --git a/copienator/configuration.py b/copienator/configuration.py index d822b32..4cfe238 100644 --- a/copienator/configuration.py +++ b/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 diff --git a/default_config.py b/default_config.py index e39913d..baee267 100644 --- a/default_config.py +++ b/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 diff --git a/docs/guide_utilisateur_gui.md b/docs/guide_utilisateur_gui.md index 62034c8..7b3a9a9 100644 --- a/docs/guide_utilisateur_gui.md +++ b/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. diff --git a/tests/test_annotation_height_config.py b/tests/test_annotation_height_config.py index 052ac97..23987c6 100644 --- a/tests/test_annotation_height_config.py +++ b/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) diff --git a/tests/test_checked_documents.py b/tests/test_checked_documents.py index 742362e..e3b50e5 100644 --- a/tests/test_checked_documents.py +++ b/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' diff --git a/tests/test_gui_core.py b/tests/test_gui_core.py index 811b08e..039b0e5 100644 --- a/tests/test_gui_core.py +++ b/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,26 +2340,43 @@ 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): - 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)) + with tempfile.TemporaryDirectory() as directory: + for index, batch in enumerate(batches): + module.save_batch(batch, "Ex 1", index, Path(directory)) + 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"] @@ -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):