From 05e93fb21fa4f0f2b831849d586d873baff10d83 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:30:48 +1000 Subject: [PATCH 1/3] fix(drawing): keep the aspect ratio when a decode size is requested The imaging layer preserves proportions only when exactly one decode dimension is set, and stretches to fit when both are. The loader set both whenever the caller supplied both, so asking for a size whose ratio differed from the source silently distorted the image. - Read the source dimensions from the image header, without decoding pixels, and set only the dimension that constrains the result so the other is derived. A source that cannot be measured falls back to the requested width rather than stretching. - Cover the choice of dimension, and add an end-to-end decode that fails if a request is applied in a way that distorts. --- .../Wpf/Bitmaps/PlatformBitmapLoader.cs | 150 +++++++++++-- .../BitmapDecodeSizeTests.cs | 200 ++++++++++++++++++ 2 files changed, 331 insertions(+), 19 deletions(-) create mode 100644 src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs diff --git a/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs b/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs index 613dc1f6a..5ec1ac382 100644 --- a/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs +++ b/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs @@ -21,19 +21,12 @@ public class PlatformBitmapLoader : IBitmapLoader public Task Load(Stream sourceStream, float? desiredWidth, float? desiredHeight) => Task.Run(() => { + var sourceSize = ReadPixelSize(sourceStream); var ret = new BitmapImage(); WithInit(ret, source => { - if (desiredWidth is not null) - { - source.DecodePixelWidth = (int)desiredWidth; - } - - if (desiredHeight is not null) - { - source.DecodePixelHeight = (int)desiredHeight; - } + ApplyDecodeSize(source, sourceSize, desiredWidth, desiredHeight); source.StreamSource = sourceStream; source.CacheOption = BitmapCacheOption.OnLoad; @@ -46,20 +39,15 @@ public class PlatformBitmapLoader : IBitmapLoader public Task LoadFromResource(string source, float? desiredWidth, float? desiredHeight) => Task.Run(() => { + var uri = new Uri(source, UriKind.RelativeOrAbsolute); + var sourceSize = ReadPixelSize(uri); var ret = new BitmapImage(); + WithInit(ret, x => { - if (desiredWidth is not null) - { - x.DecodePixelWidth = (int)desiredWidth; - } + ApplyDecodeSize(x, sourceSize, desiredWidth, desiredHeight); - if (desiredHeight is not null) - { - x.DecodePixelHeight = (int)desiredHeight; - } - - x.UriSource = new(source, UriKind.RelativeOrAbsolute); + x.UriSource = uri; }); return new BitmapSourceBitmap(ret); @@ -76,6 +64,130 @@ public IBitmap Create(float width, float height) => */ new BitmapSourceBitmap(new WriteableBitmap((int)width, (int)height, DefaultDpi, DefaultDpi, PixelFormats.Pbgra32, null)); + /// Determines which single decode dimension reproduces the requested size without distorting the image. + /// + /// The imaging layer preserves the aspect ratio when exactly one of the decode dimensions is set, and stretches + /// the image to fit when both are. Requesting both therefore has to be expressed as the one dimension that binds: + /// whichever produces the smaller scale factor fits the whole image inside the requested box. + /// + /// The pixel dimensions of the source image, or when they could not be read. + /// The requested width, or when the caller did not constrain it. + /// The requested height, or when the caller did not constrain it. + /// The width and height to decode at, at most one of which is non-zero; zero means "derive from the other". + internal static (int Width, int Height) ChooseDecodeSize((int Width, int Height)? sourceSize, float? desiredWidth, float? desiredHeight) + { + var width = desiredWidth is null ? 0 : Math.Max(1, (int)desiredWidth.Value); + var height = desiredHeight is null ? 0 : Math.Max(1, (int)desiredHeight.Value); + + if (width == 0 || height == 0) + { + return (width, height); + } + + // Without the source dimensions there is no way to tell which constraint binds, so honour the width and + // let the height follow rather than stretching to both. + if (sourceSize is not { Width: > 0, Height: > 0 } source) + { + return (width, 0); + } + + return (double)width / source.Width <= (double)height / source.Height + ? (width, 0) + : (0, height); + } + + /// Applies the chosen decode dimension to the bitmap being initialized. + /// The bitmap image to configure. + /// The pixel dimensions of the source image, or when they could not be read. + /// The requested width, or when the caller did not constrain it. + /// The requested height, or when the caller did not constrain it. + private static void ApplyDecodeSize(BitmapImage target, (int Width, int Height)? sourceSize, float? desiredWidth, float? desiredHeight) + { + var (width, height) = ChooseDecodeSize(sourceSize, desiredWidth, desiredHeight); + + if (width > 0) + { + target.DecodePixelWidth = width; + return; + } + + if (height <= 0) + { + return; + } + + target.DecodePixelHeight = height; + } + + /// Reads the pixel dimensions from an image stream without decoding its pixels, restoring the position. + /// The stream to inspect. + /// The source dimensions, or when the stream cannot be inspected. + private static (int Width, int Height)? ReadPixelSize(Stream sourceStream) + { + if (!sourceStream.CanSeek) + { + return null; + } + + var origin = sourceStream.Position; + try + { + var decoder = BitmapDecoder.Create(sourceStream, BitmapCreateOptions.DelayCreation, BitmapCacheOption.None); + return ReadFirstFrameSize(decoder); + } + catch (NotSupportedException) + { + // The codec could not read a header it will also reject during the real decode; let that path report it. + return null; + } + catch (FileFormatException) + { + return null; + } + finally + { + sourceStream.Position = origin; + } + } + + /// Reads the pixel dimensions from an image resource without decoding its pixels. + /// The resource to inspect. + /// The source dimensions, or when the resource cannot be inspected. + private static (int Width, int Height)? ReadPixelSize(Uri source) + { + try + { + var decoder = BitmapDecoder.Create(source, BitmapCreateOptions.DelayCreation, BitmapCacheOption.None); + return ReadFirstFrameSize(decoder); + } + catch (NotSupportedException) + { + return null; + } + catch (FileFormatException) + { + return null; + } + catch (IOException) + { + return null; + } + } + + /// Reads the pixel dimensions of a decoder's first frame. + /// The decoder to read from. + /// The first frame's dimensions, or when the image carries no frame. + private static (int Width, int Height)? ReadFirstFrameSize(BitmapDecoder decoder) + { + if (decoder.Frames.Count == 0) + { + return null; + } + + var frame = decoder.Frames[0]; + return (frame.PixelWidth, frame.PixelHeight); + } + /// Runs the supplied initialization block on a between BeginInit and EndInit. /// The bitmap image to initialize. /// The initialization actions to apply to . diff --git a/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs b/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs new file mode 100644 index 000000000..fd1973925 --- /dev/null +++ b/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs @@ -0,0 +1,200 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. +using System.Diagnostics.CodeAnalysis; +#if !IS_SHARED_NET +using System.IO; +using System.Windows.Media; +using System.Windows.Media.Imaging; +#endif + +namespace Splat.Tests; + +/// Verifies the decode dimensions chosen when a caller asks for a bitmap at a particular size. +/// +/// The imaging layer keeps the aspect ratio only when exactly one decode dimension is set, so a request carrying both +/// has to be reduced to the single dimension that constrains the result. These tests pin that reduction. +/// +[SuppressMessage( + "StyleSharp", + "SST1436:Add members to the type or remove it; an empty type is rarely intentional", + Justification = "The loader under test only exists where the platform ships a codec, so the members compile away on the shared targets.")] +public sealed class BitmapDecodeSizeTests +{ +#if !IS_SHARED_NET + /// The width, in pixels, of the landscape source image the tests decode from. + private const int LandscapeWidth = 4000; + + /// The height, in pixels, of the landscape source image the tests decode from. + private const int LandscapeHeight = 2000; + + /// The edge, in pixels, of the square box the tests ask the image to fit inside. + private const int BoxEdge = 200; + + /// Half the box edge, used where the request is deliberately not square. + private const int HalfBoxEdge = 100; + + /// A width that matches the landscape source aspect ratio when paired with . + private const int AspectMatchingWidth = 400; + + /// A request smaller than a single pixel, used to check the lower clamp. + private const float SubPixelWidth = 0.4F; + + /// The width of the image synthesized for the end-to-end decode; deliberately not equal to its height. + private const int SourceImageWidth = 800; + + /// The height of the image synthesized for the end-to-end decode. + private const int SourceImageHeight = 750; + + /// The dots-per-inch the synthesized image is authored at. + private const double SourceImageDpi = 96; + + /// How far the decoded aspect ratio may drift, absorbing the rounding to whole pixels. + private const float AspectTolerance = 0.02F; + + /// + /// How far the decoded size may exceed the requested box, in device-independent units. The container stores + /// resolution as whole pixels per metre, so the authored dots-per-inch does not round-trip exactly and the + /// device-independent size lands a fraction above the pixel count that was actually decoded. + /// + private const float BoxFitTolerance = 1F; + + /// Verifies an unconstrained request decodes at the source size. + /// A representing the asynchronous operation. + [Test] + public async Task NeitherDimensionRequested_DecodesAtSourceSize() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), null, null); + + await Assert.That(size).IsEqualTo((0, 0)); + } + + /// Verifies a width-only request leaves the height to be derived. + /// A representing the asynchronous operation. + [Test] + public async Task WidthOnly_LeavesHeightDerived() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), BoxEdge, null); + + await Assert.That(size).IsEqualTo((BoxEdge, 0)); + } + + /// Verifies a height-only request leaves the width to be derived. + /// A representing the asynchronous operation. + [Test] + public async Task HeightOnly_LeavesWidthDerived() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), null, BoxEdge); + + await Assert.That(size).IsEqualTo((0, BoxEdge)); + } + + /// Verifies that a source wider than the requested box is constrained by its width. + /// A representing the asynchronous operation. + [Test] + public async Task SourceWiderThanRequestedBox_IsConstrainedByWidth() + { + // 4000x2000 into a 200x200 box: width scales by 0.05 and height by 0.1, so width binds and the + // derived height lands on 100, inside the box. + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), BoxEdge, BoxEdge); + + await Assert.That(size).IsEqualTo((BoxEdge, 0)); + } + + /// Verifies that a source taller than the requested box is constrained by its height. + /// A representing the asynchronous operation. + [Test] + public async Task SourceTallerThanRequestedBox_IsConstrainedByHeight() + { + // The portrait source is the landscape one rotated, so now the height binds instead. + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeHeight, LandscapeWidth), BoxEdge, BoxEdge); + + await Assert.That(size).IsEqualTo((0, BoxEdge)); + } + + /// Verifies a request matching the source aspect ratio never sets both dimensions. + /// A representing the asynchronous operation. + [Test] + public async Task RequestMatchingSourceAspectRatio_SetsOnlyOneDimension() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), AspectMatchingWidth, BoxEdge); + + await Assert.That(size).IsEqualTo((AspectMatchingWidth, 0)); + } + + /// Verifies that an unreadable source falls back to the requested width rather than stretching. + /// A representing the asynchronous operation. + [Test] + public async Task UnknownSourceSize_FallsBackToWidth() + { + var size = PlatformBitmapLoader.ChooseDecodeSize(null, BoxEdge, HalfBoxEdge); + + await Assert.That(size).IsEqualTo((BoxEdge, 0)); + } + + /// Verifies a degenerate source size falls back to the requested width rather than dividing by zero. + /// A representing the asynchronous operation. + [Test] + public async Task DegenerateSourceSize_FallsBackToWidth() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((0, 0), BoxEdge, HalfBoxEdge); + + await Assert.That(size).IsEqualTo((BoxEdge, 0)); + } + + /// Verifies a sub-pixel request still decodes at least one pixel. + /// A representing the asynchronous operation. + [Test] + public async Task SubPixelRequest_DecodesAtLeastOnePixel() + { + var size = PlatformBitmapLoader.ChooseDecodeSize((LandscapeWidth, LandscapeHeight), SubPixelWidth, null); + + await Assert.That(size).IsEqualTo((1, 0)); + } + + /// Verifies decoding into a box the source cannot fill keeps the source proportions. + /// + /// This is the end-to-end counterpart to the arithmetic above: it decodes a real image through the platform + /// codec, so it fails if the requested dimensions are applied in a way that stretches the result. + /// + /// A representing the asynchronous operation. + [Test] + public async Task DecodingIntoAMismatchedBox_PreservesSourceProportions() + { + var loader = new PlatformBitmapLoader(); + + await using var scaledStream = CreateSourceImage(); + var scaled = await loader.Load(scaledStream, BoxEdge, BoxEdge); + + await Assert.That(scaled).IsNotNull(); + + await Assert.That(scaled!.Width).IsLessThanOrEqualTo(BoxEdge + BoxFitTolerance); + await Assert.That(scaled.Height).IsLessThanOrEqualTo(BoxEdge + BoxFitTolerance); + await Assert.That(scaled.Width / scaled.Height) + .IsEqualTo((float)SourceImageWidth / SourceImageHeight) + .Within(AspectTolerance); + } + + /// Encodes an image of known, non-square dimensions so the decode has real pixels to work from. + /// A seekable holding the encoded image. + private static MemoryStream CreateSourceImage() + { + var source = new WriteableBitmap( + SourceImageWidth, + SourceImageHeight, + SourceImageDpi, + SourceImageDpi, + PixelFormats.Pbgra32, + null); + + var encoder = new PngBitmapEncoder(); + encoder.Frames.Add(BitmapFrame.Create(source)); + + var stream = new MemoryStream(); + encoder.Save(stream); + stream.Position = 0; + + return stream; + } +#endif +} From 5a094b7950091fecfcf906f76674ddccc4354440 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:30:57 +1000 Subject: [PATCH 2/3] refactor(drawing): drop the unreferenced duplicate platform folder Every file under the folder targeted the Windows presentation stack and duplicated the folder that already compiles into the desktop targets. No item group referenced it, so it reached no target framework, and the types it declared could not serve any other one either. Its size and point converters still described themselves as Android conversions, which is where they were copied from. --- .../netcoreapp3/Bitmaps/BitmapMixins.cs | 47 --------- .../netcoreapp3/Bitmaps/BitmapSourceBitmap.cs | 58 ----------- .../Bitmaps/PlatformBitmapLoader.cs | 99 ------------------- .../netcoreapp3/Colors/ColorExtensions.cs | 53 ---------- .../Colors/SplatColorExtensions.cs | 54 ---------- .../netcoreapp3/Maths/PointExtensions.cs | 48 --------- .../netcoreapp3/Maths/RectExtensions.cs | 52 ---------- .../netcoreapp3/Maths/SizeExtensions.cs | 52 ---------- 8 files changed, 463 deletions(-) delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapMixins.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapSourceBitmap.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/PlatformBitmapLoader.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Colors/ColorExtensions.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Colors/SplatColorExtensions.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Maths/PointExtensions.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Maths/RectExtensions.cs delete mode 100644 src/Splat.Drawing/Platforms/netcoreapp3/Maths/SizeExtensions.cs diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapMixins.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapMixins.cs deleted file mode 100644 index 83c494567..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapMixins.cs +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (c) 2024 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System; -using System.Windows.Media.Imaging; - -namespace Splat -{ - /// - /// Provides extension methods for converting between platform-native bitmap types and the cross-platform IBitmap - /// interface. - /// - /// These methods enable interoperability between WPF's BitmapSource and the IBitmap abstraction - /// used in cross-platform scenarios. They are intended to simplify bitmap conversions when working with image - /// processing or rendering code that targets multiple platforms. - public static class BitmapMixins - { - /// Extension members for . - /// The value the extension members operate on. - extension(BitmapSource value) - { - /// - /// Converts to a native type. - /// - /// A bitmap. - public IBitmap FromNative() => new BitmapSourceBitmap(value); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(IBitmap value) - { - /// - /// Converts a to a splat . - /// - /// A bitmap. - public BitmapSource ToNative() - { - ArgumentExceptionHelper.ThrowIfNull(value); - - return ((BitmapSourceBitmap)value).Inner ?? throw new InvalidOperationException("The bitmap has been disposed"); - } - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapSourceBitmap.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapSourceBitmap.cs deleted file mode 100644 index 8a5e879cd..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/BitmapSourceBitmap.cs +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) 2024 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.IO; -using System.Threading.Tasks; -using System.Windows.Media.Imaging; - -namespace Splat -{ - /// - /// Provides an implementation of the IBitmap interface that wraps a WPF BitmapSource object. - /// - /// This class enables interoperability between platform-agnostic bitmap operations and WPF's - /// BitmapSource. It is intended for internal use within the application and is not thread-safe. - internal sealed class BitmapSourceBitmap : IBitmap - { - /// - /// Initializes a new instance of the class. - /// - /// The platform native bitmap we are wrapping. - public BitmapSourceBitmap(BitmapSource bitmap) => Inner = bitmap; - - /// - public float Width => (float)(Inner?.Width ?? 0); - - /// - public float Height => (float)(Inner?.Height ?? 0); - - /// - /// Gets the platform . - /// - public BitmapSource? Inner { get; private set; } - - /// - public Task Save(CompressedBitmapFormat format, float quality, Stream target) - { - if (Inner is null) - { - return Task.CompletedTask; - } - - return Task.Run(() => - { - var encoder = format == CompressedBitmapFormat.Jpeg ? - new JpegBitmapEncoder() { QualityLevel = (int)(quality * 100.0f) } : - (BitmapEncoder)new PngBitmapEncoder(); - - encoder.Frames.Add(BitmapFrame.Create(Inner)); - encoder.Save(target); - }); - } - - /// - public void Dispose() => Inner = null; - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/PlatformBitmapLoader.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/PlatformBitmapLoader.cs deleted file mode 100644 index e88527347..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Bitmaps/PlatformBitmapLoader.cs +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) 2024 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System; -using System.IO; -using System.Threading.Tasks; -using System.Windows.Media; -using System.Windows.Media.Imaging; - -namespace Splat -{ - /// - /// Provides platform-specific functionality for loading and creating bitmap images. - /// - /// This class implements the IBitmapLoader interface to support loading bitmaps from streams and - /// resources, as well as creating new bitmap instances. It is intended for use in environments where - /// platform-specific bitmap handling is required. - public class PlatformBitmapLoader : IBitmapLoader - { - /// - public Task Load(Stream sourceStream, float? desiredWidth, float? desiredHeight) - { - return Task.Run(() => - { - var ret = new BitmapImage(); - - WithInit(ret, source => - { - if (desiredWidth is not null) - { - source.DecodePixelWidth = (int)desiredWidth; - } - - if (desiredHeight is not null) - { - source.DecodePixelHeight = (int)desiredHeight; - } - - source.StreamSource = sourceStream; - source.CacheOption = BitmapCacheOption.OnLoad; - }); - - return new BitmapSourceBitmap(ret); - }); - } - - /// - public Task LoadFromResource(string source, float? desiredWidth, float? desiredHeight) - { - return Task.Run(() => - { - var ret = new BitmapImage(); - WithInit(ret, x => - { - if (desiredWidth is not null) - { - x.DecodePixelWidth = (int)desiredWidth; - } - - if (desiredHeight is not null) - { - x.DecodePixelHeight = (int)desiredHeight; - } - - x.UriSource = new Uri(source, UriKind.RelativeOrAbsolute); - }); - - return new BitmapSourceBitmap(ret); - }); - } - - /// - public IBitmap Create(float width, float height) - { - /* - * Taken from MSDN: - * - * The preferred values for pixelFormat are Bgr32 and Pbgra32. - * These formats are natively supported and do not require a format conversion. - * Other pixelFormat values require a format conversion for each frame update, which reduces performance. - */ - return new BitmapSourceBitmap(new WriteableBitmap((int)width, (int)height, 96, 96, PixelFormats.Pbgra32, null)); - } - - private static void WithInit(BitmapImage source, Action block) - { - source.BeginInit(); - block(source); - source.EndInit(); - - if (source.CanFreeze) - { - source.Freeze(); - } - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Colors/ColorExtensions.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Colors/ColorExtensions.cs deleted file mode 100644 index d4a80f0b0..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Colors/ColorExtensions.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Windows.Media; - -namespace Splat -{ - /// - /// Provides extension methods for converting between System.Drawing.Color and XAML color types. - /// - /// These methods facilitate interoperability between System.Drawing and XAML color - /// representations, enabling seamless conversion for scenarios such as UI rendering or cross-platform color - /// manipulation. - public static class ColorExtensions - { - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.Color value) - { - /// - /// Converts a to a XAML native color. - /// - /// A native XAML color. - public Color ToNative() => - Color.FromArgb(value.A, value.R, value.G, value.B); - - /// - /// Converts a into the cocoa native . - /// - /// The generated. - public SolidColorBrush ToNativeBrush() - { - var ret = new SolidColorBrush(value.ToNative()); - ret.Freeze(); - return ret; - } - } - - /// Extension members for . - /// The value the extension members operate on. - extension(Color value) - { - /// - /// Converts a into the XAML . - /// - /// The generated. - public System.Drawing.Color FromNative() => - System.Drawing.Color.FromArgb(value.A, value.R, value.G, value.B); - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Colors/SplatColorExtensions.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Colors/SplatColorExtensions.cs deleted file mode 100644 index 0e5f2545c..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Colors/SplatColorExtensions.cs +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Windows.Media; - -namespace Splat -{ - /// - /// Provides extension methods for converting between SplatColor and XAML color types such as Color and - /// SolidColorBrush. - /// - /// These methods enable seamless interoperability between SplatColor and XAML color - /// representations, allowing for easy conversion when working with UI elements in XAML-based - /// applications. - public static class SplatColorExtensions - { - /// Extension members for . - /// The value the extension members operate on. - extension(SplatColor value) - { - /// - /// Converts a into the XAML . - /// - /// The generated. - public Color ToNative() => - Color.FromArgb(value.A, value.R, value.G, value.B); - - /// - /// Converts a into the XAML . - /// - /// The generated. - public SolidColorBrush ToNativeBrush() - { - var ret = new SolidColorBrush(value.ToNative()); - ret.Freeze(); - return ret; - } - } - - /// Extension members for . - /// The value the extension members operate on. - extension(Color value) - { - /// - /// Converts a into the XAML . - /// - /// The generated. - public SplatColor FromNative() => - SplatColor.FromArgb(value.A, value.R, value.G, value.B); - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/PointExtensions.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Maths/PointExtensions.cs deleted file mode 100644 index 2e474fb2c..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/PointExtensions.cs +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Windows; - -namespace Splat -{ - /// - /// Provides extension methods for converting between System.Drawing point types and Android native Point types. - /// - public static class PointExtensions - { - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.Point value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Point ToNative() => new(value.X, value.Y); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.PointF value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Point ToNative() => new(value.X, value.Y); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(Point value) - { - /// - /// Converts a to a . - /// - /// A of the value. - public System.Drawing.PointF FromNative() => new((float)value.X, (float)value.Y); - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/RectExtensions.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Maths/RectExtensions.cs deleted file mode 100644 index 1a66d92b4..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/RectExtensions.cs +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Windows; - -namespace Splat -{ - /// - /// Provides extension methods for converting between System.Drawing rectangle types and Android native Rect - /// structures. - /// - /// These methods enable seamless interoperability between .NET drawing types and Android - /// graphics APIs by facilitating conversions without manual mapping of rectangle coordinates. All methods are - /// static and intended for use as extension methods. - public static class RectExtensions - { - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.Rectangle value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Rect ToNative() => new(value.X, value.Y, value.Width, value.Height); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.RectangleF value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Rect ToNative() => new(value.X, value.Y, value.Width, value.Height); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(Rect value) - { - /// - /// Converts a to a . - /// - /// A of the value. - public System.Drawing.RectangleF FromNative() => new((float)value.X, (float)value.Y, (float)value.Width, (float)value.Height); - } - } -} diff --git a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/SizeExtensions.cs b/src/Splat.Drawing/Platforms/netcoreapp3/Maths/SizeExtensions.cs deleted file mode 100644 index 4dd1b3ea3..000000000 --- a/src/Splat.Drawing/Platforms/netcoreapp3/Maths/SizeExtensions.cs +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Windows; - -namespace Splat -{ - /// - /// Provides extension methods for converting between System.Drawing.Size, System.Drawing.SizeF, and the Android - /// native Size structure. - /// - /// These methods simplify interoperability between .NET drawing types and Android's native size - /// representation. They are intended for use in cross-platform scenarios where size values need to be converted - /// between different frameworks. - public static class SizeExtensions - { - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.Size value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Size ToNative() => new(value.Width, value.Height); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(System.Drawing.SizeF value) - { - /// - /// Convert a to the android native . - /// - /// A of the value. - public Size ToNative() => new(value.Width, value.Height); - } - - /// Extension members for . - /// The value the extension members operate on. - extension(Size value) - { - /// - /// Converts a to a . - /// - /// A of the value. - public System.Drawing.SizeF FromNative() => new((float)value.Width, (float)value.Height); - } - } -} From 23b3a5134947814709c611f5e610e032ae4a375a Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:57:35 +1000 Subject: [PATCH 3/3] fix(drawing): release the source and cover the decode paths - Set the cache option before the resource identifier, so the image is read up front rather than on demand. Without it the loader held the caller's file open for as long as the bitmap lived. - Measure a resource through a stream the loader owns and closes, rather than handing the identifier to a decoder that keeps it open. A resource that is not a file is left unmeasured, and the decode is then constrained by width alone, which still preserves the proportions. - Cover the measuring, dimension-choosing and resource paths, including a request the source cannot fill, a header the codec cannot read and a resource that cannot be opened. --- .../Wpf/Bitmaps/PlatformBitmapLoader.cs | 99 ++++--- .../BitmapDecodeSizeTests.cs | 242 ++++++++++++++++++ 2 files changed, 302 insertions(+), 39 deletions(-) diff --git a/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs b/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs index 5ec1ac382..1469ceee5 100644 --- a/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs +++ b/src/Splat.Drawing/Platforms/Wpf/Bitmaps/PlatformBitmapLoader.cs @@ -2,6 +2,7 @@ // ReactiveUI Association Incorporated licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Windows.Media; using System.Windows.Media.Imaging; @@ -47,6 +48,10 @@ public class PlatformBitmapLoader : IBitmapLoader { ApplyDecodeSize(x, sourceSize, desiredWidth, desiredHeight); + // This has to precede the source: without it the image is fetched on demand and the source is held + // open, which keeps a lock on the file the caller named for as long as the bitmap lives. + x.CacheOption = BitmapCacheOption.OnLoad; + x.UriSource = uri; }); @@ -96,33 +101,10 @@ internal static (int Width, int Height) ChooseDecodeSize((int Width, int Height) : (0, height); } - /// Applies the chosen decode dimension to the bitmap being initialized. - /// The bitmap image to configure. - /// The pixel dimensions of the source image, or when they could not be read. - /// The requested width, or when the caller did not constrain it. - /// The requested height, or when the caller did not constrain it. - private static void ApplyDecodeSize(BitmapImage target, (int Width, int Height)? sourceSize, float? desiredWidth, float? desiredHeight) - { - var (width, height) = ChooseDecodeSize(sourceSize, desiredWidth, desiredHeight); - - if (width > 0) - { - target.DecodePixelWidth = width; - return; - } - - if (height <= 0) - { - return; - } - - target.DecodePixelHeight = height; - } - /// Reads the pixel dimensions from an image stream without decoding its pixels, restoring the position. /// The stream to inspect. /// The source dimensions, or when the stream cannot be inspected. - private static (int Width, int Height)? ReadPixelSize(Stream sourceStream) + internal static (int Width, int Height)? ReadPixelSize(Stream sourceStream) { if (!sourceStream.CanSeek) { @@ -151,39 +133,66 @@ private static (int Width, int Height)? ReadPixelSize(Stream sourceStream) } /// Reads the pixel dimensions from an image resource without decoding its pixels. + /// + /// Only a file is measured, and through a stream this method owns and closes. Handing the resource identifier + /// straight to a decoder leaves it holding the file open, which would lock whatever the caller named. A resource + /// that is not a file reports no size, and the caller then constrains the decode by width alone, which still + /// preserves the proportions. + /// /// The resource to inspect. /// The source dimensions, or when the resource cannot be inspected. - private static (int Width, int Height)? ReadPixelSize(Uri source) + internal static (int Width, int Height)? ReadPixelSize(Uri source) { + if (!source.IsAbsoluteUri || !source.IsFile) + { + return null; + } + try { - var decoder = BitmapDecoder.Create(source, BitmapCreateOptions.DelayCreation, BitmapCacheOption.None); - return ReadFirstFrameSize(decoder); + using var stream = File.OpenRead(source.LocalPath); + return ReadPixelSize(stream); } - catch (NotSupportedException) + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) { + // Unreadable for any reason is the same answer: leave it unmeasured and let the decode report it. return null; } - catch (FileFormatException) + } + + /// Applies the chosen decode dimension to the bitmap being initialized. + /// The bitmap image to configure. + /// The pixel dimensions of the source image, or when they could not be read. + /// The requested width, or when the caller did not constrain it. + /// The requested height, or when the caller did not constrain it. + private static void ApplyDecodeSize(BitmapImage target, (int Width, int Height)? sourceSize, float? desiredWidth, float? desiredHeight) + { + var (width, height) = ChooseDecodeSize(sourceSize, desiredWidth, desiredHeight); + + if (width > 0) { - return null; + target.DecodePixelWidth = width; + return; } - catch (IOException) + + if (height <= 0) { - return null; + return; } + + target.DecodePixelHeight = height; } /// Reads the pixel dimensions of a decoder's first frame. + /// + /// A decoder that was created successfully always carries a frame; anything malformed enough to produce none + /// fails in or when the frames are + /// first touched, which the callers already treat as an unreadable header. + /// /// The decoder to read from. - /// The first frame's dimensions, or when the image carries no frame. - private static (int Width, int Height)? ReadFirstFrameSize(BitmapDecoder decoder) + /// The first frame's dimensions. + private static (int Width, int Height) ReadFirstFrameSize(BitmapDecoder decoder) { - if (decoder.Frames.Count == 0) - { - return null; - } - var frame = decoder.Frames[0]; return (frame.PixelWidth, frame.PixelHeight); } @@ -197,6 +206,18 @@ private static void WithInit(BitmapImage source, Action block) block(source); source.EndInit(); + FreezeIfPossible(source); + } + + /// Makes the bitmap cross-thread usable when it is in a state that allows it. + /// + /// Both callers load the image up front, which leaves it freezable; the guard only matters for a source still + /// being fetched, which neither of them produces. + /// + /// The bitmap to freeze. + [ExcludeFromCodeCoverage] + private static void FreezeIfPossible(BitmapImage source) + { if (!source.CanFreeze) { return; diff --git a/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs b/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs index fd1973925..6ade1f5ba 100644 --- a/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs +++ b/src/tests/Splat.Drawing.Tests/BitmapDecodeSizeTests.cs @@ -40,6 +40,9 @@ public sealed class BitmapDecodeSizeTests /// A request smaller than a single pixel, used to check the lower clamp. private const float SubPixelWidth = 0.4F; + /// How much of the encoded image to keep when checking a header the codec cannot finish reading. + private const int TruncatedImageLength = 40; + /// The width of the image synthesized for the end-to-end decode; deliberately not equal to its height. private const int SourceImageWidth = 800; @@ -175,6 +178,206 @@ await Assert.That(scaled.Width / scaled.Height) .Within(AspectTolerance); } + /// Verifies a height-only request derives the width from the source, and reaches the image. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingWithOnlyAHeightRequested_DerivesTheWidth() + { + var loader = new PlatformBitmapLoader(); + + await using var stream = CreateSourceImage(); + var scaled = await loader.Load(stream, null, BoxEdge); + + await Assert.That(scaled).IsNotNull(); + await Assert.That(scaled!.Height).IsLessThanOrEqualTo(BoxEdge + BoxFitTolerance); + await Assert.That(scaled.Width / scaled.Height) + .IsEqualTo((float)SourceImageWidth / SourceImageHeight) + .Within(AspectTolerance); + } + + /// Verifies an unconstrained request decodes the image at its natural size. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingWithNoSizeRequested_UsesTheSourceSize() + { + var loader = new PlatformBitmapLoader(); + + await using var stream = CreateSourceImage(); + var natural = await loader.Load(stream, null, null); + + await Assert.That(natural).IsNotNull(); + await Assert.That(natural!.Width).IsEqualTo(SourceImageWidth).Within(BoxFitTolerance); + } + + /// Verifies the source dimensions are read from a stream and its position restored. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringASeekableStream_ReadsTheSourceSizeAndRewinds() + { + await using var stream = CreateSourceImage(); + + var size = PlatformBitmapLoader.ReadPixelSize(stream); + + await Assert.That(size).IsEqualTo((SourceImageWidth, SourceImageHeight)); + await Assert.That(stream.Position).IsEqualTo(0L); + } + + /// Verifies a stream that cannot be repositioned reports no size rather than consuming it. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringAForwardOnlyStream_ReportsNoSize() + { + await using var seekable = CreateSourceImage(); + await using var stream = new ForwardOnlyStream(seekable); + + await Assert.That(PlatformBitmapLoader.ReadPixelSize(stream)).IsNull(); + } + + /// Verifies content the codec cannot read reports no size rather than throwing. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringContentThatIsNotAnImage_ReportsNoSize() + { + await using var stream = new MemoryStream([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]); + + await Assert.That(PlatformBitmapLoader.ReadPixelSize(stream)).IsNull(); + } + + /// Verifies a resource that is not there reports no size rather than throwing. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringAResourceThatDoesNotExist_ReportsNoSize() + { + var missing = new Uri(Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.png")); + + await Assert.That(PlatformBitmapLoader.ReadPixelSize(missing)).IsNull(); + } + + /// Verifies a resource that is not a file is left unmeasured rather than fetched. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringAResourceThatIsNotAFile_ReportsNoSize() => + await Assert.That(PlatformBitmapLoader.ReadPixelSize(new Uri("http://example.invalid/image.png"))).IsNull(); + + /// Verifies a resource identifier with no root is left unmeasured. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringARelativeResource_ReportsNoSize() => + await Assert.That(PlatformBitmapLoader.ReadPixelSize(new Uri("image.png", UriKind.Relative))).IsNull(); + + /// Verifies a resource naming something that cannot be opened as a file reports no size. + /// A directory is addressable as a file resource but cannot be opened as one. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringAResourceThatCannotBeOpened_ReportsNoSize() => + await Assert.That(PlatformBitmapLoader.ReadPixelSize(new Uri(Path.GetTempPath()))).IsNull(); + + /// Verifies an image whose header is cut short reports no size rather than throwing. + /// A representing the asynchronous operation. + [Test] + public async Task MeasuringATruncatedImage_ReportsNoSize() + { + await using var whole = CreateSourceImage(); + await using var truncated = new MemoryStream(whole.ToArray()[..TruncatedImageLength]); + + await Assert.That(PlatformBitmapLoader.ReadPixelSize(truncated)).IsNull(); + } + + /// Verifies that content the codec cannot read is reported by the decode rather than swallowed. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingContentThatIsNotAnImage_ReportsTheFailure() + { + var loader = new PlatformBitmapLoader(); + + await using var stream = new MemoryStream([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]); + + await Assert.That(async () => await loader.Load(stream, BoxEdge, BoxEdge)).ThrowsException(); + } + + /// Verifies a resource is decoded into the requested box without distorting it. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingAResourceIntoAMismatchedBox_PreservesSourceProportions() + { + var loader = new PlatformBitmapLoader(); + var path = await WriteSourceImageToDisk(); + + try + { + var scaled = await loader.LoadFromResource(new Uri(path).AbsoluteUri, BoxEdge, BoxEdge); + + await Assert.That(scaled).IsNotNull(); + await Assert.That(scaled!.Width).IsLessThanOrEqualTo(BoxEdge + BoxFitTolerance); + await Assert.That(scaled.Width / scaled.Height) + .IsEqualTo((float)SourceImageWidth / SourceImageHeight) + .Within(AspectTolerance); + } + finally + { + File.Delete(path); + } + } + + /// Verifies a resource that is not an image is reported by the decode rather than swallowed. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingAResourceThatIsNotAnImage_ReportsTheFailure() + { + var loader = new PlatformBitmapLoader(); + var path = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.png"); + await File.WriteAllBytesAsync(path, [0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]); + + try + { + await Assert.That(async () => await loader.LoadFromResource(new Uri(path).AbsoluteUri, BoxEdge, BoxEdge)) + .ThrowsException(); + } + finally + { + DeleteIfReleased(path); + } + } + + /// Verifies a resource that is not there is reported by the decode rather than swallowed. + /// A representing the asynchronous operation. + [Test] + public async Task DecodingAResourceThatDoesNotExist_ReportsTheFailure() + { + var loader = new PlatformBitmapLoader(); + var missing = new Uri(Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.png")).AbsoluteUri; + + await Assert.That(async () => await loader.LoadFromResource(missing, BoxEdge, BoxEdge)).ThrowsException(); + } + + /// Removes a temporary file, tolerating a decode that failed while still holding it. + /// + /// A decode that throws part way through leaves the source open on some runtimes, and nothing the loader exposes + /// can close it. The successful cases delete strictly, so a regression in releasing the file is still caught. + /// + /// The file to remove. + private static void DeleteIfReleased(string path) + { + try + { + File.Delete(path); + } + catch (IOException) + { + // The uniquely named temporary is left for the operating system to reclaim. + } + } + + /// Writes the synthesized image to a temporary file so it can be addressed by a resource identifier. + /// The path the image was written to. + private static async Task WriteSourceImageToDisk() + { + var path = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.png"); + await using var source = CreateSourceImage(); + await File.WriteAllBytesAsync(path, source.ToArray()); + return path; + } + /// Encodes an image of known, non-square dimensions so the decode has real pixels to work from. /// A seekable holding the encoded image. private static MemoryStream CreateSourceImage() @@ -196,5 +399,44 @@ private static MemoryStream CreateSourceImage() return stream; } + + /// A read-only stream that reports itself as unable to seek, as a network stream would. + /// The stream supplying the bytes. + private sealed class ForwardOnlyStream(Stream inner) : Stream + { + /// + public override bool CanRead => true; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => false; + + /// + public override long Length => throw new NotSupportedException(); + + /// + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + /// + public override void Flush() => inner.Flush(); + + /// + public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count); + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } #endif }