diff --git a/PAPERCUTS.md b/PAPERCUTS.md index c96dceaf2749..1ca8ee317169 100644 --- a/PAPERCUTS.md +++ b/PAPERCUTS.md @@ -116,6 +116,29 @@ House rules: the exit code" hazard, different cause. +## 2026-07-27 — Three C# tests fail intermittently under load + +- **Cut:** `CheckAudioForAllText_SpansAudioMissing`, `BringBookUpToDate_MovesMetaDataToJson` and + `InsertPageAfter_FromAnotherBook_CopiesWidget(True)` fail sometimes and pass sometimes. They + failed together on one full-suite run, passed on a second run at the *identical* commit, and + passed when run on their own — so they are flaky rather than broken, and none of them is anywhere + near what that branch was changing (image handling). The cost is that a full run can no longer be + trusted on one reading: an agent has to run the suite twice to tell noise from a real regression. + That matters more now than when this was first written, because the full suite otherwise comes + back completely green (see the 2026-08-04 note below) — these three are the only remaining noise, + so any other failure is signal. +- **Idea:** Find the shared state (all three build books/collections in temp folders, so likely a + fixture or folder-name collision when the suite runs under load) — or, cheaply, quarantine them + with `[Retry]` so the noise stops masking real failures. +- **Context:** BloomDesktop, seen during `/preflight` of PR #8111 (BL-16597), on two of six + full-suite runs that day. +- **2026-08-04:** All three passed on a full run of 3027 tests with **0 failures** — the first + entirely green full suite under `build/agent-dotnet.sh`, now that PR #8107 has fixed the nine + environmental failures that used to accompany them. Not evidence against this cut: intermittent + is intermittent. Recorded because it removes the nine-failure baseline the original wording + leaned on. + + ## 2026-07-24 — agent-dotnet.sh test exits 0 even when tests fail - **Cut:** `build/agent-dotnet.sh test src/BloomTests/BloomTests.csproj` returned exit code 0 on a diff --git a/src/BloomBrowserUI/react_components/image-gallery/ImageGalleryDialog.tsx b/src/BloomBrowserUI/react_components/image-gallery/ImageGalleryDialog.tsx index f6e2ad562702..24afaf390704 100644 --- a/src/BloomBrowserUI/react_components/image-gallery/ImageGalleryDialog.tsx +++ b/src/BloomBrowserUI/react_components/image-gallery/ImageGalleryDialog.tsx @@ -101,16 +101,24 @@ const ImageGalleryDialog: React.FunctionComponent<{ {}, ); if (!response) return undefined; - const { filePath, previewUrl } = response.data as { + const { filePath, previewUrl, width, height, size } = response.data as { filePath: string; previewUrl: string; + width: number; + height: number; + size: number; }; if (!filePath) return undefined; return { thumbnailUrl: previewUrl, reasonableSizeUrl: previewUrl, localPath: filePath, - size: 0, + // C# reports the original file's dimensions and byte count. These matter because + // previewUrl may serve a downscaled stand-in for an image too large for the + // browser to display, and we want to report what the user actually chose. + width: width || undefined, + height: height || undefined, + size: size ?? 0, type: "image", }; }; diff --git a/src/BloomExe/web/controllers/ImageGalleryApi.cs b/src/BloomExe/web/controllers/ImageGalleryApi.cs index 1b5482cd8fe9..f309b60189d5 100644 --- a/src/BloomExe/web/controllers/ImageGalleryApi.cs +++ b/src/BloomExe/web/controllers/ImageGalleryApi.cs @@ -7,6 +7,7 @@ using System.Text.RegularExpressions; using System.Threading.Tasks; using System.Windows.Forms; +using System.Windows.Media.Imaging; using Bloom.Api; using Bloom.Book; using Bloom.Edit; @@ -15,6 +16,7 @@ using Bloom.Utils; using SIL.Core.ClearShare; using SIL.IO; +using SIL.Reporting; using SIL.Windows.Forms.ClearShare; using SIL.Windows.Forms.ImageToolbox; @@ -24,7 +26,7 @@ namespace Bloom.web.controllers /// Api for the image gallery (image chooser dialog) — local collections, file picker, /// remote search results, and saving chosen images into the current book. /// - public class ImageGalleryApi + public class ImageGalleryApi : IDisposable { public EditingView View { get; set; } @@ -37,6 +39,53 @@ public class ImageGalleryApi /// private string _lastPickedLocalImagePath; + /// + /// A temporary JPEG standing in for _lastPickedLocalImagePath when the browser could + /// not display the original itself (see MakeBrowserSafePreview). + /// Null when the original is served as-is. Deleted when another file is picked. + /// + private string _lastPickedLocalImagePreviewPath; + + /// + /// The file _lastPickedLocalImagePreviewPath was made from, or null if we have not yet + /// looked at any file. This is what the cache is keyed on, so that a stand-in can only + /// ever be served for the file it was made from — rather than that being an invariant + /// maintained by the order in which HandlePickLocalImageFile does things. + /// It is also how we remember that a file needs no stand-in, which is why it is set + /// even in that case: without it we would re-examine such a file on every request. + /// + private string _lastPickedLocalImagePreviewSourcePath; + + /// + /// Guards the two fields above. The gallery uses the preview URL as both the + /// thumbnail and the large preview, so two requests for it typically arrive at once; + /// without this they would each spend seconds building a preview and one would leak. + /// + private readonly object _previewLock = new object(); + + /// + /// Chromium — and therefore WebView2 — flatly refuses to decode an image whose pixel + /// count times 4 bytes/pixel overflows a signed 32-bit int, i.e. anything larger than + /// about 536.9 megapixels. The <img> fires "error" a fraction of a second after + /// the bytes arrive and nothing paints, which is why the image chooser showed an empty + /// preview for a 30000x23756 scan (BL-16597). Decoding at a reduced size does not 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. + /// + /// Well below that hard ceiling, handing the renderer a hundred-megapixel image still + /// costs it seconds of decode time and gigabytes of RAM, all to fill a preview pane a + /// few hundred pixels tall. So past this threshold we substitute a downscaled JPEG. + /// (Size is only one of the two reasons for substituting one — see NeedsStandIn.) + /// + internal const long kMaxPreviewPixels = 40L * 1000 * 1000; + + /// + /// Longest side, in pixels, of a generated preview. The gallery's preview pane caps the + /// image at 420px tall, so this leaves plenty of room for high-DPI screens. + /// + internal const int kPreviewMaxDimension = 1600; + /// /// The root folder where SIL image collections (including Art of Reading) are installed. /// @@ -59,10 +108,18 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler) HandlePickLocalImageFile, false ); + // requiresSync: false deliberately. Building a stand-in for a very large image + // decodes and re-encodes it, which takes on the order of seconds; if this ran under + // the api handler's global lock it would stall every other synchronized request for + // that whole time. Nothing here needs that lock: the only state shared between + // requests is the preview cache, and _previewLock already guards it (and still + // coalesces the gallery's near-simultaneous thumbnail and large-preview requests + // into a single build). apiHandler.RegisterEndpointHandler( "imageGallery/localFilePreview", HandleLocalFilePreview, - false + false, + requiresSync: false ); apiHandler.RegisterEndpointHandler( "imageGallery/local-collections/collections", @@ -367,12 +424,370 @@ private void HandlePickLocalImageFile(ApiRequest request) ) ); + // The previous pick's downscaled stand-in (if any) is now unreachable. Serving the + // wrong image is prevented by the cache's key, not by this; the point here is just + // to not leave the temp file lying around until the next pick. + DeleteLastPickedImagePreview(); _lastPickedLocalImagePath = selectedPath; + var previewUrl = string.IsNullOrEmpty(selectedPath) ? "" : "/bloom/api/imageGallery/localFilePreview?path=" + Uri.EscapeDataString(selectedPath); - request.ReplyWithJson(new { filePath = selectedPath, previewUrl }); + + // Report the *original* file's dimensions and byte count. The gallery displays + // these, and without them it would fall back to measuring whatever the preview + // turns out to be — which for a huge image is the downscaled stand-in, not the + // file the user actually chose. + var (width, height) = GetImageDimensions(selectedPath); + long size = 0; + if (!string.IsNullOrEmpty(selectedPath) && RobustFile.Exists(selectedPath)) + { + try + { + size = new FileInfo(selectedPath).Length; + } + catch (Exception e) + { + Logger.WriteMinorEvent( + $"ImageGalleryApi could not read the size of {selectedPath}: {e.Message}" + ); + } + } + + request.ReplyWithJson( + new + { + filePath = selectedPath, + previewUrl, + width, + height, + size, + } + ); + } + + /// + /// Reads an image's pixel dimensions without decoding its pixels. Returns (0,0) if the + /// dimensions can't be determined (e.g. an SVG, or a format WIC doesn't know). + /// + internal static (int width, int height) GetImageDimensions(string path) + { + if (string.IsNullOrEmpty(path) || !RobustFile.Exists(path)) + return (0, 0); + try + { + var header = ReadImageHeader(path); + // Report the dimensions as they will actually be seen, i.e. after the viewer + // applies any EXIF rotation. + return IsQuarterTurned(header.exifOrientation) + ? (header.height, header.width) + : (header.width, header.height); + } + catch (Exception e) + { + Logger.WriteMinorEvent( + $"ImageGalleryApi could not read the dimensions of {path}: {e.Message}" + ); + return (0, 0); + } + } + + /// + /// Reads an image's dimensions and EXIF orientation without decoding its pixels. WIC + /// parses the header up front but reads everything else lazily, so all the values we + /// want must be pulled out while the stream is still open — hence one method rather + /// than handing a BitmapFrame back to the caller. + /// + private static (int width, int height, int exifOrientation) ReadImageHeader(string path) + { + using var stream = RobustFile.OpenRead(path); + var decoder = BitmapDecoder.Create( + stream, + BitmapCreateOptions.IgnoreColorProfile | BitmapCreateOptions.DelayCreation, + BitmapCacheOption.None + ); + var frame = decoder.Frames[0]; + return ( + frame.PixelWidth, + frame.PixelHeight, + ReadExifOrientation(frame.Metadata as BitmapMetadata) + ); + } + + /// + /// The EXIF orientation recorded in the given metadata, or 1 ("upright") if there is + /// none. The chooser accepts TIFFs as well as JPEGs, and the tag lives at a different + /// query path in each, so both are tried. + /// + private static int ReadExifOrientation(BitmapMetadata metadata) + { + if (metadata == null) + return 1; + // 274 (0x112) is the EXIF "Orientation" tag. In a JPEG it hangs off the APP1 + // marker segment; in a TIFF it is in the top-level IFD. + foreach (var query in new[] { "/app1/ifd/{ushort=274}", "/ifd/{ushort=274}" }) + { + try + { + if ( + metadata.GetQuery(query) is ushort orientation + && orientation >= 1 + && orientation <= 8 + ) + return orientation; + } + catch (Exception) + { + // Plenty of images carry no EXIF block at all, and asking a decoder for a + // query path its format doesn't have is how we find that out. + } + } + return 1; + } + + /// Whether the given EXIF orientation swaps width and height. + private static bool IsQuarterTurned(int exifOrientation) => exifOrientation >= 5; + + /// + /// The rotation/flip an EXIF orientation calls for, or null when none is needed. + /// + private static System.Windows.Media.Transform GetOrientationTransform(int exifOrientation) + { + switch (exifOrientation) + { + case 2: + return new System.Windows.Media.ScaleTransform(-1, 1); + case 3: + return new System.Windows.Media.RotateTransform(180); + case 4: + return new System.Windows.Media.ScaleTransform(1, -1); + case 5: + var transpose = new System.Windows.Media.TransformGroup(); + transpose.Children.Add(new System.Windows.Media.RotateTransform(90)); + transpose.Children.Add(new System.Windows.Media.ScaleTransform(-1, 1)); + return transpose; + case 6: + return new System.Windows.Media.RotateTransform(90); + case 7: + var transverse = new System.Windows.Media.TransformGroup(); + transverse.Children.Add(new System.Windows.Media.RotateTransform(270)); + transverse.Children.Add(new System.Windows.Media.ScaleTransform(-1, 1)); + return transverse; + case 8: + return new System.Windows.Media.RotateTransform(270); + default: + return null; + } + } + + /// + /// Lays the image over a white background, so that anything see-through in it becomes + /// white rather than whatever happens to be in the RGB channels underneath. + /// + /// A stand-in is encoded as JPEG, which has no alpha channel: without this, a large + /// image with transparent areas — a line-art scan on transparency, say — previews with + /// those areas solid black, because that is what is stored under a fully transparent + /// pixel. White is what the chooser and the page behind it both use. + /// + /// Done unconditionally rather than only for images that carry transparency, because + /// deciding that reliably means enumerating the pixel formats that can hold an alpha + /// channel *and* checking indexed palettes for a transparent entry, whereas this is one + /// pass over an image already capped at kPreviewMaxDimension — milliseconds against the + /// seconds the decode itself takes. Pure pixel arithmetic, so unlike the WPF rendering + /// stack it is safe on whatever thread the API server hands us. + /// + private static BitmapSource FlattenOntoWhite(BitmapSource source) + { + // Bgra32 is straight (non-premultiplied) alpha, which is what the blend below + // assumes; Pbgra32 would already have the colour scaled by the alpha. + var bgra = new FormatConvertedBitmap( + source, + System.Windows.Media.PixelFormats.Bgra32, + null, + 0 + ); + int width = bgra.PixelWidth, + height = bgra.PixelHeight; + int stride = width * 4; + var pixels = new byte[(long)height * stride]; + bgra.CopyPixels(pixels, stride, 0); + for (int i = 0; i < pixels.Length; i += 4) + { + byte alpha = pixels[i + 3]; + if (alpha == 255) + continue; + // result = colour*alpha + white*(1-alpha), in 0..255 integer arithmetic. + int inverse = 255 - alpha; + pixels[i] = (byte)((pixels[i] * alpha + 255 * inverse) / 255); + pixels[i + 1] = (byte)((pixels[i + 1] * alpha + 255 * inverse) / 255); + pixels[i + 2] = (byte)((pixels[i + 2] * alpha + 255 * inverse) / 255); + pixels[i + 3] = 255; + } + var flattened = BitmapSource.Create( + width, + height, + 96, + 96, + System.Windows.Media.PixelFormats.Bgra32, + null, + pixels, + stride + ); + flattened.Freeze(); + return flattened; + } + + /// + /// File types the browser can put in an <img>. The chooser also offers .tif/.tiff, + /// which it cannot, so those get a stand-in (see NeedsStandIn). Matched on extension + /// rather than on the actual bytes deliberately: ReplyWithImage derives the response's + /// content type from the extension, so the extension is what the browser goes on. + /// + private static readonly HashSet s_browserRenderableExtensions = new HashSet( + StringComparer.OrdinalIgnoreCase + ) + { + ".jpg", + ".jpeg", + ".png", + ".gif", + ".bmp", + ".svg", + ".webp", + }; + + /// + /// Whether the browser would fail to display this file as it stands, so that we should + /// serve it a re-encoded stand-in instead. Two separate reasons: the file is a format + /// the browser cannot draw at all, or it is so large the browser refuses to decode it + /// (see kMaxPreviewPixels). + /// + private static bool NeedsStandIn(string path, int width, int height) + { + if (!s_browserRenderableExtensions.Contains(Path.GetExtension(path))) + return true; + return (long)width * height > kMaxPreviewPixels; + } + + /// + /// If the browser could not display as it stands — too + /// many pixels, or a format it cannot draw — writes a JPEG stand-in to a temp file and + /// returns its path. Otherwise returns null, meaning "serve the original". + /// + /// WIC's DecodePixelWidth/Height 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. + /// + internal static string MakeBrowserSafePreview(string originalPath) + { + try + { + var header = ReadImageHeader(originalPath); + if (!NeedsStandIn(originalPath, header.width, header.height)) + return null; + + // Constrain the longer side; WIC preserves the aspect ratio when only one of + // DecodePixelWidth/DecodePixelHeight is set, and does the scaling as part of + // the decode rather than decoding full size and shrinking afterwards. An image + // that is already smaller than that is left at its own size — a stand-in made + // only because of its format has no reason to be enlarged. + // A stream rather than a UriSource, because Uri would treat a "#" in a + // perfectly legal filename as the start of a fragment. CacheOption.OnLoad + // makes EndInit() do the decoding, so the stream can be closed right after. + var scaled = new BitmapImage(); + using (var stream = RobustFile.OpenRead(originalPath)) + { + scaled.BeginInit(); + scaled.StreamSource = stream; + scaled.CacheOption = BitmapCacheOption.OnLoad; + scaled.CreateOptions = BitmapCreateOptions.IgnoreColorProfile; + if (Math.Max(header.width, header.height) > kPreviewMaxDimension) + { + if (header.width >= header.height) + scaled.DecodePixelWidth = kPreviewMaxDimension; + else + scaled.DecodePixelHeight = kPreviewMaxDimension; + } + scaled.EndInit(); + } + scaled.Freeze(); + + // Bake in any EXIF rotation. We re-encode without an EXIF block, so a viewer + // has no orientation tag to apply and the pixels must already be upright. + BitmapSource upright = scaled; + var transform = GetOrientationTransform(header.exifOrientation); + if (transform != null) + { + var rotated = new TransformedBitmap(scaled, transform); + rotated.Freeze(); + upright = rotated; + } + + // A plain temp path rather than SIL's TempFile, whose Dispose would delete the + // file we are about to start serving. DeleteLastPickedImagePreview cleans up. + var previewPath = Path.Combine( + Path.GetTempPath(), + "BloomImagePreview-" + Guid.NewGuid() + ".jpg" + ); + var encoder = new JpegBitmapEncoder { QualityLevel = 85 }; + encoder.Frames.Add(BitmapFrame.Create(FlattenOntoWhite(upright))); + using (var output = RobustFile.Create(previewPath)) + encoder.Save(output); + + Logger.WriteMinorEvent( + $"ImageGalleryApi made a {upright.PixelWidth}x{upright.PixelHeight} preview" + + $" for {Path.GetFileName(originalPath)}" + + $" ({header.width}x{header.height}, which the browser could not display)" + ); + return previewPath; + } + catch (Exception e) + { + // Includes SVG and anything else WIC can't open. Falling back to the original + // is right: those are the formats the browser handles fine anyway. + Logger.WriteMinorEvent( + $"ImageGalleryApi could not make a downscaled preview for {originalPath}: {e.Message}" + ); + return null; + } + } + + /// + /// Empties the preview cache, deleting the downscaled stand-in if there is one. + /// + private void DeleteLastPickedImagePreview() + { + lock (_previewLock) + { + if (!string.IsNullOrEmpty(_lastPickedLocalImagePreviewPath)) + { + try + { + RobustFile.Delete(_lastPickedLocalImagePreviewPath); + } + catch (Exception e) + { + // Nothing to do about it; it's a temp file, and it is named + // recognisably enough to be swept up later. + Logger.WriteMinorEvent( + $"ImageGalleryApi could not delete {_lastPickedLocalImagePreviewPath}: {e.Message}" + ); + } + } + _lastPickedLocalImagePreviewPath = null; + _lastPickedLocalImagePreviewSourcePath = null; + } + } + + /// + /// Picking a new file removes the previous file's stand-in, so at most one is ever + /// left over — the one for the last file picked. This is where that one goes; Autofac + /// disposes us with the project's lifetime scope. + /// + public void Dispose() + { + DeleteLastPickedImagePreview(); } /// @@ -397,7 +812,30 @@ private void HandleLocalFilePreview(ApiRequest request) return; } - request.ReplyWithImage(fullPath); + // Images past a certain size don't render in the browser at all, so substitute a + // downscaled copy (BL-16597). What we worked out about a file is remembered and + // reused, since the gallery asks for this URL as both the thumbnail and the large + // preview — including the "this one is fine as it is" answer, which costs a header + // read to reach. + string pathToServe; + lock (_previewLock) + { + var answerIsAboutThisFile = + _lastPickedLocalImagePreviewSourcePath == fullPath + && ( + _lastPickedLocalImagePreviewPath == null + || RobustFile.Exists(_lastPickedLocalImagePreviewPath) + ); + if (!answerIsAboutThisFile) + { + DeleteLastPickedImagePreview(); + _lastPickedLocalImagePreviewPath = MakeBrowserSafePreview(fullPath); + _lastPickedLocalImagePreviewSourcePath = fullPath; + } + pathToServe = _lastPickedLocalImagePreviewPath ?? fullPath; + } + + request.ReplyWithImage(pathToServe); } /// diff --git a/src/BloomTests/web/controllers/ImageGalleryApiTests.cs b/src/BloomTests/web/controllers/ImageGalleryApiTests.cs index 0298bb259021..91dd0f7302b8 100644 --- a/src/BloomTests/web/controllers/ImageGalleryApiTests.cs +++ b/src/BloomTests/web/controllers/ImageGalleryApiTests.cs @@ -1,4 +1,8 @@ +using System; +using System.Drawing; +using System.Drawing.Imaging; using System.IO; +using System.Threading; using Bloom.web.controllers; using NUnit.Framework; @@ -88,5 +92,410 @@ public void HandlesMultiWordGrantor() Assert.That(credits, Is.EqualTo("Acme Publishing House")); } } + + /// + /// Covers the machinery behind BL-16597: reporting the chosen file's real dimensions, + /// and substituting a downscaled JPEG for an image the browser cannot display. + /// + [TestFixture] + public class LargeImagePreviewTests + { + private string _tempFolder; + + [SetUp] + public void Setup() + { + _tempFolder = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(_tempFolder); + } + + [TearDown] + public void TearDown() + { + Directory.Delete(_tempFolder, recursive: true); + } + + /// + /// Writes a blank PNG of the given pixel size. 1-bit-per-pixel keeps even a + /// deliberately enormous test image down to a few megabytes of memory and a + /// trivially small file, so the over-the-threshold cases are cheap to run. + /// + private string WritePng(string name, int width, int height) + { + var path = Path.Combine(_tempFolder, name); + using (var bitmap = new Bitmap(width, height, PixelFormat.Format1bppIndexed)) + bitmap.Save(path, ImageFormat.Png); + return path; + } + + /// + /// Writes a JPEG carrying the given EXIF orientation, by splicing a hand-built APP1 + /// segment in directly after the start-of-image marker. Done by hand because + /// System.Drawing cannot construct a PropertyItem to attach, and pulling WPF into + /// this test project just to write a fixture would be a heavier price than 20 bytes + /// of well-specified header. + /// + private string WriteJpegWithExifOrientation( + string name, + int width, + int height, + ushort orientation + ) + { + var path = Path.Combine(_tempFolder, name); + byte[] jpeg; + using (var bitmap = new Bitmap(width, height, PixelFormat.Format24bppRgb)) + using (var memory = new MemoryStream()) + { + using (var g = Graphics.FromImage(bitmap)) + g.Clear(Color.CornflowerBlue); + bitmap.Save(memory, ImageFormat.Jpeg); + jpeg = memory.ToArray(); + } + Assert.That( + jpeg[0] == 0xFF && jpeg[1] == 0xD8, + Is.True, + "Test setup: expected the file to start with a JPEG start-of-image marker" + ); + + // APP1 holding a minimal little-endian TIFF header whose only IFD entry is + // tag 274 (0x0112, Orientation), type 3 (SHORT), one value. + var app1 = new byte[] + { + 0xFF, + 0xE1, // APP1 marker + 0x00, + 0x22, // segment length (34), including these two bytes + 0x45, + 0x78, + 0x69, + 0x66, + 0x00, + 0x00, // "Exif\0\0" + 0x49, + 0x49, + 0x2A, + 0x00, // little-endian TIFF header + 0x08, + 0x00, + 0x00, + 0x00, // offset of IFD0 from the TIFF header + 0x01, + 0x00, // one directory entry + 0x12, + 0x01, // tag 0x0112 = Orientation + 0x03, + 0x00, // type 3 = SHORT + 0x01, + 0x00, + 0x00, + 0x00, // one value + (byte)(orientation & 0xFF), + (byte)(orientation >> 8), + 0x00, + 0x00, // the value, in the entry itself + 0x00, + 0x00, + 0x00, + 0x00, // no next IFD + }; + + using (var file = File.Create(path)) + { + file.Write(jpeg, 0, 2); // the start-of-image marker + file.Write(app1, 0, app1.Length); + file.Write(jpeg, 2, jpeg.Length - 2); // the rest of the original + } + return path; + } + + /// + /// Writes a TIFF, optionally with see-through areas. TIFF is the format the browser + /// cannot draw, so these are the fixtures for the stand-in-because-of-format path. + /// + private string WriteTiff(string name, int width, int height, bool transparent) + { + var path = Path.Combine(_tempFolder, name); + using (var bitmap = new Bitmap(width, height, PixelFormat.Format32bppArgb)) + { + using (var g = Graphics.FromImage(bitmap)) + g.Clear(transparent ? Color.Transparent : Color.CornflowerBlue); + bitmap.Save(path, ImageFormat.Tiff); + } + return path; + } + + [Test] + public void GetImageDimensions_ReturnsThePixelDimensions() + { + var path = WritePng("small.png", 640, 480); + + var (width, height) = ImageGalleryApi.GetImageDimensions(path); + + Assert.That(width, Is.EqualTo(640)); + Assert.That(height, Is.EqualTo(480)); + } + + [Test] + public void GetImageDimensions_ReturnsZeros_ForMissingFile() + { + var path = Path.Combine(_tempFolder, "nothing-here.png"); + Assert.That(File.Exists(path), Is.False, "Test setup: the file must not exist"); + + Assert.That(ImageGalleryApi.GetImageDimensions(path), Is.EqualTo((0, 0))); + } + + [Test] + public void GetImageDimensions_ReturnsZeros_ForSomethingThatIsNotAnImage() + { + // An SVG is the realistic case: the chooser allows it, but WIC cannot read it. + var path = Path.Combine(_tempFolder, "drawing.svg"); + File.WriteAllText(path, ""); + + Assert.That(ImageGalleryApi.GetImageDimensions(path), Is.EqualTo((0, 0))); + } + + [Test] + public void MakeBrowserSafePreview_ReturnsNull_WhenTheBrowserCanCopeWithTheOriginal() + { + var path = WritePng("modest.png", 2000, 1500); + Assert.That( + 2000L * 1500, + Is.LessThan(ImageGalleryApi.kMaxPreviewPixels), + "Test setup: this image is supposed to be under the threshold" + ); + + // Null means "serve the original". + Assert.That(ImageGalleryApi.MakeBrowserSafePreview(path), Is.Null); + } + + [Test] + public void MakeBrowserSafePreview_DownscalesAnImageTooBigForTheBrowser() + { + const int originalWidth = 8000; + const int originalHeight = 6000; + Assert.That( + (long)originalWidth * originalHeight, + Is.GreaterThan(ImageGalleryApi.kMaxPreviewPixels), + "Test setup: this image is supposed to be over the threshold" + ); + var path = WritePng("enormous.png", originalWidth, originalHeight); + + var previewPath = ImageGalleryApi.MakeBrowserSafePreview(path); + + Assert.That(previewPath, Is.Not.Null, "Should have made a downscaled stand-in"); + try + { + Assert.That(File.Exists(previewPath), Is.True); + using var preview = Image.FromFile(previewPath); + // The longer side is constrained; the aspect ratio is preserved. + Assert.That(preview.Width, Is.EqualTo(ImageGalleryApi.kPreviewMaxDimension)); + Assert.That( + preview.Height, + Is.EqualTo( + ImageGalleryApi.kPreviewMaxDimension * originalHeight / originalWidth + ) + ); + } + finally + { + File.Delete(previewPath); + } + } + + // The eight EXIF orientations. 5-8 are the ones that stand the image on its side, + // so for those the dimensions the viewer ends up seeing are the stored ones swapped. + [TestCase((ushort)1, 400, 200, TestName = "Orientation1_Upright")] + [TestCase((ushort)2, 400, 200, TestName = "Orientation2_MirroredHorizontally")] + [TestCase((ushort)3, 400, 200, TestName = "Orientation3_UpsideDown")] + [TestCase((ushort)4, 400, 200, TestName = "Orientation4_MirroredVertically")] + [TestCase((ushort)5, 200, 400, TestName = "Orientation5_Transposed")] + [TestCase((ushort)6, 200, 400, TestName = "Orientation6_QuarterTurnedClockwise")] + [TestCase((ushort)7, 200, 400, TestName = "Orientation7_Transverse")] + [TestCase((ushort)8, 200, 400, TestName = "Orientation8_QuarterTurnedAnticlockwise")] + public void GetImageDimensions_AllowsForExifRotation( + ushort orientation, + int expectedWidth, + int expectedHeight + ) + { + // Stored 400x200 every time; only the orientation tag differs. + var path = WriteJpegWithExifOrientation( + $"rotated{orientation}.jpg", + 400, + 200, + orientation + ); + + var (width, height) = ImageGalleryApi.GetImageDimensions(path); + + Assert.That(width, Is.EqualTo(expectedWidth)); + Assert.That(height, Is.EqualTo(expectedHeight)); + } + + [Test] + public void GetImageDimensions_TreatsAnOutOfRangeOrientationAsUpright() + { + // 9 is not a real EXIF orientation; the reader should ignore it rather than + // feed it to the rotation table. + var path = WriteJpegWithExifOrientation("nonsense.jpg", 400, 200, 9); + + Assert.That(ImageGalleryApi.GetImageDimensions(path), Is.EqualTo((400, 200))); + } + + [Test] + public void MakeBrowserSafePreview_MakesAStandInForATiff_EvenASmallOne() + { + // No browser will draw a TIFF, so size is not the only reason to substitute one. + const int width = 300; + const int height = 200; + Assert.That( + (long)width * height, + Is.LessThan(ImageGalleryApi.kMaxPreviewPixels), + "Test setup: this one is meant to be under the size threshold, so that only " + + "its format can be the reason for a stand-in" + ); + var path = WriteTiff("scan.tif", width, height, transparent: false); + + var previewPath = ImageGalleryApi.MakeBrowserSafePreview(path); + + Assert.That(previewPath, Is.Not.Null, "A TIFF always needs a stand-in"); + try + { + using var preview = Image.FromFile(previewPath); + Assert.That( + preview.RawFormat.Guid, + Is.EqualTo(ImageFormat.Jpeg.Guid), + "The stand-in has to be something the browser can draw" + ); + // Small enough already; there is no reason to enlarge it. + Assert.That(preview.Width, Is.EqualTo(width)); + Assert.That(preview.Height, Is.EqualTo(height)); + } + finally + { + File.Delete(previewPath); + } + } + + [Test] + public void MakeBrowserSafePreview_ShowsSeeThroughAreasAsWhite_NotBlack() + { + // JPEG has no alpha channel, so without compositing first, a fully transparent + // pixel encodes as whatever is in its colour channels — normally black. + var path = WriteTiff("transparent.tif", 120, 90, transparent: true); + + var previewPath = ImageGalleryApi.MakeBrowserSafePreview(path); + + Assert.That(previewPath, Is.Not.Null, "Test setup: expected a stand-in for a TIFF"); + try + { + using var preview = new Bitmap(previewPath); + var corner = preview.GetPixel(0, 0); + var middle = preview.GetPixel(preview.Width / 2, preview.Height / 2); + // JPEG is lossy, so allow a little drift rather than demanding pure 255s. + Assert.That( + corner.R + corner.G + corner.B, + Is.GreaterThan(720), + $"Transparent areas should come out white; the corner was {corner}" + ); + Assert.That( + middle.R + middle.G + middle.B, + Is.GreaterThan(720), + $"Transparent areas should come out white; the middle was {middle}" + ); + } + finally + { + File.Delete(previewPath); + } + } + + [Test] + public void MakeBrowserSafePreview_KeepsTheColoursOfAnOpaqueImage() + { + // Sanity check on the compositing above: it must not wash out a solid image. + var path = WriteTiff("solid.tif", 120, 90, transparent: false); + + var previewPath = ImageGalleryApi.MakeBrowserSafePreview(path); + + Assert.That(previewPath, Is.Not.Null, "Test setup: expected a stand-in for a TIFF"); + try + { + using var preview = new Bitmap(previewPath); + var middle = preview.GetPixel(preview.Width / 2, preview.Height / 2); + var expected = Color.CornflowerBlue; + Assert.That( + Math.Abs(middle.R - expected.R) + + Math.Abs(middle.G - expected.G) + + Math.Abs(middle.B - expected.B), + Is.LessThan(30), + $"Expected roughly {expected} but the stand-in had {middle}" + ); + } + finally + { + File.Delete(previewPath); + } + } + + [Test] + public void MakeBrowserSafePreview_WorksOnAnMtaBackgroundThread() + { + // 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 — not the STA thread NUnit + // runs tests on (see the Apartment attribute in BloomTests.csproj). WPF + // imaging is fine off the UI thread as long as the bitmaps are frozen, which + // MakeBrowserSafePreview does; this pins that down where it would otherwise + // only be true by inspection. + var path = WritePng("enormous-mta.png", 8000, 6000); + string previewPath = null; + Exception failure = null; + var thread = new Thread(() => + { + try + { + previewPath = ImageGalleryApi.MakeBrowserSafePreview(path); + } + catch (Exception e) + { + failure = e; + } + }); + thread.SetApartmentState(ApartmentState.MTA); + thread.Start(); + + Assert.That( + thread.Join(TimeSpan.FromMinutes(1)), + Is.True, + "Preview generation did not finish" + ); + Assert.That(failure, Is.Null, $"Preview generation threw: {failure}"); + Assert.That(previewPath, Is.Not.Null, "Should have made a downscaled stand-in"); + try + { + using var preview = Image.FromFile(previewPath); + Assert.That( + preview.Width, + Is.EqualTo(ImageGalleryApi.kPreviewMaxDimension), + "The stand-in made off the UI thread should be scaled like any other" + ); + } + finally + { + File.Delete(previewPath); + } + } + + [Test] + public void MakeBrowserSafePreview_ReturnsNull_ForAFormatItCannotRead() + { + var path = Path.Combine(_tempFolder, "drawing.svg"); + File.WriteAllText(path, ""); + + // Falling back to the original is right: the browser handles SVG fine. + Assert.That(ImageGalleryApi.MakeBrowserSafePreview(path), Is.Null); + } + } } }