diff --git a/src/Magick.NET/Helpers/TemporaryImageAttacher.cs b/src/Magick.NET/Helpers/TemporaryImageAttacher.cs index 1a17e88a31..8fa07dacd7 100644 --- a/src/Magick.NET/Helpers/TemporaryImageAttacher.cs +++ b/src/Magick.NET/Helpers/TemporaryImageAttacher.cs @@ -22,6 +22,9 @@ internal sealed class TemporaryImageAttacher : IDisposable public TemporaryImageAttacher(List> images) { + if (images.Count == 0) + throw new InvalidOperationException("Operation requires at least one image."); + _images = images; AttachImages(); } diff --git a/src/Magick.NET/MagickImageCollection.cs b/src/Magick.NET/MagickImageCollection.cs index 0b65e66470..0ad67eaf2a 100644 --- a/src/Magick.NET/MagickImageCollection.cs +++ b/src/Magick.NET/MagickImageCollection.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.Globalization; using System.IO; -using System.Runtime.InteropServices.ComTypes; using System.Threading; using System.Threading.Tasks; @@ -354,18 +353,9 @@ public void AddRange(Stream stream, IMagickReadSettings? readSettin /// Thrown when an error is raised by ImageMagick. public IMagickImage AppendHorizontally() { - ThrowIfEmpty(); - - try - { - AttachImages(); - var image = _nativeInstance.Append(_images[0], false); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Append(_images[0], false); + return MagickImage.Create(image, GetSettings()); } /// @@ -375,18 +365,9 @@ public IMagickImage AppendHorizontally() /// Thrown when an error is raised by ImageMagick. public IMagickImage AppendVertically() { - ThrowIfEmpty(); - - try - { - AttachImages(); - var image = _nativeInstance.Append(_images[0], true); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Append(_images[0], true); + return MagickImage.Create(image, GetSettings()); } /// @@ -396,24 +377,9 @@ public IMagickImage AppendVertically() /// Thrown when an error is raised by ImageMagick. public void Coalesce() { - ThrowIfEmpty(); - - var settings = GetSettings().Clone(); - - IntPtr images; - try - { - AttachImages(); - images = _nativeInstance.Coalesce(_images[0]); - } - finally - { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - Add(image); + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.Coalesce(_images[0]); + ReplaceImages(images); } /// @@ -462,18 +428,9 @@ public IMagickImage Combine() /// Thrown when an error is raised by ImageMagick. public IMagickImage Combine(ColorSpace colorSpace) { - ThrowIfEmpty(); - - try - { - AttachImages(); - var image = _nativeInstance.Combine(_images[0], colorSpace); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Combine(_images[0], colorSpace); + return MagickImage.Create(image, GetSettings()); } /// @@ -484,32 +441,19 @@ public IMagickImage Combine(ColorSpace colorSpace) public void Complex(IComplexSettings complexSettings) { Throw.IfNull(nameof(complexSettings), complexSettings); - ThrowIfEmpty(); - var settings = GetSettings().Clone(); + using var imageAttacher = new TemporaryImageAttacher(_images); - IntPtr images; - try - { - AttachImages(); + if (complexSettings.SignalToNoiseRatio is not null) + _images[0].SetArtifact("complex:snr", complexSettings.SignalToNoiseRatio.Value.ToString(CultureInfo.InvariantCulture)); - if (complexSettings.SignalToNoiseRatio is not null) - _images[0].SetArtifact("complex:snr", complexSettings.SignalToNoiseRatio.Value.ToString(CultureInfo.InvariantCulture)); + var images = _nativeInstance.Complex(_images[0], complexSettings.ComplexOperator); + ReplaceImages(images); - images = _nativeInstance.Complex(_images[0], complexSettings.ComplexOperator); - } - finally + if (complexSettings.SignalToNoiseRatio is not null) { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - { - if (complexSettings.SignalToNoiseRatio is not null) + foreach (var image in _images) image.RemoveArtifact("complex:snr"); - - Add(image); } } @@ -551,24 +495,9 @@ public void CopyTo(IMagickImage[] array, int arrayIndex) /// Thrown when an error is raised by ImageMagick. public void Deconstruct() { - ThrowIfEmpty(); - - var settings = GetSettings().Clone(); - - IntPtr images; - try - { - AttachImages(); - images = _nativeInstance.Deconstruct(_images[0]); - } - finally - { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - Add(image); + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.Deconstruct(_images[0]); + ReplaceImages(images); } /// @@ -589,18 +518,9 @@ public void Dispose() /// Thrown when an error is raised by ImageMagick. public IMagickImage Evaluate(EvaluateOperator evaluateOperator) { - ThrowIfEmpty(); - - try - { - AttachImages(); - var image = _nativeInstance.Evaluate(_images[0], evaluateOperator); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Evaluate(_images[0], evaluateOperator); + return MagickImage.Create(image, GetSettings()); } /// @@ -621,20 +541,17 @@ public IMagickImage Flatten() /// The resulting image of the flatten operation. public IMagickImage Flatten(IMagickColor backgroundColor) { - ThrowIfEmpty(); - var originalColor = _images[0].BackgroundColor; _images[0].BackgroundColor = backgroundColor; try { - AttachImages(); + using var imageAttacher = new TemporaryImageAttacher(_images); var image = _nativeInstance.Merge(_images[0], LayerMethod.Flatten); return MagickImage.Create(image, GetSettings()); } finally { - DetachImages(); _images[0].BackgroundColor = originalColor; } } @@ -690,20 +607,11 @@ public void Map(IMagickImage image) /// Thrown when an error is raised by ImageMagick. public void Map(IMagickImage image, IQuantizeSettings settings) { - ThrowIfEmpty(); - Throw.IfNull(nameof(image), image); Throw.IfNull(nameof(settings), settings); - try - { - AttachImages(); - _nativeInstance.Map(_images[0], settings, image); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.Map(_images[0], settings, image); } /// @@ -723,34 +631,25 @@ public IMagickImage Merge() /// Thrown when an error is raised by ImageMagick. public IMagickImage Montage(IMontageSettings settings) { - ThrowIfEmpty(); - Throw.IfNull(nameof(settings), settings); - IntPtr imagesPtr; - try - { - AttachImages(); - if (!string.IsNullOrEmpty(settings.Label)) - _images[0].Label = settings.Label; - imagesPtr = _nativeInstance.Montage(_images[0], settings); - } - finally - { - DetachImages(); - } + if (!string.IsNullOrEmpty(settings.Label)) + _images[0].Label = settings.Label; + + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.Montage(_images[0], settings); - using var images = new MagickImageCollection(); - images.AddRange(MagickImage.CreateList(imagesPtr, GetSettings())); + using var result = new MagickImageCollection(); + result.AddRange(MagickImage.CreateList(images, GetSettings())); if (settings.TransparentColor is not null) { - foreach (var image in images) + foreach (var image in result) { image.Transparent(settings.TransparentColor); } } - return images.Merge(); + return result.Merge(); } /// @@ -763,22 +662,9 @@ public void Morph(int frames) { ThrowIfCountLowerThan(2); - var settings = GetSettings().Clone(); - - IntPtr images; - try - { - AttachImages(); - images = _nativeInstance.Morph(_images[0], frames); - } - finally - { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - Add(image); + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.Morph(_images[0], frames); + ReplaceImages(images); } /// @@ -798,24 +684,9 @@ public IMagickImage Mosaic() /// Thrown when an error is raised by ImageMagick. public void Optimize() { - ThrowIfEmpty(); - - var settings = GetSettings().Clone(); - - IntPtr images; - try - { - AttachImages(); - images = _nativeInstance.Optimize(_images[0]); - } - finally - { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - Add(image); + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.Optimize(_images[0]); + ReplaceImages(images); } /// @@ -825,24 +696,9 @@ public void Optimize() /// Thrown when an error is raised by ImageMagick. public void OptimizePlus() { - ThrowIfEmpty(); - - var settings = GetSettings().Clone(); - - IntPtr images; - try - { - AttachImages(); - images = _nativeInstance.OptimizePlus(_images[0]); - } - finally - { - DetachImages(); - } - - Clear(); - foreach (var image in MagickImage.CreateList(images, settings)) - Add(image); + using var imageAttacher = new TemporaryImageAttacher(_images); + var images = _nativeInstance.OptimizePlus(_images[0]); + ReplaceImages(images); } /// @@ -852,17 +708,8 @@ public void OptimizePlus() /// Thrown when an error is raised by ImageMagick. public void OptimizeTransparency() { - ThrowIfEmpty(); - - try - { - AttachImages(); - _nativeInstance.OptimizeTransparency(_images[0]); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.OptimizeTransparency(_images[0]); } /// @@ -987,20 +834,11 @@ public void Ping(string fileName, IMagickReadSettings? readSettings /// corresponding terms (coefficient and degree pairs). public IMagickImage Polynomial(double[] terms) { - ThrowIfEmpty(); - Throw.IfNullOrEmpty(nameof(terms), terms); - try - { - AttachImages(); - var image = _nativeInstance.Polynomial(_images[0], terms, terms.Length); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Polynomial(_images[0], terms, terms.Length); + return MagickImage.Create(image, GetSettings()); } /// @@ -1019,19 +857,10 @@ public IMagickImage Polynomial(double[] terms) /// Thrown when an error is raised by ImageMagick. public IMagickErrorInfo? Quantize(IQuantizeSettings settings) { - ThrowIfEmpty(); - Throw.IfNull(nameof(settings), settings); - try - { - AttachImages(); - _nativeInstance.Quantize(_images[0], settings); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.Quantize(_images[0], settings); if (settings.MeasureErrors && _images[0] is MagickImage image) return MagickImage.CreateErrorInfo(image); @@ -1463,24 +1292,16 @@ public byte[] ToByteArray() var settings = GetSettings().Clone(); settings.FileName = null; - try - { - AttachImages(); - - var wrapper = new ByteArrayWrapper(); - var writer = new ReadWriteStreamDelegate(wrapper.Write); - var seeker = new SeekStreamDelegate(wrapper.Seek); - var teller = new TellStreamDelegate(wrapper.Tell); - var reader = new ReadWriteStreamDelegate(wrapper.Read); + var wrapper = new ByteArrayWrapper(); + var writer = new ReadWriteStreamDelegate(wrapper.Write); + var seeker = new SeekStreamDelegate(wrapper.Seek); + var teller = new TellStreamDelegate(wrapper.Tell); + var reader = new ReadWriteStreamDelegate(wrapper.Read); - _nativeInstance.WriteStream(_images[0], settings, writer, seeker, teller, reader); + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.WriteStream(_images[0], settings, writer, seeker, teller, reader); - return wrapper.GetBytes(); - } - finally - { - DetachImages(); - } + return wrapper.GetBytes(); } /// @@ -1536,17 +1357,8 @@ public string ToBase64(MagickFormat format) /// Thrown when an error is raised by ImageMagick. public void TrimBounds() { - ThrowIfEmpty(); - - try - { - AttachImages(); - _nativeInstance.Merge(_images[0], LayerMethod.Trimbounds); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.Merge(_images[0], LayerMethod.Trimbounds); } /// @@ -1605,31 +1417,23 @@ public void Write(Stream stream) var settings = GetSettings().Clone(); settings.FileName = null; - try - { - AttachImages(); - - using var wrapper = StreamWrapper.CreateForWriting(stream); - var writer = new ReadWriteStreamDelegate(wrapper.Write); - ReadWriteStreamDelegate? reader = null; - SeekStreamDelegate? seeker = null; - TellStreamDelegate? teller = null; - - if (stream.CanSeek) - { - seeker = new SeekStreamDelegate(wrapper.Seek); - teller = new TellStreamDelegate(wrapper.Tell); - } - - if (stream.CanRead) - reader = new ReadWriteStreamDelegate(wrapper.Read); + using var wrapper = StreamWrapper.CreateForWriting(stream); + var writer = new ReadWriteStreamDelegate(wrapper.Write); + ReadWriteStreamDelegate? reader = null; + SeekStreamDelegate? seeker = null; + TellStreamDelegate? teller = null; - _nativeInstance.WriteStream(_images[0], settings, writer, seeker, teller, reader); - } - finally + if (stream.CanSeek) { - DetachImages(); + seeker = new SeekStreamDelegate(wrapper.Seek); + teller = new TellStreamDelegate(wrapper.Tell); } + + if (stream.CanRead) + reader = new ReadWriteStreamDelegate(wrapper.Read); + + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.WriteStream(_images[0], settings, writer, seeker, teller, reader); } /// @@ -1675,15 +1479,8 @@ public void Write(string fileName) var settings = GetSettings().Clone(); settings.FileName = fileName; - try - { - AttachImages(); - _nativeInstance.WriteFile(_images[0], settings); - } - finally - { - DetachImages(); - } + using var imageAttacher = new TemporaryImageAttacher(_images); + _nativeInstance.WriteFile(_images[0], settings); } /// @@ -1822,17 +1619,10 @@ public async Task WriteAsync(Stream stream, CancellationToken cancellationToken) if (_images.Count == 0) return; - try - { - AttachImages(); + using var imageAttacher = new TemporaryImageAttacher(_images); - var bytes = ToByteArray(); - await stream.WriteAsync(bytes, 0, bytes.Length, cancellationToken).ConfigureAwait(false); - } - finally - { - DetachImages(); - } + var bytes = ToByteArray(); + await stream.WriteAsync(bytes, 0, bytes.Length, cancellationToken).ConfigureAwait(false); } /// @@ -2047,15 +1837,6 @@ private void AddImages(IntPtr result, MagickSettings settings) } } - private void AttachImages() - { - for (var i = 0; i < _images.Count - 1; i++) - { - if (_images[i] is MagickImage image) - image.SetNext(_images[i + 1]); - } - } - private void CheckDuplicate(IMagickImage item) { foreach (var image in _images) @@ -2065,15 +1846,6 @@ private void CheckDuplicate(IMagickImage item) } } - private void DetachImages() - { - for (var i = 0; i < _images.Count - 1; i++) - { - if (_images[i] is MagickImage image) - image.SetNext(null); - } - } - private void Dispose(bool disposing) { if (_nativeInstance is not null) @@ -2088,18 +1860,18 @@ private MagickSettings GetSettings() private IMagickImage Merge(LayerMethod layerMethod) { - ThrowIfEmpty(); + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Merge(_images[0], layerMethod); + return MagickImage.Create(image, GetSettings()); + } - try - { - AttachImages(); - var image = _nativeInstance.Merge(_images[0], layerMethod); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } + private void ReplaceImages(IntPtr images) + { + var settings = GetSettings().Clone(); + + Clear(); + foreach (var image in MagickImage.CreateList(images, settings)) + Add(image); } private void OnWarning(object sender, WarningEventArgs arguments) @@ -2117,24 +1889,9 @@ private void SetDefines(IWriteDefines defines) private IMagickImage Smush(int offset, bool stack) { - ThrowIfEmpty(); - - try - { - AttachImages(); - var image = _nativeInstance.Smush(_images[0], offset, stack); - return MagickImage.Create(image, GetSettings()); - } - finally - { - DetachImages(); - } - } - - private void ThrowIfEmpty() - { - if (_images.Count == 0) - throw new InvalidOperationException("Operation requires at least one image."); + using var imageAttacher = new TemporaryImageAttacher(_images); + var image = _nativeInstance.Smush(_images[0], offset, stack); + return MagickImage.Create(image, GetSettings()); } private void ThrowIfCountLowerThan(int count) @@ -2156,17 +1913,8 @@ private async Task WriteAsyncInternal(string fileName, MagickFormat? format, Can } else { - try - { - AttachImages(); - - var bytes = format.HasValue ? ToByteArray(format.Value) : ToByteArray(); - await FileHelper.WriteAllBytesAsync(fileName, bytes, cancellationToken).ConfigureAwait(false); - } - finally - { - DetachImages(); - } + var bytes = format.HasValue ? ToByteArray(format.Value) : ToByteArray(); + await FileHelper.WriteAllBytesAsync(fileName, bytes, cancellationToken).ConfigureAwait(false); } } } diff --git a/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMapMethod.cs b/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMapMethod.cs index 4af67e5a6c..b601abc62f 100644 --- a/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMapMethod.cs +++ b/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMapMethod.cs @@ -14,17 +14,20 @@ public class TheMapMethod [Fact] public void ShouldThrowExceptionWhenCollectionIsEmpty() { + using var remapImage = new MagickImage(); using var images = new MagickImageCollection(); - Assert.Throws(() => images.Map(null)); + Assert.Throws(() => images.Map(remapImage)); } [Fact] public void ShouldThrowExceptionWhenCollectionIsEmptyAndImageIsNotNull() { - using var colors = new MagickImageCollection(); - colors.Add(new MagickImage(MagickColors.Red, 1, 1)); - colors.Add(new MagickImage(MagickColors.Green, 1, 1)); + using var colors = new MagickImageCollection + { + new MagickImage(MagickColors.Red, 1, 1), + new MagickImage(MagickColors.Green, 1, 1), + }; using var remapImage = colors.AppendHorizontally(); using var images = new MagickImageCollection(); diff --git a/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMontageMethod.cs b/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMontageMethod.cs index 8d4a79bd9f..29bc979485 100644 --- a/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMontageMethod.cs +++ b/tests/Magick.NET.Tests/MagickImageCollectionTests/TheMontageMethod.cs @@ -14,16 +14,19 @@ public class TheMontageMethod [Fact] public void ShouldThrowExceptionWhenCollectionIsEmpty() { + var settings = new MontageSettings(); using var images = new MagickImageCollection(); - Assert.Throws(() => images.Montage(null)); + Assert.Throws(() => images.Montage(settings)); } [Fact] public void ShouldThrowExceptionWhenSettingsIsNull() { - using var images = new MagickImageCollection(); - images.Add(new MagickImage(MagickColors.Magenta, 1, 1)); + using var images = new MagickImageCollection + { + new MagickImage(MagickColors.Magenta, 1, 1), + }; Assert.Throws("settings", () => images.Montage(null)); }