Skip to content

bug/trim-large-pages: hi_res page trimming keeps the wrong slice when the MediaBox does not start at y=0 #363

Open

Description

@Mohith26

Describe the bug

With strategy="hi_res" and PDF splitting, SplitPdfHook._trim_large_pages trims any page taller than MAX_PAGE_LENGTH (4000pt) to its top 4000pt. It does that by setting mediabox.top = mediabox.height and then bottom = top - 4000, which assumes the MediaBox starts at y=0. When the MediaBox has a non-zero bottom edge, the window it keeps is shifted down by that offset:

  • MediaBox [0, 1000, 612, 6000] becomes [0, 1000, 612, 5000]: the top 1000pt of the page are dropped and the bottom part is kept instead.
  • MediaBox [0, 5000, 612, 10000] becomes [0, 1000, 612, 5000], which doesn't overlap the page at all, so that chunk goes to the API as a blank page.

To Reproduce

On main (c1b153f, unstructured-client 0.47.0, pypdf 6.21.4):

import io, sys
from pypdf import PdfReader, PdfWriter
from pypdf.generic import RectangleObject
from unstructured_client._hooks.custom.split_pdf_hook import SplitPdfHook, MAX_PAGE_LENGTH

origin = float(sys.argv[1]) if len(sys.argv) > 1 else 1000.0
w = PdfWriter()
page = w.add_blank_page(width=612, height=5000)
page.mediabox = RectangleObject([0, origin, 612, origin + 5000])  # 5000pt tall, bottom edge at y=origin
buf = io.BytesIO(); w.write(buf); buf.seek(0)

before = PdfReader(buf).pages[0].mediabox
after = SplitPdfHook()._trim_large_pages(PdfReader(buf), {"strategy": "hi_res"}).pages[0].mediabox
print("original:", [float(x) for x in before])
print("trimmed: ", [float(x) for x in after])
print("expected:", [0.0, float(before.top) - MAX_PAGE_LENGTH, 612.0, float(before.top)])
origin 0     trimmed [0, 1000, 612, 5000]   expected [0, 1000, 612, 5000]
origin 1000  trimmed [0, 1000, 612, 5000]   expected [0, 2000, 612, 6000]
origin 5000  trimmed [0, 1000, 612, 5000]   expected [0, 6000, 612, 10000]

Expected behavior

Keep the top 4000pt of the page wherever the MediaBox sits. Dropping the top assignment and only moving the bottom edge does that:

if page.mediabox.height >= max_page_length:
    page.mediabox.bottom = page.mediabox.top - max_page_length

With that change all three cases above match the expected box, and it is the same result as today for MediaBoxes that start at 0. _trim_large_pages has no direct test right now (the unit tests patch it out); I'd suggest a parametrized one over the MediaBox origin like the script above.

Additional context

This only runs when the hook actually splits (hi_res, more pages than one split or a page range). I found it reading the hook, not from a customer file, so I don't know how common offset MediaBoxes are in practice; they do show up in cropped or imposed PDFs. I saw CONTRIBUTING asks for issues rather than PRs, so filing it here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions