diff --git a/src/documents/parsers.py b/src/documents/parsers.py index a669c20ac..2e26eb4fc 100644 --- a/src/documents/parsers.py +++ b/src/documents/parsers.py @@ -79,8 +79,8 @@ _THUMBNAIL_MAX_WIDTH = 500 _THUMBNAIL_MAX_HEIGHT = 5000 # Used only when the page geometry cannot be read _THUMBNAIL_FALLBACK_DPI = 150 -# Cap on the target density, applied before supersampling: a capped tiny page -# is rasterized at twice this and halved to the size a 300 DPI render gave +# Cap on the target density, applied before supersampling, so a tiny page is +# not blown up past the size a plain 300 DPI render would give _THUMBNAIL_MAX_DPI = 300 # Pages with known geometry are rendered at this multiple of the computed DPI # and downsampled with Lanczos, which keeps text noticeably crisper than @@ -93,7 +93,6 @@ def rasterize_pdf_page_to_png( out_path: Path, *, dpi: int, - use_cropbox: bool = True, logging_group=None, ) -> None: """ @@ -113,10 +112,10 @@ def rasterize_pdf_page_to_png( str(dpi), "-png", "-singlefile", + "-cropbox", + str(in_path), + str(out_path.with_suffix("")), ] - if use_cropbox: - args.append("-cropbox") - args += [str(in_path), str(out_path.with_suffix(""))] logger.debug("Execute: " + " ".join(args), extra={"group": logging_group}) @@ -132,18 +131,15 @@ def encode_thumbnail_webp( png_path: Path, out_path: Path, *, - max_width: int = _THUMBNAIL_MAX_WIDTH, - max_height: int = _THUMBNAIL_MAX_HEIGHT, supersample: int = 1, ) -> None: """ Flattens any alpha onto white and saves the image as WebP. A render made at supersample times the target density is first - downsampled by that factor with Lanczos. max_width/max_height then trim - the result to the exact thumbnail size, since the computed DPI is rounded - up and lands at or slightly above it. The image is never enlarged, - matching the previous "-scale WxH>" behavior. + downsampled by that factor with Lanczos. The result is then trimmed to the + thumbnail size, since the computed DPI is rounded up and lands at or + slightly above it. The image is never enlarged. """ from PIL import Image @@ -164,7 +160,7 @@ def encode_thumbnail_webp( Image.Resampling.LANCZOS, ) - flattened.thumbnail((max_width, max_height)) + flattened.thumbnail((_THUMBNAIL_MAX_WIDTH, _THUMBNAIL_MAX_HEIGHT)) flattened.save(out_path, format="WEBP") except (OSError, Image.DecompressionBombError) as e: raise ParseError(f"Unable to encode thumbnail from {png_path}") from e @@ -172,15 +168,8 @@ def encode_thumbnail_webp( def _compute_thumbnail_dpi(in_path: Path, logging_group=None) -> tuple[int, int]: """ - Computes the DPI at which the first page of the PDF reaches at or just - above the thumbnail size, never above the 300 DPI the thumbnail was - previously rendered at before being scaled down, and the supersampling - factor to render with. - - Returns (dpi, supersample). The page is rendered at supersample * dpi and - downsampled by supersample afterwards. When the page geometry cannot be - read the fixed fallback DPI is used without supersampling, as that render - is not bounded by the thumbnail size. + Returns (dpi, supersample): render at dpi * supersample, then downsample + by supersample. Unknown page geometry gives the fallback DPI, unsupersampled. """ from paperless.parsers.utils import get_pdf_first_page_size_points @@ -195,17 +184,13 @@ def _compute_thumbnail_dpi(in_path: Path, logging_group=None) -> tuple[int, int] width_pts, height_pts = size dpi_for_width = _THUMBNAIL_MAX_WIDTH * 72 / width_pts dpi_for_height = _THUMBNAIL_MAX_HEIGHT * 72 / height_pts - # The old pipeline rendered at 300 DPI and then shrank to fit, so only - # pages too small to reach the thumbnail size even at 300 DPI end up - # smaller than it. Rounding up keeps the downsampled render at or above - # the target, so the shrink-only clamp in encode_thumbnail_webp trims it - # to exactly the thumbnail size instead of leaving it a few pixels short. + # Round up so the downsampled render is never a few pixels short of the + # target; the shrink-only clamp in encode_thumbnail_webp trims the excess. dpi = max( 1, math.ceil(min(_THUMBNAIL_MAX_DPI, dpi_for_width, dpi_for_height)), ) - # A page so large it hits the 1 DPI floor is already oversized, so - # doubling it would only quadruple the pixel count for no benefit + # At the 1 DPI floor the page is already oversized, so do not supersample return dpi, 1 if dpi == 1 else _THUMBNAIL_SUPERSAMPLE @@ -220,20 +205,33 @@ def _render_pdf_thumbnail( in_path, png_path, dpi=dpi * supersample, - use_cropbox=True, logging_group=logging_group, ) encode_thumbnail_webp(png_path, out_path, supersample=supersample) +def _repair_pdf_with_qpdf(in_path: Path, out_path: Path) -> None: + # qpdf rewrites in place, so work on a copy and leave the original alone. + # qpdf exits 3 when it had to repair the file, which is the expected + # outcome here, so warnings must not count as failure. + try: + shutil.copy(in_path, out_path) + run_subprocess( + ["qpdf", "--warning-exit-0", "--replace-input", str(out_path)], + logger=logger, + ) + except (subprocess.CalledProcessError, OSError) as e: + raise ParseError(f"qpdf repair failed for {in_path}") from e + + def make_thumbnail_from_pdf_qpdf_fallback( in_path: Path, temp_dir: Path, logging_group=None, ) -> Path: - png_path: Path = Path(temp_dir) / "page1_repaired.png" - out_path: Path = Path(temp_dir) / "convert_qpdf.webp" - repaired_path: Path = Path(temp_dir) / "repaired.pdf" + png_path = temp_dir / "page1_repaired.png" + out_path = temp_dir / "convert_qpdf.webp" + repaired_path = temp_dir / "repaired.pdf" logger.warning( "Thumbnail generation with pdftoppm failed, attempting qpdf repair and retry.", @@ -241,25 +239,8 @@ def make_thumbnail_from_pdf_qpdf_fallback( ) try: - # qpdf rewrites in place, so work on a copy and leave the original alone. - # qpdf exits 3 when it had to repair the file, which is the expected - # outcome here, so warnings must not count as failure. - try: - shutil.copy(in_path, repaired_path) - run_subprocess( - [ - "qpdf", - "--warning-exit-0", - "--replace-input", - str(repaired_path), - ], - logger=logger, - ) - except (subprocess.CalledProcessError, OSError) as e: - raise ParseError(f"qpdf repair failed for {in_path}") from e - + _repair_pdf_with_qpdf(in_path, repaired_path) _render_pdf_thumbnail(repaired_path, png_path, out_path, logging_group) - return out_path except ParseError as e: @@ -267,7 +248,7 @@ def make_thumbnail_from_pdf_qpdf_fallback( # The caller might expect a generated thumbnail that can be moved, # so we need to copy it before it gets moved. # https://github.com/paperless-ngx/paperless-ngx/issues/3631 - default_thumbnail_path: Path = Path(temp_dir) / "document.webp" + default_thumbnail_path = temp_dir / "document.webp" copy_file_with_basic_stats(get_default_thumbnail(), default_thumbnail_path) return default_thumbnail_path diff --git a/src/documents/tests/test_parsers.py b/src/documents/tests/test_parsers.py index 23155b60e..835aea2a5 100644 --- a/src/documents/tests/test_parsers.py +++ b/src/documents/tests/test_parsers.py @@ -169,10 +169,11 @@ class TestComputeThumbnailDpi: WHEN: - The thumbnail DPI is computed THEN: - - The DPI is rounded up so the render reaches 500x5000, never - exceeds 300, is at least 1, and is supersampled 2x (except at the - 1 DPI floor); an unknown - size gives the plain 150 DPI fallback without supersampling + - The DPI is rounded up so the render reaches 500x5000, is capped + at 300 and floored at 1, and is supersampled 2x except at the + 1 DPI floor + - An unknown size gives the plain 150 DPI fallback without + supersampling """ mocker.patch( "paperless.parsers.utils.get_pdf_first_page_size_points", @@ -182,6 +183,19 @@ class TestComputeThumbnailDpi: class TestMakeThumbnailFromPdf: + @pytest.fixture + def work_dir(self, tmp_path: Path) -> Path: + path = tmp_path / "work" + path.mkdir() + return path + + @staticmethod + def _write_blank_pdf(path: Path, page_size: tuple[int, int] = (612, 792)) -> Path: + pdf = pikepdf.new() + pdf.add_blank_page(page_size=page_size) + pdf.save(path, object_stream_mode=pikepdf.ObjectStreamMode.disable) + return path + @pytest.mark.parametrize( ("size", "expected_dpi", "expected_supersample"), [ @@ -193,6 +207,7 @@ class TestMakeThumbnailFromPdf: self, mocker: MockerFixture, tmp_path: Path, + work_dir: Path, size: tuple[float, float] | None, expected_dpi: int, expected_supersample: int, @@ -213,8 +228,6 @@ class TestMakeThumbnailFromPdf: ) rasterize = mocker.patch("documents.parsers.rasterize_pdf_page_to_png") encode = mocker.patch("documents.parsers.encode_thumbnail_webp") - work_dir = tmp_path / "work" - work_dir.mkdir() make_thumbnail_from_pdf(tmp_path / "in.pdf", work_dir) @@ -234,6 +247,7 @@ class TestMakeThumbnailFromPdf: def test_thumbnail_width( self, tmp_path: Path, + work_dir: Path, page_size: tuple[int, int], expected_width: int, ) -> None: @@ -246,12 +260,7 @@ class TestMakeThumbnailFromPdf: - The WebP thumbnail is exactly 500px wide, unless the page is too small to reach that even at 300 DPI """ - pdf = pikepdf.new() - pdf.add_blank_page(page_size=page_size) - pdf_path = tmp_path / "in.pdf" - pdf.save(pdf_path) - work_dir = tmp_path / "work" - work_dir.mkdir() + pdf_path = self._write_blank_pdf(tmp_path / "in.pdf", page_size) thumb = make_thumbnail_from_pdf(pdf_path, work_dir) @@ -260,20 +269,22 @@ class TestMakeThumbnailFromPdf: assert im.format == "WEBP" assert im.width == expected_width - @staticmethod - def _write_pdf_without_xref(path: Path) -> Path: + @classmethod + def _write_pdf_without_xref(cls, path: Path) -> Path: """ Writes a valid one page PDF, then cuts off its cross reference table and trailer, which poppler cannot recover from but qpdf can. """ - pdf = pikepdf.new() - pdf.add_blank_page(page_size=(612, 792)) - pdf.save(path, object_stream_mode=pikepdf.ObjectStreamMode.disable) + cls._write_blank_pdf(path) data = path.read_bytes() path.write_bytes(data[: data.rindex(b"\nxref")]) return path - def test_qpdf_repair_produces_real_thumbnail(self, tmp_path: Path) -> None: + def test_qpdf_repair_produces_real_thumbnail( + self, + tmp_path: Path, + work_dir: Path, + ) -> None: """ GIVEN: - A PDF without a cross reference table or trailer, which @@ -288,8 +299,6 @@ class TestMakeThumbnailFromPdf: """ pdf_path = self._write_pdf_without_xref(tmp_path / "broken.pdf") original_bytes = pdf_path.read_bytes() - work_dir = tmp_path / "work" - work_dir.mkdir() with pytest.raises(ParseError): rasterize_pdf_page_to_png(pdf_path, work_dir / "probe.png", dpi=50) @@ -297,7 +306,6 @@ class TestMakeThumbnailFromPdf: thumb = make_thumbnail_from_pdf(pdf_path, work_dir) assert thumb == work_dir / "convert_qpdf.webp" - assert thumb.read_bytes() != get_default_thumbnail().read_bytes() with Image.open(thumb) as im: assert im.format == "WEBP" assert im.width == 500 @@ -314,6 +322,7 @@ class TestMakeThumbnailFromPdf: self, mocker: MockerFixture, tmp_path: Path, + work_dir: Path, qpdf_error: subprocess.CalledProcessError | None, ) -> None: """ @@ -332,17 +341,11 @@ class TestMakeThumbnailFromPdf: ) if qpdf_error is not None: mocker.patch("documents.parsers.run_subprocess", side_effect=qpdf_error) - pdf = pikepdf.new() - pdf.add_blank_page(page_size=(612, 792)) - pdf_path = tmp_path / "in.pdf" - pdf.save(pdf_path) - work_dir = tmp_path / "work" - work_dir.mkdir() + pdf_path = self._write_blank_pdf(tmp_path / "in.pdf") thumb = make_thumbnail_from_pdf(pdf_path, work_dir) assert thumb == work_dir / "document.webp" - assert thumb != get_default_thumbnail() assert thumb.read_bytes() == get_default_thumbnail().read_bytes() @@ -457,49 +460,19 @@ class TestEncodeThumbnailWebp: red, green, blue = im.getpixel((10, 5)) assert min(red, green, blue) >= 250 - @pytest.mark.parametrize( - ("in_size", "expected_size"), - [ - pytest.param((1000, 2000), (500, 1000), id="too-wide-shrunk"), - pytest.param((100, 10000), (50, 5000), id="too-tall-shrunk"), - pytest.param((100, 200), (100, 200), id="small-not-enlarged"), - ], - ) - def test_size_clamped_without_enlarging( - self, - tmp_path: Path, - in_size: tuple[int, int], - expected_size: tuple[int, int], - ) -> None: - """ - GIVEN: - - A rendered page image of the given size - WHEN: - - It is encoded as a thumbnail - THEN: - - It is shrunk to fit 500x5000 keeping aspect ratio, and never - enlarged - """ - png_path = tmp_path / "in.png" - Image.new("RGB", in_size, (255, 255, 255)).save(png_path) - out_path = tmp_path / "out.webp" - - encode_thumbnail_webp(png_path, out_path) - - with Image.open(out_path) as im: - assert im.size == expected_size - @pytest.mark.parametrize( ("in_size", "supersample", "expected_size"), [ + pytest.param((1000, 2000), 1, (500, 1000), id="too-wide-shrunk"), + pytest.param((100, 10000), 1, (50, 5000), id="too-tall-shrunk"), + pytest.param((100, 200), 1, (100, 200), id="small-not-enlarged"), pytest.param((1000, 1400), 2, (500, 700), id="2x-halved"), pytest.param((1001, 1401), 2, (500, 700), id="2x-odd-rounded"), - pytest.param((1000, 1400), 1, (500, 700), id="no-supersample-clamped"), pytest.param((600, 800), 2, (300, 400), id="2x-small-not-enlarged"), pytest.param((900, 1200), 2, (450, 600), id="2x-below-clamp"), ], ) - def test_supersampled_render_downsampled( + def test_size_clamped_and_downsampled( self, tmp_path: Path, in_size: tuple[int, int], @@ -508,11 +481,13 @@ class TestEncodeThumbnailWebp: ) -> None: """ GIVEN: - - A rendered page image made at a supersampling factor + - A rendered page image of the given size, made at the given + supersampling factor WHEN: - It is encoded as a thumbnail with that factor THEN: - - It is downsampled by the factor before the 500x5000 clamp + - It is downsampled by the factor, then shrunk to fit 500x5000 + keeping aspect ratio, and never enlarged """ png_path = tmp_path / "in.png" Image.new("RGB", in_size, (255, 255, 255)).save(png_path) diff --git a/src/paperless/tests/parsers/test_tesseract_parser.py b/src/paperless/tests/parsers/test_tesseract_parser.py index 2effa0a8a..c47c5a2f9 100644 --- a/src/paperless/tests/parsers/test_tesseract_parser.py +++ b/src/paperless/tests/parsers/test_tesseract_parser.py @@ -17,8 +17,8 @@ import pytest from ocrmypdf import SubprocessOutputError from PIL import Image -import documents.parsers from documents.parsers import ParseError +from documents.parsers import rasterize_pdf_page_to_png from paperless.models import ModeChoices from paperless.parsers import ParserProtocol from paperless.parsers.tesseract import RasterisedDocumentParser @@ -328,13 +328,12 @@ class TestGetThumbnail: - The PDF is repaired with qpdf and rasterized again, producing a real thumbnail rather than the default placeholder """ - real_rasterize = documents.parsers.rasterize_pdf_page_to_png original = tesseract_samples_dir / "simple-digital.pdf" def _fail_on_original(in_path: Path, out_path: Path, **kwargs) -> None: if in_path == original: raise ParseError("Does not compute.") - real_rasterize(in_path, out_path, **kwargs) + rasterize_pdf_page_to_png(in_path, out_path, **kwargs) rasterize = mocker.patch( "documents.parsers.rasterize_pdf_page_to_png",