Skip to content

Show a preview for images too large for the browser to display (BL-16597) - #8111

Open
JohnThomson wants to merge 8 commits into
masterfrom
BL-16597LargeImagePreview
Open

Show a preview for images too large for the browser to display (BL-16597)#8111
JohnThomson wants to merge 8 commits into
masterfrom
BL-16597LargeImagePreview

Conversation

@JohnThomson

@JohnThomson JohnThomson commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Chromium — and therefore WebView2 — refuses to decode an image whose pixel count times 4 bytes/pixel overflows a signed 32-bit int (about 536.9 megapixels). The <img> fires error a moment after the bytes arrive and nothing paints, which is why the image chooser showed an empty preview pane for a 30000x23756 scan. Decoding at a reduced size doesn't help: createImageBitmap() with resizeWidth and the WebCodecs ImageDecoder with desiredWidth both fail identically, because the limit is tested against the image's natural size before any scaling is applied. And well below that hard ceiling, handing the renderer a hundred-megapixel image still costs seconds of decode and gigabytes of RAM to fill a preview pane a few hundred pixels tall.

So when a picked file is over 40 megapixels, imageGallery/localFilePreview now serves a downscaled JPEG instead of the original:

  • WIC's DecodePixelWidth/DecodePixelHeight does the scaling as part of the decode, so a 713 megapixel JPEG costs about 2.5 seconds and under 100MB rather than the ~2.6GB a full decode would need.
  • Any EXIF rotation is baked into the stand-in, since it is re-encoded without an EXIF block.
  • The stand-in is generated once per picked file and reused, because the gallery asks for the same URL as both the thumbnail and the large preview. It is deleted when another file is picked, and when the project's lifetime scope is disposed.
  • Anything WIC can't open (SVG, and other formats the browser handles fine anyway) falls back to serving the original.

imageGallery/pickLocalImageFile now also reports the original file's pixel dimensions and byte count, so the gallery describes the file the user actually chose rather than the downscaled stand-in it is being shown.

Tests cover reading dimensions (including the not-an-image fallback), the below-threshold "serve the original" case, and the downscaling itself.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16597

Devin review


This change is Reviewable

JohnThomson and others added 2 commits July 27, 2026 12:30
…597)

Chromium (and therefore WebView2) refuses to decode an image whose pixel
count times 4 bytes overflows a signed 32-bit int, so the image chooser
showed an empty preview pane for a 30000x23756 scan. Decoding at a reduced
size does not help: the limit is tested against the image's natural size
before any scaling.

So when a picked file is over 40 megapixels, the localFilePreview endpoint
now serves a downscaled JPEG instead of the original. WIC's DecodePixelWidth
does the scaling as part of the decode, so a 713 megapixel JPEG costs about
2.5 seconds and under 100MB rather than the ~2.6GB a full decode would need.
Any EXIF rotation is baked into the stand-in, since it is re-encoded without
an EXIF block. The stand-in is generated once per picked file and reused,
because the gallery asks for the same URL as both thumbnail and large
preview; it is deleted when another file is picked or when the project's
lifetime scope is disposed.

pickLocalImageFile now also reports the original file's dimensions and byte
count, so the gallery describes the file the user actually chose rather than
the downscaled stand-in it is being shown.

Tests cover reading dimensions (including the not-an-image fallback), the
below-threshold "serve the original" case, and the downscaling itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JohnThomson and others added 2 commits July 27, 2026 12:41
The image chooser accepts .tif/.tiff, but the orientation tag was only
looked for at the JPEG APP1 query path, so a rotated TIFF was treated as
upright and previewed sideways. Try the TIFF top-level IFD path as well.

Also: the comment in HandleLocalFilePreview claimed the stand-in is
"generated once and reused". That is true of the stand-in, but for an image
small enough to serve as-is there is nothing to cache, so the (cheap) header
read does repeat per request. Say what actually happens.

Both noticed by Devin (informational flags on PR #8111).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cached downscaled stand-in was keyed only on "one exists", so nothing in
HandleLocalFilePreview itself established that it belonged to the file being
requested; that was an invariant maintained at a distance, by the order in
which HandlePickLocalImageFile cleared the cache and reassigned the
authorized path. Unreachable in practice — picking a file means working a
modal dialog, and the front-end only asks for the preview URL after the pick
replies — but it is cheaper to make the cache say what it is about than to
keep the reasoning.

Remembering the source file also lets us remember "this one needs no
stand-in", so an image small enough to serve as-is is no longer re-examined
on every request.

Devin raised this on PR #8111; the suggested hardening is what is implemented
here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/web/controllers/ImageGalleryApi.cs
This is the only place in Bloom that uses WPF's WIC-backed imaging, and the
API server calls it from a thread-pool thread. That is fine as long as the
bitmaps are frozen, which MakeBrowserSafePreview does — but NUnit runs this
project's tests on an STA thread, so nothing here actually exercised the
apartment the real caller uses. Run it on an MTA thread and check the result.

Raised by Devin on PR #8111, which asked for a runtime sanity check; a test
seemed better than checking once by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/web/controllers/ImageGalleryApi.cs Outdated
Comment thread src/BloomExe/web/controllers/ImageGalleryApi.cs
Comment thread src/BloomExe/web/controllers/ImageGalleryApi.cs
@JohnThomson

Copy link
Copy Markdown
Contributor Author

(Claude Opus 5) Consulted Devin on 2026-07-27, up to commit 407e275740ee386d43d4ac416a835ea108f44e84 — four reviews in all, one per pushed commit.

What it raised, and where each landed:

  • Transparent huge images preview as a black block — real; left open above for John to decide how to encode the stand-in. The only finding still needing a person.
  • Small TIFFs still don't preview — real, and pre-existing (TIFFs have never previewed here); left open above as a scope question, since making them work is new behaviour rather than part of BL-16597.
  • Preview cache wasn't keyed to the file it was made from — fixed in e1237d202d; thread resolved.
  • WPF/WIC imaging on the server thread — checked and now covered by a test that runs it on an MTA thread, 407e275740; thread resolved.
  • Reporting the real file size might trip size gating in the gallery — checked, no such gating exists; thread resolved.
  • EXIF orientation only read from JPEGs, not TIFFs — fixed in 86057f3d1a. Devin separately verified the orientation transforms (including transpose/transverse) as correct.
  • Plus assorted informational notes; the ones worth acting on became the fixes above.

CI (pr-automation, track / set-waiting) is green. Greptile has not posted on this PR — nor on #8110, opened twenty minutes earlier by someone else — while every earlier PR today got a review within about three minutes, so it looks like Greptile is down rather than slow.

JohnThomson and others added 2 commits July 27, 2026 13:33
Two things the reviewers turned up, both decided by John.

Substituting a stand-in was keyed on size alone, but size is only one of the
two reasons a browser won't display a picked file. The chooser's filter
accepts .tif/.tiff and no browser will draw one, so a TIFF showed the same
empty pane this card is about — and, once big TIFFs started being converted
on their way past the size threshold, whether a TIFF previewed depended on
how many pixels it had, which is not explicable to anyone. Bloom already
applies the rule "convert what browsers can't draw so they can draw it" when
importing: a TIFF lands in the book folder as a PNG. This is the same rule,
applied to the preview. Matched on extension rather than on the bytes,
deliberately: ReplyWithImage takes the response's content type from the
extension, so the extension is what the browser goes on. An image already
smaller than kPreviewMaxDimension is no longer enlarged, since a stand-in
made only because of format has no reason to grow.

A stand-in is a JPEG, which has no alpha channel, so a picture with
see-through areas previewed with them solid black — whatever sits in the
colour channels under a transparent pixel. Composite onto white first. Done
unconditionally rather than only for images carrying transparency: deciding
that reliably means enumerating the pixel formats that can hold alpha and
checking indexed palettes for a transparent entry, against one pass over an
image already capped at 1600px. It is pixel arithmetic rather than the WPF
rendering stack, so it stays safe on the API server's threads.

The transparency test was confirmed to fail without the fix (corner pixel
came out 0,0,0), so it is testing what it claims to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Devin pointed out that every image fixture so far was orientation 1, so none
of the rotated paths were exercised and a regression in them would go
unnoticed. It also observed that baking in the rotation by hand is only
correct if WPF's decoder doesn't already do it — true as far as anyone knew,
but nothing established it.

Both are now settled by the same test. A JPEG fixture carrying a chosen EXIF
orientation is built by splicing a hand-written APP1 segment in after the
start-of-image marker (System.Drawing cannot construct a PropertyItem to
attach one, and pulling WPF into the test project just to write a fixture
costs more than twenty bytes of well-specified header). All eight
orientations are then checked: 5-8 must report the stored dimensions
swapped, 1-4 unswapped. Those first four would pass even if the tag were
being ignored entirely, but 5-8 cannot — and they would also fail if WPF
were auto-rotating, since the swap would then be applied twice.

Also covers an orientation value outside 1-8 being ignored rather than
reaching the rotation table.

Riding along: a "seen again" note on the existing papercut about the nine
tests build/agent-dotnet.sh cannot run, which cost time again on this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/web/controllers/ImageGalleryApi.cs
@JohnThomson

Copy link
Copy Markdown
Contributor Author

(Claude Opus 5) Consulted Devin again on 2026-07-27, up to commit d784d7f483ffab85d52afce85c85a582326d02a9 — six reviews now, one per pushed commit.

Since the last log, John made the calls on the two findings that were waiting, and both are now fixed with their threads resolved:

  • Transparent huge images previewing black — fixed in 72c2959731 by compositing onto white before the JPEG encode. The test for it was confirmed to fail without the fix.
  • TIFFs not previewing — fixed in the same commit. The deciding argument was that Bloom already converts non-web formats to PNG on import (verified by running the import path: scan.tif lands in the book folder as scan.png), so the preview pane was the one place not applying a rule Bloom already has.
  • EXIF orientation applied by hand / rotated paths untested — new this round, and now covered in d784d7f483. All eight orientations are tested against a JPEG with a hand-built EXIF header. That also settles Devin's "only correct if WPF doesn't auto-rotate" caveat empirically: if WPF were auto-rotating, the swap would be applied twice and cases 5-8 would fail.

One thing to be aware of when reading Devin's page directly: its last three jobs each re-list findings that were already fixed, quoting line numbers from before the fix — the transparency bug is still reported at ImageGalleryApi.cs:630 (new JpegBitmapEncoder), which at this commit is line 725 and is wrapped in FlattenOntoWhite. It appears to accumulate findings across the PR's life rather than re-evaluating each against the current file. Every one of them has a resolved thread here recording the fix.

CI green. Greptile still silent on this PR — as on #8110 — so it has been recorded as timed out rather than clean.

Noticed while preflighting this branch. One full-suite run showed 12
failures rather than the usual 9 environmental ones; a second run at the same
commit showed 9, and the three extras passed in isolation. None of them are
near this branch's subject. Recording it because the cost is that a run of
12 can no longer be read at a glance — you have to run the suite twice to
find out which failures are noise.

Docs only; no product code in this commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JohnThomson
JohnThomson marked this pull request as ready for review July 27, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants