fix(app): report the decode allocation limit instead of "Allowed formats" - #2417
fix(app): report the decode allocation limit instead of "Allowed formats"#2417Sanjays2402 wants to merge 2 commits into
Conversation
…ats" When a supported image decodes to more than QImageReader's allocation limit (256 MB by default), QImage.fromData returns a null image and the image-load path fell through to the generic "Allowed formats" dialog, which wrongly implies the file format is unsupported. Add _image_too_large_message(), which re-reads the header with QImageReader to get the pixel dimensions without decoding, compares the required allocation against QImageReader.allocationLimit(), and returns a message naming the dimensions and the limit. The generic unsupported-format message is still used when the data is not readable at all or fits within the limit. Adds regression tests covering the over-limit, undecodable, and within-limit cases. Closes wkentaro#2388
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Thanks for taking this on. I ran a two-axis review (repo standards + fidelity to #2388) plus the test suite and lint locally. The core approach is right, and I verified the mechanism actually works for the reported case. A few things to address before this can be recommended for merge. Verified workingI traced the concern that would have sunk this: Locally: Blocking1. The CLA is unsigned. The 2. No CHANGELOG entry. Per Spec gap worth closing3. The ~32767 px per-side raster limit is unhandled. #2388 explicitly calls this out:
Suggest adding a per-side check alongside the allocation check, with a message that names whichever ceiling was actually hit. 4. The message drops
The current text says "split the image into tiles" without naming a tool. Naming it is the difference between advice and an actionable next step. Minor5. Redundant local annotations. 6. The docstring's first line restates the function name. Sibling module-level functions in Not an issue: Follow-up filed, not blocking this PR: #2418 covers the ~36s / ~7.4 GB eager decode-and-re-encode in |
The too-large message only compared width*height*4 against the decode allocation limit, so an image whose one side exceeds the raster engine limit but whose allocation math fits fell back to the misleading "Allowed formats" text. Check the per-side ceiling first and say which limit was hit, since raising the allocation limit cannot lift it. Also name gdal_retile.py in both messages, drop the inferable local annotations and the docstring line that restated the function name, and add the CHANGELOG entry.
|
Pushed 1dc918a covering points 2-6. The per-side gap was real and you're right that it's the same failure through the other door. Also: CHANGELOG entry under
On the CLA: that's tied to my identity as a legal agreement, so I'd rather sign it deliberately than have it happen as part of an automated pass. I'll post the signature comment myself. |
Closes #2388
A supported image whose decode exceeds
QImageReader.allocationLimit()(256 MB by default) makesQImage.fromData()return a null image, which fell through to the generic "Allowed formats" dialog and wrongly implied the file format was unsupported._image_too_large_message()re-reads the header withQImageReader(no decode) to get the pixel dimensions, compares the required allocation against the limit, and returns a message naming both. The generic message is still used when the data is unreadable or fits within the limit.