Skip to content

perf(io): bail out on oversized images before decoding - #2750

Merged
wkentaro merged 9 commits into
wkentaro:mainfrom
tongwentao:perf/read-image-file-header-check-2418
Oct 16, 2026
Merged

wkentaro merged 9 commits into
wkentaro:mainfrom
tongwentao:perf/read-image-file-header-check-2418

Conversation

@tongwentao

@tongwentao tongwentao commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #2418. read_image_file now reads the image size from the header and raises ImageTooLargeError before decoding any pixels when main would refuse the image anyway: a side over Qt's 32767 px raster limit and a decode over QImageReader.allocationLimit(). In that case the app does not offer to raise the limit, so the decode was wasted.

 MainWindow
-  read_image_file(path)
+  read_image_file(path, max_decode_mb=allocationLimit or None)
-  read_label_file(path)
+  read_label_file(path, max_decode_mb=...)        # passes it to read_image_file

 _imread / _imread_tiff
+  _check_decodable(width, height, bytes_per_pixel, max_decode_mb)
+    if max(width, height) > 32767 and width * height * bytes_per_pixel > limit
+      raise ImageTooLargeError
   decode pixels

bytes_per_pixel is a lower bound on Qt's decoded format (1/8 for 1-bit, otherwise 1), so the check never rejects an image Qt would accept. A 40000 x 100 PNG still opens, as it does on main.

The dialog shows the existing translated "too large" message whether the user opens the image directly or through a label JSON. read_label_file wraps the error in LabelFileReadError, so _show_file_open_error unwraps __cause__.

Evidence

  • Before: on main, the 37296 x 49319 GeoTIFF from Misleading "Allowed formats" error when the decoded image exceeds Qt's allocation limit #2388 took about 36 s and peaked at 7.4 GB RSS before Qt rejected it (numbers from read_image_file eagerly decodes and re-encodes the whole image, costing ~36s and ~7.4 GB on a large GeoTIFF #2418).
    After: the PNG and TIFF tests use files whose headers claim 37296 x 49319 but contain no pixel data. They get ImageTooLargeError without decoding. The planar TIFF test fails the test if tifffile.imread is called.
  • These tests fail on main or on an earlier revision of this branch:
    test_read_image_file_rejects_oversized_png_from_header
    test_read_image_file_rejects_oversized_tiff_from_header
    test_read_image_file_rejects_oversized_planar_tiff_before_decoding  # earlier shape[:2] check missed the width
    test_read_image_file_reads_wide_image_within_decode_limit           # earlier hard 32767 gate rejected it
    test_MainWindow_rejects_oversized_image_from_header_instantly[image]
    test_MainWindow_rejects_oversized_image_from_header_instantly[label]  # untranslated without the unwrap
    test_MainWindow_opens_wide_image_within_decode_limit[image]
    test_MainWindow_opens_wide_image_within_decode_limit[label]
    
    The full suite passes (1707 passed, 7 skipped), and just lint passes.

Merge Danger

Door: two-way

Blast Radius: small

The early error only fires when the size, read from the header, means Qt would refuse to decode.

tongwentao added a commit to tongwentao/labelme that referenced this pull request Oct 10, 2026
@wkentaro
wkentaro force-pushed the perf/read-image-file-header-check-2418 branch from 89963f3 to 2a83a7d Compare October 15, 2026 14:36
wkentaro pushed a commit to tongwentao/labelme that referenced this pull request Oct 15, 2026
@wkentaro

Copy link
Copy Markdown
Owner

AI review by gpt-6-astra (Codex, high effort) at 828131bd.

recommend-merge for 828131bd.

  1. Meaningful: Yes. The base decodes and re-encodes TIFF pixels before Qt can reject the image.
  2. Worth adding: Yes. Header checks avoid that wasted work with no new dependency.
  3. Right approach: Yes. The app passes its runtime allocation limit into the shared reader, which checks both Pillow and tifffile paths before decoding.
  4. Correct: Yes for the intended behavior. All 154 focused tests pass; both new oversized-image GUI tests fail on the base, and wide-image compatibility tests pass on both.
  5. Minimal: Yes. The changes cover the reader, both app entry paths, existing error-message reuse, regression tests, and changelog.

