From 383223f0676bbc3ae2801e33637eb92f8aa33cbe Mon Sep 17 00:00:00 2001 From: Rossi-Luciano Date: Thu, 20 Aug 2026 21:47:57 -0300 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20corrige=20compara=C3=A7=C3=A3o=20de?= =?UTF-8?q?=20unidade=20(cm=20vs=20EMU)=20na=20decis=C3=A3o=20de=20largura?= =?UTF-8?q?=20de=20figura?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decide_figure_layout comparava a largura natural da imagem (width_in_cm, um float em centímetros) contra single_col_width - mas esse último degrada de Cm para um número cru em EMU após subtração/divisão (Length do python-docx não sobrecarrega operadores aritméticos pra preservar a unidade). A comparação cm >= EMU tornava o ramo full-width inalcançável na prática para qualquer imagem real, independente do tamanho. Converte de volta pra cm antes de comparar. Confirmado contra a Figura 1 real de tests/fixtures/pdf/a1.xml (mapa 698x493px, ~18.5cm de largura natural): antes sempre "double-column-layout" (dentro da coluna, ~8.2cm), depois "single-column-layout" (largura total), batendo com o PDF oficial (a1.pdf) - inclusive movendo a figura pra página 3, igual ao original. Refs #1278. --- .../sps/formats/pdf/renderer/docx/figure.py | 8 ++- tests/sps/formats/pdf/renderer/__init__.py | 0 .../sps/formats/pdf/renderer/docx/__init__.py | 0 .../formats/pdf/renderer/docx/test_figure.py | 54 +++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 tests/sps/formats/pdf/renderer/__init__.py create mode 100644 tests/sps/formats/pdf/renderer/docx/__init__.py create mode 100644 tests/sps/formats/pdf/renderer/docx/test_figure.py diff --git a/packtools/sps/formats/pdf/renderer/docx/figure.py b/packtools/sps/formats/pdf/renderer/docx/figure.py index 4139d5cc4..5fb77d1e9 100644 --- a/packtools/sps/formats/pdf/renderer/docx/figure.py +++ b/packtools/sps/formats/pdf/renderer/docx/figure.py @@ -101,8 +101,14 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB except Exception: return pdf_enum.SINGLE_COLUMN_PAGE_LABEL + # single_col_width degraded from a Cm object to a raw EMU number in the + # subtraction/division above (python-docx's Length has no operator + # overloads that preserve units) - convert back to cm so this compares + # against width_in_cm in the same unit, instead of cm against EMU. + single_col_width_cm = single_col_width / Cm(1) + # If the image is wider than a single column by the threshold factor, prefer a single-column-layout (full width). Otherwise, keep double-column-layout. - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL if width_in_cm >= float(threshold) * float(single_col_width) else pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + return pdf_enum.SINGLE_COLUMN_PAGE_LABEL if width_in_cm >= float(threshold) * single_col_width_cm else pdf_enum.DOUBLE_COLUMN_PAGE_LABEL # ----------------- diff --git a/tests/sps/formats/pdf/renderer/__init__.py b/tests/sps/formats/pdf/renderer/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sps/formats/pdf/renderer/docx/__init__.py b/tests/sps/formats/pdf/renderer/docx/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sps/formats/pdf/renderer/docx/test_figure.py b/tests/sps/formats/pdf/renderer/docx/test_figure.py new file mode 100644 index 000000000..0f49fa32a --- /dev/null +++ b/tests/sps/formats/pdf/renderer/docx/test_figure.py @@ -0,0 +1,54 @@ +import os +import tempfile +import unittest + +from docx import Document +from PIL import Image + +from packtools.sps.formats.pdf.renderer.docx.figure import decide_figure_layout +from packtools.sps.formats.pdf import enum as pdf_enum + + +class TestDecideFigureLayoutUnits(unittest.TestCase): + """ + Regression test for a units bug: single_col_width degrades from a Cm + object to a raw EMU number under python-docx's Length arithmetic (it has + no operator overloads that preserve units), so comparing it directly + against width_in_cm (a real centimeter float) compared cm against EMU - + a ~360000x scale mismatch that made the full-width branch unreachable + for any real image, regardless of size. + """ + + def setUp(self): + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + + def _make_png(self, px_width, px_height=200, dpi=96): + path = os.path.join(self.tmpdir.name, f"img_{px_width}.png") + Image.new("RGB", (px_width, px_height), color="white").save( + path, format="PNG", dpi=(dpi, dpi) + ) + return path + + def test_wide_image_gets_full_width_layout(self): + # ~18.5cm at 96 DPI - wider than a single column (~8.2cm) by far more + # than the 1.1 threshold, on an A4/2-column default page. + img_path = self._make_png(px_width=698) + docx = Document() + fig = {"href": img_path, "label": "Figure 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.SINGLE_COLUMN_PAGE_LABEL + ) + + def test_narrow_image_stays_within_column(self): + # ~2.6cm at 96 DPI - well under the single-column width. + img_path = self._make_png(px_width=100) + docx = Document() + fig = {"href": img_path, "label": "Figure 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + + +if __name__ == "__main__": + unittest.main() From 89d7b7630e1c56b1f358bbc36354c9cf326b7788 Mon Sep 17 00:00:00 2001 From: Rossi-Luciano Date: Mon, 24 Aug 2026 13:46:03 -0300 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20corrige=20decis=C3=A3o=20de=20layout?= =?UTF-8?q?=20de=20figuras=20irm=C3=A3s=20com=20DPI=20inconsistente?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decide_figure_layout calcula a largura física da figura via pixels/DPI do metadado do arquivo para decidir entre largura total e largura de coluna. Analisando um artigo real (tests/fixtures/pdf/a4.xml, 7 gráficos "Graph 1" a "Graph 7" na mesma seção) apareceu um caso onde só um deles renderizava gigante em página de largura total, isolado dos demais - mesmo sendo do mesmo estilo/origem que os outros 6. Três causas, encontradas nesta ordem: 1. extract_figure_data (pipeline/xml.py) usava fig_node.find('.//graphic'), que casa com o primeiro em ordem de documento mesmo dentro de - sempre pegava o .tif de produção (primeiro na lista) em vez de rodar a lógica de ranqueamento já existente no código, que preferiria a variante specific-use="scielo-web". Corrigido para só casar com um filho direto, deixando cair na lógica de ranqueamento. 2. Corrigido (1), as 7 figuras passaram a usar .png sem metadado de DPI nenhum - caindo no fallback de 96 DPI (resolução de tela), baixo demais pra essas imagens (~700-800px), fazendo as 7 (não mais 1) virarem largura total. Fallback subiu de 96 para 300 DPI, batendo com a convenção observada nos arquivos corretamente rotulados deste mesmo corpus. 3. Mesmo com (1) e (2), o caso original de DPI explicitamente inconsistente entre arquivos-irmãos (72 DPI vs 300 DPI no .tif, quando não há pra escolher) continua possível. Adicionado _flag_dpi_outliers (pipeline/docx.py): compara o DPI entre as figuras de uma mesma seção antes de decidir o layout: se uma destoa da mediana do grupo por 2x ou mais, decide_figure_layout usa a mediana em vez do DPI do próprio arquivo (novo campo layout_dpi_override). Validado ponta a ponta contra a4.xml: antes, 4 seções DOCX alternando 1/2 colunas (Graph 1 isolado); depois, 2 seções (título + corpo), todos os 7 gráficos com tamanho consistente fluindo com o texto em 2 colunas. Contagem de página/palavras não regride. Refs #1278, #1293. Baseia-se no fix de comparação de unidade cm/EMU já commitado nesta branch (decide_figure_layout retornava sempre "double-column-layout" antes disso, o que mascararia completamente este bug). --- packtools/sps/formats/pdf/pipeline/docx.py | 33 ++++++++++ packtools/sps/formats/pdf/pipeline/xml.py | 7 ++- .../sps/formats/pdf/renderer/docx/figure.py | 63 +++++++++++++------ tests/sps/formats/pdf/pipeline/test_docx.py | 55 ++++++++++++++++ tests/sps/formats/pdf/pipeline/test_xml.py | 43 ++++++++++++- .../formats/pdf/renderer/docx/test_figure.py | 23 +++++++ 6 files changed, 201 insertions(+), 23 deletions(-) diff --git a/packtools/sps/formats/pdf/pipeline/docx.py b/packtools/sps/formats/pdf/pipeline/docx.py index 9c104f58a..1ec72bb8c 100644 --- a/packtools/sps/formats/pdf/pipeline/docx.py +++ b/packtools/sps/formats/pdf/pipeline/docx.py @@ -1,3 +1,5 @@ +import statistics + from packtools.sps.formats.pdf import enum as pdf_enum from packtools.sps.formats.pdf.pipeline import xml as xml_pipe from packtools.sps.formats.pdf.renderer import docx as docx_renderer @@ -548,8 +550,39 @@ def _figure_layout(docx, fig): return layout +def _flag_dpi_outliers(docx, figures, outlier_ratio=2.0): + """ + Compare embedded DPI across a batch of sibling figures (e.g. all the + graphs in one section) and flag figures whose own DPI is wildly off from + the group's median, so decide_figure_layout can be given a corrected + value instead of trusting a possibly-wrong per-image tag. Figures with + an explicit 'layout' override are left untouched; outliers get + 'layout_dpi_override' set to the group median. + """ + probed = [] + for fig in figures: + if not isinstance(fig, dict) or fig.get('layout'): + continue + probe = docx_renderer.figure.probe_image_dpi(docx, fig) + if probe is not None: + probed.append((fig, probe[1])) + + if len(probed) < 2: + return + + median_dpi = statistics.median(dpi for _, dpi in probed) + if median_dpi <= 0: + return + + for fig, dpi in probed: + ratio = dpi / median_dpi + if ratio >= outlier_ratio or ratio <= 1 / outlier_ratio: + fig['layout_dpi_override'] = median_dpi + + def _render_figures(docx, figures): """Render figures, switching to single column when the layout requires it.""" + _flag_dpi_outliers(docx, figures) for fig in figures: layout = _figure_layout(docx, fig) diff --git a/packtools/sps/formats/pdf/pipeline/xml.py b/packtools/sps/formats/pdf/pipeline/xml.py index 92d54758c..d5f67df68 100644 --- a/packtools/sps/formats/pdf/pipeline/xml.py +++ b/packtools/sps/formats/pdf/pipeline/xml.py @@ -423,11 +423,14 @@ def _get_href_from_node(node): caption_texts.append(txt) caption = ' '.join([c for c in caption_texts if c]) - # graphic may be direct child or inside + # graphic may be a direct child, or offered as several representations + # inside - only match the direct child here so the latter + # case falls through to the ranking logic below instead of grabbing the + # first in document order. href = None alt_text = None - graphic = fig_node.find('.//graphic') + graphic = fig_node.find('graphic') if graphic is not None: href = _get_href_from_node(graphic) alt_text = graphic.get('alt') or graphic.get('alt-text') diff --git a/packtools/sps/formats/pdf/renderer/docx/figure.py b/packtools/sps/formats/pdf/renderer/docx/figure.py index 5fb77d1e9..6d228c2f2 100644 --- a/packtools/sps/formats/pdf/renderer/docx/figure.py +++ b/packtools/sps/formats/pdf/renderer/docx/figure.py @@ -38,7 +38,7 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB Decide whether a figure should occupy the full page width (double-column-layout) or a single column (single-column-layout). Heuristic: - - Compute the natural width of the image in centimeters using Pillow and its DPI metadata (tries dpi, jfif_density; defaults to 150 DPI when missing). + - Compute the natural width of the image in centimeters using Pillow and its DPI metadata (tries dpi, jfif_density; defaults to 300 DPI when missing). - Compute the available content width (page width minus margins) and the single-column width ((content - column_spacing)/2). - If natural image width >= threshold * single-column width, return 'double-column-layout'; otherwise 'single-column-layout'. @@ -82,24 +82,21 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB column_spacing_cm = Cm(column_spacing_twips / 567.0) single_col_width = (content_width - column_spacing_cm) / 2 - # Resolve image path (local or download) - context = _get_docx_context(docx) - href = figure_data.get('href') if isinstance(figure_data, dict) else None - img_path = _resolve_image_path(href, context) - if not img_path: + probe = probe_image_dpi(docx, figure_data) + if probe is None: return pdf_enum.SINGLE_COLUMN_PAGE_LABEL + px_w, dpi = probe - # Open image and compute its natural width in Cm using DPI (default 72 DPI) - if not os.path.exists(img_path): - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL - try: - with Image.open(img_path) as im: - px_w = im.width - dpi = _infer_image_dpi(im) - # width in inches then to Cm - width_in_cm = (px_w / max(1.0, dpi)) * 2.54 - except Exception: - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL + # A caller may have compared this image's DPI against its siblings and + # supplied a more trustworthy value here - see 'layout_dpi_override'. + if isinstance(figure_data, dict) and figure_data.get('layout_dpi_override'): + try: + dpi = float(figure_data['layout_dpi_override']) + except (TypeError, ValueError): + pass + + # width in inches then to Cm + width_in_cm = (px_w / max(1.0, dpi)) * 2.54 # single_col_width degraded from a Cm object to a raw EMU number in the # subtraction/division above (python-docx's Length has no operator @@ -111,17 +108,43 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB return pdf_enum.SINGLE_COLUMN_PAGE_LABEL if width_in_cm >= float(threshold) * single_col_width_cm else pdf_enum.DOUBLE_COLUMN_PAGE_LABEL +def probe_image_dpi(docx, figure_data): + """ + Resolve a figure's image and return its (pixel_width, dpi), or None if + the image can't be opened. + """ + try: + from PIL import Image + except Exception: + return None + + context = _get_docx_context(docx) + href = figure_data.get('href') if isinstance(figure_data, dict) else None + img_path = _resolve_image_path(href, context) + if not img_path or not os.path.exists(img_path): + return None + + try: + with Image.open(img_path) as im: + return im.width, _infer_image_dpi(im) + except Exception: + return None + + # ----------------- # Private helpers # ----------------- +_NO_METADATA_DPI_FALLBACK = 300.0 + + def _infer_image_dpi(im) -> float: """Infer the horizontal DPI from a PIL Image, considering multiple metadata sources. Priority: - im.info['dpi']: tuple or number - im.info['jfif_unit'] and im.info['jfif_density'] (unit 1=inches, 2=cm) - Fallback: 96 dpi + Fallback: _NO_METADATA_DPI_FALLBACK. """ try: info = getattr(im, 'info', {}) or {} @@ -144,10 +167,10 @@ def _infer_image_dpi(im) -> float: if unit == 2: # per cm return float(density[0]) * 2.54 - return 96.0 + return _NO_METADATA_DPI_FALLBACK except Exception: - return 96.0 + return _NO_METADATA_DPI_FALLBACK def _add_paragraph_with_formatting(docx, text, style_name='SCL Paragraph'): """Minimal helper to add a paragraph with an optional style, avoiding circular imports.""" diff --git a/tests/sps/formats/pdf/pipeline/test_docx.py b/tests/sps/formats/pdf/pipeline/test_docx.py index 0755081e4..afbc90192 100644 --- a/tests/sps/formats/pdf/pipeline/test_docx.py +++ b/tests/sps/formats/pdf/pipeline/test_docx.py @@ -1,4 +1,5 @@ import unittest +from unittest.mock import patch from packtools.sps.formats.pdf.pipeline import docx as docx_pipe @@ -91,3 +92,57 @@ class TestDocxAcknowledgmentsPipe(unittest.TestCase): class TestDocxSupplementaryMaterialPipe(unittest.TestCase): # TODO ... + + +class TestFlagDpiOutliers(unittest.TestCase): + """Tests for _flag_dpi_outliers: flags a figure's DPI as untrustworthy + when it diverges from its sibling figures' median DPI, in the same + batch, by more than outlier_ratio.""" + + def test_flags_dpi_outlier_against_siblings(self): + figures = [{'href': 'graph1.tif'}, {'href': 'graph2.tif'}, {'href': 'graph3.tif'}] + probes = {'graph1.tif': (612, 72.0), 'graph2.tif': (800, 300.0), 'graph3.tif': (700, 300.0)} + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + side_effect=lambda docx, fig: probes[fig['href']], + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + + self.assertEqual(figures[0]['layout_dpi_override'], 300.0) + self.assertNotIn('layout_dpi_override', figures[1]) + self.assertNotIn('layout_dpi_override', figures[2]) + + def test_does_not_flag_similar_dpis(self): + figures = [{'href': 'a.tif'}, {'href': 'b.tif'}] + probes = {'a.tif': (700, 280.0), 'b.tif': (700, 300.0)} + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + side_effect=lambda docx, fig: probes[fig['href']], + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + + self.assertNotIn('layout_dpi_override', figures[0]) + self.assertNotIn('layout_dpi_override', figures[1]) + + def test_skips_figures_with_explicit_layout(self): + # 'a.tif' already has a resolved layout and must not be probed; only + # 'b.tif' should be. With a single figure left to probe there aren't + # enough siblings to compare against, so no override is set either. + figures = [{'href': 'a.tif', 'layout': 'single-column-layout'}, {'href': 'b.tif'}] + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + return_value=(700, 300.0), + ) as mock_probe: + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + mock_probe.assert_called_once_with(None, figures[1]) + self.assertNotIn('layout_dpi_override', figures[0]) + self.assertNotIn('layout_dpi_override', figures[1]) + + def test_single_figure_is_never_flagged(self): + figures = [{'href': 'a.tif'}] + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + return_value=(612, 72.0), + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + self.assertNotIn('layout_dpi_override', figures[0]) diff --git a/tests/sps/formats/pdf/pipeline/test_xml.py b/tests/sps/formats/pdf/pipeline/test_xml.py index f3b428a93..d4fc1c647 100644 --- a/tests/sps/formats/pdf/pipeline/test_xml.py +++ b/tests/sps/formats/pdf/pipeline/test_xml.py @@ -1238,7 +1238,48 @@ def test_extract_trans_abstract_data_custom_namespace(self): } ] result = xml_pipe.extract_trans_abstract_data( - xml, + xml, namespaces={'xml': 'http://custom.namespace'} ) self.assertEqual(result, expected) + + +class TestExtractFigureData(unittest.TestCase): + """Tests for extract_figure_data's graphic href resolution, including + the ranking logic used when a figure only offers .""" + + def test_direct_graphic_child_is_used_as_is(self): + xml = etree.fromstring( + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'figure1.jpg') + + def test_alternatives_prefers_scielo_web_over_raw_tif(self): + xml = etree.fromstring( + '' + '' + '' + '' + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'web.png') + + def test_alternatives_falls_back_to_tif_when_no_better_option(self): + xml = etree.fromstring( + '' + '' + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'raw.tif') diff --git a/tests/sps/formats/pdf/renderer/docx/test_figure.py b/tests/sps/formats/pdf/renderer/docx/test_figure.py index 0f49fa32a..c8a00c07d 100644 --- a/tests/sps/formats/pdf/renderer/docx/test_figure.py +++ b/tests/sps/formats/pdf/renderer/docx/test_figure.py @@ -49,6 +49,29 @@ def test_narrow_image_stays_within_column(self): decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL ) + def test_untagged_image_uses_print_resolution_fallback(self): + # An image with no DPI tag at all must fall back to print + # resolution, not screen resolution. + img_path = self._make_png(px_width=700) + with Image.open(img_path) as im: + im.info.pop("dpi", None) + im.save(img_path) # re-save without the dpi tag + docx = Document() + fig = {"href": img_path, "label": "Graph 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + + def test_layout_dpi_override_replaces_embedded_dpi(self): + # 'layout_dpi_override' must take precedence over the image's own + # embedded DPI. + img_path = self._make_png(px_width=612, dpi=72) + docx = Document() + fig = {"href": img_path, "label": "Graph 1", "layout_dpi_override": 300.0} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + if __name__ == "__main__": unittest.main()