Known caveat: oversized planar TIFFs that previously produced garbled strips can now be rejected; the existing planar decoder bug remains.
The full-suite claim and original GeoTIFF timing and memory figures were not independently verified.

@wkentaro wkentaro added the recommend-merge pr: Agent finalized and endorses it: review and merge label Oct 15, 2026
tongwentao and others added 9 commits October 16, 2026 09:37
read_image_file() eagerly decoded and re-encoded oversized images
(such as large GeoTIFFs exceeding 32767 pixels per side) into PNG/JPEG
in memory, costing tens of seconds and gigabytes of memory only to
have Qt reject the decoded image anyway.

Check image dimensions lazily from the image header (PIL image_pil.size
or tifffile IFD page shape) before decoding pixels. If either dimension
exceeds the Qt raster engine's 32767 pixel limit, immediately raise
ImageTooLargeError without decoding pixels or re-encoding.

Closes wkentaro#2418
For a planar-separate TIFF, page.shape is (samples, height, width), so
shape[:2] compared the band count and height and missed an oversized
width, falling back to the full decode. Read imagelength and imagewidth
from the first page instead.
Move the translated too-large message into one helper and use the
RASTER_MAX_SIDE constant directly instead of an alias in _app.py.
read_label_file wraps ImageTooLargeError in LabelFileReadError, so the
dialog missed the isinstance check and showed the exception's English
text. Unwrap the cause, and keep the exception text to the plain fact
now that the dialog always supplies the translated advice.
Fragments are named by PR number, and the entry now describes what a
user sees instead of the internal read_image_file function.
Qt decodes and paints images wider than 32767 pixels when they fit the
allocation limit, so rejecting every such image refused a 40000x100
PNG that opens on main. main refuses an image only when a side exceeds
32767 and the decode exceeds the allocation limit, because the app
does not offer to raise the limit in that case. Check that same
condition before decoding, using a lower bound on Qt's bytes per pixel
so the check never rejects an image Qt would accept.
The early error now covers only images over both the per-side and the
decode limits, not every image wider or taller than 32767 pixels.
_imread_tiff now decodes through TiffPageSeries.asarray instead of
tifffile.imread, so patching imread no longer proved that the early
check runs before any pixels are decoded.
@wkentaro
wkentaro force-pushed the perf/read-image-file-header-check-2418 branch from 828131b to 3b0885f Compare October 16, 2026 00:38
@wkentaro wkentaro removed the recommend-merge pr: Agent finalized and endorses it: review and merge label Oct 16, 2026
@wkentaro

Copy link
Copy Markdown
Owner

AI review by gpt-6-astra (Codex, high effort) at 3b0885f0.

recommend-merge for head 3b0885f0 against origin/main.

  1. Meaningful: Yes. The base branch decodes TIFF pixels and re-encodes the image before Qt rejects it.
  2. Worth adding: Yes. Header checks avoid that wasted work with no new dependency or rendering changes.
  3. Right approach: Yes. The app supplies Qt’s runtime limit, and both Pillow and TIFF fallback paths check dimensions before decoding.
  4. Correct: Yes. All 162 relevant tests pass; both new oversized-image GUI cases fail on base, while wide-image compatibility cases pass on both.
  5. Minimal: Yes. The diff contains the shared check, caller wiring, existing-message reuse, regression tests, and changelog entry.

The original GeoTIFF benchmark and full-suite/lint claims were not independently reproduced. The conservative estimate can still let some oversized images reach decoding before Qt rejects them.

@wkentaro wkentaro added the recommend-merge pr: Agent finalized and endorses it: review and merge label Oct 16, 2026
@wkentaro
wkentaro merged commit 2a3be32 into wkentaro:main Oct 16, 2026
22 checks passed
alfiethology pushed a commit to alfiethology/labelme that referenced this pull request Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

recommend-merge pr: Agent finalized and endorses it: review and merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

read_image_file eagerly decodes and re-encodes the whole image, costing ~36s and ~7.4 GB on a large GeoTIFF

2 participants