From 7900b43d1d75c53441e989d44809f5d5f2bca00d Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 13 May 2021 22:19:36 +0300 Subject: [PATCH 01/48] Image.Frames now properly throws ObjectDisposedException after being disposed --- src/ImageSharp/Image{TPixel}.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 83ecc37530..0a0b0d0d60 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -25,6 +25,8 @@ namespace SixLabors.ImageSharp { private bool isDisposed; + private ImageFrameCollection frames; + /// /// Initializes a new instance of the class /// with the height and the width of the image. @@ -84,7 +86,7 @@ namespace SixLabors.ImageSharp internal Image(Configuration configuration, int width, int height, ImageMetadata metadata) : base(configuration, PixelTypeInfo.Create(), metadata, width, height) { - this.Frames = new ImageFrameCollection(this, width, height, default(TPixel)); + this.frames = new ImageFrameCollection(this, width, height, default(TPixel)); } /// @@ -104,7 +106,7 @@ namespace SixLabors.ImageSharp ImageMetadata metadata) : base(configuration, PixelTypeInfo.Create(), metadata, width, height) { - this.Frames = new ImageFrameCollection(this, width, height, memoryGroup); + this.frames = new ImageFrameCollection(this, width, height, memoryGroup); } /// @@ -124,7 +126,7 @@ namespace SixLabors.ImageSharp ImageMetadata metadata) : base(configuration, PixelTypeInfo.Create(), metadata, width, height) { - this.Frames = new ImageFrameCollection(this, width, height, backgroundColor); + this.frames = new ImageFrameCollection(this, width, height, backgroundColor); } /// @@ -137,7 +139,7 @@ namespace SixLabors.ImageSharp internal Image(Configuration configuration, ImageMetadata metadata, IEnumerable> frames) : base(configuration, PixelTypeInfo.Create(), metadata, ValidateFramesAndGetSize(frames)) { - this.Frames = new ImageFrameCollection(this, frames); + this.frames = new ImageFrameCollection(this, frames); } /// @@ -146,7 +148,14 @@ namespace SixLabors.ImageSharp /// /// Gets the collection of image frames. /// - public new ImageFrameCollection Frames { get; } + public new ImageFrameCollection Frames + { + get + { + this.EnsureNotDisposed(); + return this.frames; + } + } /// /// Gets the root frame. From d48b15227da821e9b49c78c6dc88586e892d7145 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 13 May 2021 22:25:05 +0300 Subject: [PATCH 02/48] Image private methods no longer check if object was disposed - it is done at public method calls --- src/ImageSharp/Image{TPixel}.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 0a0b0d0d60..4db3badb01 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -235,10 +235,10 @@ namespace SixLabors.ImageSharp { this.EnsureNotDisposed(); - var clonedFrames = new ImageFrame[this.Frames.Count]; + var clonedFrames = new ImageFrame[this.frames.Count]; for (int i = 0; i < clonedFrames.Length; i++) { - clonedFrames[i] = this.Frames[i].Clone(configuration); + clonedFrames[i] = this.frames[i].Clone(configuration); } return new Image(configuration, this.Metadata.DeepClone(), clonedFrames); @@ -254,10 +254,10 @@ namespace SixLabors.ImageSharp { this.EnsureNotDisposed(); - var clonedFrames = new ImageFrame[this.Frames.Count]; + var clonedFrames = new ImageFrame[this.frames.Count]; for (int i = 0; i < clonedFrames.Length; i++) { - clonedFrames[i] = this.Frames[i].CloneAs(configuration); + clonedFrames[i] = this.frames[i].CloneAs(configuration); } return new Image(configuration, this.Metadata.DeepClone(), clonedFrames); @@ -273,7 +273,7 @@ namespace SixLabors.ImageSharp if (disposing) { - this.Frames.Dispose(); + this.frames.Dispose(); } this.isDisposed = true; @@ -315,9 +315,12 @@ namespace SixLabors.ImageSharp { Guard.NotNull(pixelSource, nameof(pixelSource)); - for (int i = 0; i < this.Frames.Count; i++) + this.EnsureNotDisposed(); + + ImageFrameCollection sourceFrames = pixelSource.Frames; + for (int i = 0; i < this.frames.Count; i++) { - this.Frames[i].SwapOrCopyPixelsBufferFrom(pixelSource.Frames[i]); + this.frames[i].SwapOrCopyPixelsBufferFrom(sourceFrames[i]); } this.UpdateSize(pixelSource.Size()); From 7029b2ffb16fa6257b9ce2716fb02de540949c7a Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 13 May 2021 22:39:15 +0300 Subject: [PATCH 03/48] Image private property PixelSource no longer checks if object was disposed, check is delegated to public methods using that property --- src/ImageSharp/Image{TPixel}.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 4db3badb01..9c32a2c314 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -160,7 +160,7 @@ namespace SixLabors.ImageSharp /// /// Gets the root frame. /// - private IPixelSource PixelSource => this.Frames?.RootFrame ?? throw new ObjectDisposedException(nameof(Image)); + private IPixelSource PixelSource => this.frames.RootFrame; /// /// Gets or sets the pixel at the specified position. @@ -174,6 +174,8 @@ namespace SixLabors.ImageSharp [MethodImpl(InliningOptions.ShortMethod)] get { + this.EnsureNotDisposed(); + this.VerifyCoords(x, y); return this.PixelSource.PixelBuffer.GetElementUnsafe(x, y); } @@ -181,6 +183,8 @@ namespace SixLabors.ImageSharp [MethodImpl(InliningOptions.ShortMethod)] set { + this.EnsureNotDisposed(); + this.VerifyCoords(x, y); this.PixelSource.PixelBuffer.GetElementUnsafe(x, y) = value; } @@ -198,6 +202,8 @@ namespace SixLabors.ImageSharp Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex)); Guard.MustBeLessThan(rowIndex, this.Height, nameof(rowIndex)); + this.EnsureNotDisposed(); + return this.PixelSource.PixelBuffer.GetRowSpan(rowIndex); } From 1c45c1a7055a90af8bcb288140422eaa7db405ba Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 13 May 2021 22:51:55 +0300 Subject: [PATCH 04/48] Removed GC.SuppressFinalize(this) from Image.Dispose() due to it not having a Finalization method --- src/ImageSharp/Image.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index fbb3ec2069..c07c7fe83f 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -78,11 +78,7 @@ namespace SixLabors.ImageSharp Configuration IConfigurationProvider.Configuration => this.configuration; /// - public void Dispose() - { - this.Dispose(true); - GC.SuppressFinalize(this); - } + public void Dispose() => this.Dispose(true); /// /// Saves the image to the given stream using the given image encoder. From 095ce416260625bff00b5e4877954bcf446ab7b5 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 00:29:41 +0300 Subject: [PATCH 05/48] Added tests for issue#1628 --- tests/ImageSharp.Tests/Image/ImageTests.cs | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.cs b/tests/ImageSharp.Tests/Image/ImageTests.cs index b7c6b3835a..1c942ac055 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.IO; using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; @@ -169,5 +171,74 @@ namespace SixLabors.ImageSharp.Tests Assert.Equal("y", ex.ParamName); } } + + public class Dispose + { + private readonly Configuration configuration = Configuration.CreateDefaultInstance(); + + public void MultipleDisposeCalls() + { + var image = new Image(this.configuration, 10, 10); + image.Dispose(); + image.Dispose(); + } + + [Fact] + public void NonPrivateProperties_ObjectDisposedException() + { + var image = new Image(this.configuration, 10, 10); + var genericImage = (Image)image; + + image.Dispose(); + + // Image + Assert.Throws(() => { var prop = image.Frames; }); + Assert.Throws(() => { var prop = image.Metadata; }); + Assert.Throws(() => { var prop = image.PixelType; }); + + // Image + Assert.Throws(() => { var prop = genericImage.Frames; }); + } + + [Fact] + public void Save_ObjectDisposedException() + { + var image = new Image(this.configuration, 10, 10); + var stream = new MemoryStream(); + var encoder = new JpegEncoder(); + + image.Dispose(); + + // Image + Assert.Throws(() => image.Save(stream, encoder)); + } + + [Fact] + public void AcceptVisitor_ObjectDisposedException() + { + // This test technically should exist but it's impossible to write proper test case without reflection: + // All visitor types are private and can't be created without context of some save/processing operation + // Save_ObjectDisposedException test checks this method with AcceptVisitor(EncodeVisitor) anyway + return; + } + + [Fact] + public void NonPrivateMethods_ObjectDisposedException() + { + var image = new Image(this.configuration, 10, 10); + var genericImage = (Image)image; + + image.Dispose(); + + // Image + Assert.Throws(() => { var res = image.Clone(this.configuration); }); + Assert.Throws(() => { var res = image.CloneAs(this.configuration); }); + Assert.Throws(() => { var res = image.GetPixelRowSpan(default); }); + Assert.Throws(() => { var res = image.TryGetSinglePixelSpan(out var _); }); + + // Image + Assert.Throws(() => { var res = genericImage.CloneAs(this.configuration); }); + } + } } } From acf9d85e8ca8a5ebd4894e85e1d6fe82d2e097b2 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 00:36:40 +0300 Subject: [PATCH 06/48] Moved dispose control logic to base Image class internal call EnsureNotDisposed is no longer virtual -> micro speedup gain in pixel index accessor Image[x, y]. --- src/ImageSharp/Image.cs | 22 ++++++++++++++++++++-- src/ImageSharp/Image{TPixel}.cs | 18 ------------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index c07c7fe83f..3aa30063d8 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -19,6 +19,8 @@ namespace SixLabors.ImageSharp /// public abstract partial class Image : IImage, IConfigurationProvider { + private bool isDisposed; + private Size size; private readonly Configuration configuration; @@ -78,7 +80,17 @@ namespace SixLabors.ImageSharp Configuration IConfigurationProvider.Configuration => this.configuration; /// - public void Dispose() => this.Dispose(true); + public void Dispose() + { + if (this.isDisposed) + { + return; + } + + this.Dispose(true); + + this.isDisposed = true; + } /// /// Saves the image to the given stream using the given image encoder. @@ -144,7 +156,13 @@ namespace SixLabors.ImageSharp /// /// Throws if the image is disposed. /// - internal abstract void EnsureNotDisposed(); + internal void EnsureNotDisposed() + { + if (this.isDisposed) + { + throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); + } + } /// /// Accepts a . diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 9c32a2c314..c436430522 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -23,8 +23,6 @@ namespace SixLabors.ImageSharp public sealed class Image : Image where TPixel : unmanaged, IPixel { - private bool isDisposed; - private ImageFrameCollection frames; /// @@ -272,26 +270,10 @@ namespace SixLabors.ImageSharp /// protected override void Dispose(bool disposing) { - if (this.isDisposed) - { - return; - } - if (disposing) { this.frames.Dispose(); } - - this.isDisposed = true; - } - - /// - internal override void EnsureNotDisposed() - { - if (this.isDisposed) - { - throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); - } } /// From 8ec1013ce8ff41e933b21590333431ac45b4b536 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 00:55:27 +0300 Subject: [PATCH 07/48] Removed redundant flag from Image.Dispose(bool) call As Image does not have unmanaged resources and does not implement finalizer method, there's no need for disposable pattern with a pair of Dispose() & Dispose(bool). Due Dispose(bool) was changed to DisposeManaged(). --- src/ImageSharp/Image.cs | 7 +++---- src/ImageSharp/Image{TPixel}.cs | 8 +------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 3aa30063d8..a3b425233b 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp return; } - this.Dispose(true); + this.DisposeManaged(); this.isDisposed = true; } @@ -148,10 +148,9 @@ namespace SixLabors.ImageSharp protected void UpdateSize(Size size) => this.size = size; /// - /// Disposes the object and frees resources for the Garbage Collector. + /// Internal routine for freeing managed resources called from /// - /// Whether to dispose of managed and unmanaged objects. - protected abstract void Dispose(bool disposing); + protected abstract void DisposeManaged(); /// /// Throws if the image is disposed. diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index c436430522..1fc77dc1f0 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -268,13 +268,7 @@ namespace SixLabors.ImageSharp } /// - protected override void Dispose(bool disposing) - { - if (disposing) - { - this.frames.Dispose(); - } - } + protected override void DisposeManaged() => this.frames.Dispose(); /// public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; From ff4b269d590fbbad7d277a4f200ee0c7ac080e41 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 01:03:50 +0300 Subject: [PATCH 08/48] Removed invalid tests Subject to discuss. Image public properties Height, Width, Metadata and PixelType can't corrupt anything if backing image was disposed so I don't see any point altering that behaviour, it wasn't throwing before this branch and shouldn't throw after. --- tests/ImageSharp.Tests/Image/ImageTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.cs b/tests/ImageSharp.Tests/Image/ImageTests.cs index 1c942ac055..b6d78a356d 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.cs @@ -193,8 +193,6 @@ namespace SixLabors.ImageSharp.Tests // Image Assert.Throws(() => { var prop = image.Frames; }); - Assert.Throws(() => { var prop = image.Metadata; }); - Assert.Throws(() => { var prop = image.PixelType; }); // Image Assert.Throws(() => { var prop = genericImage.Frames; }); From cbca5657889fb9a370f5b049d60dbae108104cfd Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 03:00:57 +0300 Subject: [PATCH 09/48] ImageFrameCollection now properly implements IDisposable interface & ensures all operations are called on valid object --- src/ImageSharp/ImageFrameCollection.cs | 95 +++++++++++++++++-- .../ImageFrameCollection{TPixel}.cs | 3 +- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/ImageFrameCollection.cs b/src/ImageSharp/ImageFrameCollection.cs index 62ecc71f55..53ab2bf7c1 100644 --- a/src/ImageSharp/ImageFrameCollection.cs +++ b/src/ImageSharp/ImageFrameCollection.cs @@ -11,8 +11,10 @@ namespace SixLabors.ImageSharp /// Encapsulates a pixel-agnostic collection of instances /// that make up an . /// - public abstract class ImageFrameCollection : IEnumerable + public abstract class ImageFrameCollection : IDisposable, IEnumerable { + private bool isDisposed; + /// /// Gets the number of frames. /// @@ -21,7 +23,15 @@ namespace SixLabors.ImageSharp /// /// Gets the root frame. /// - public ImageFrame RootFrame => this.NonGenericRootFrame; + public ImageFrame RootFrame + { + get + { + this.EnsureNotDisposed(); + + return this.NonGenericRootFrame; + } + } /// /// Gets the root frame. (Implements .) @@ -36,7 +46,15 @@ namespace SixLabors.ImageSharp /// /// The index. /// The at the specified index. - public ImageFrame this[int index] => this.NonGenericGetFrame(index); + public ImageFrame this[int index] + { + get + { + this.EnsureNotDisposed(); + + return this.NonGenericGetFrame(index); + } + } /// /// Determines the index of a specific in the . @@ -59,7 +77,12 @@ namespace SixLabors.ImageSharp /// /// The raw pixel data to generate the from. /// The cloned . - public ImageFrame AddFrame(ImageFrame source) => this.NonGenericAddFrame(source); + public ImageFrame AddFrame(ImageFrame source) + { + this.EnsureNotDisposed(); + + return this.NonGenericAddFrame(source); + } /// /// Removes the frame at the specified index and frees all freeable resources associated with it. @@ -91,7 +114,12 @@ namespace SixLabors.ImageSharp /// The zero-based index of the frame to export. /// Cannot remove last frame. /// The new with the specified frame. - public Image ExportFrame(int index) => this.NonGenericExportFrame(index); + public Image ExportFrame(int index) + { + this.EnsureNotDisposed(); + + return this.NonGenericExportFrame(index); + } /// /// Creates an with only the frame at the specified index @@ -99,7 +127,12 @@ namespace SixLabors.ImageSharp /// /// The zero-based index of the frame to clone. /// The new with the specified frame. - public Image CloneFrame(int index) => this.NonGenericCloneFrame(index); + public Image CloneFrame(int index) + { + this.EnsureNotDisposed(); + + return this.NonGenericCloneFrame(index); + } /// /// Creates a new and appends it to the end of the collection. @@ -107,7 +140,12 @@ namespace SixLabors.ImageSharp /// /// The new . /// - public ImageFrame CreateFrame() => this.NonGenericCreateFrame(); + public ImageFrame CreateFrame() + { + this.EnsureNotDisposed(); + + return this.NonGenericCreateFrame(); + } /// /// Creates a new and appends it to the end of the collection. @@ -116,14 +154,53 @@ namespace SixLabors.ImageSharp /// /// The new . /// - public ImageFrame CreateFrame(Color backgroundColor) => this.NonGenericCreateFrame(backgroundColor); + public ImageFrame CreateFrame(Color backgroundColor) + { + this.EnsureNotDisposed(); + + return this.NonGenericCreateFrame(backgroundColor); + } /// - public IEnumerator GetEnumerator() => this.NonGenericGetEnumerator(); + public void Dispose() + { + if (this.isDisposed) + { + return; + } + + this.DisposeManaged(); + + this.isDisposed = true; + } + + /// + public IEnumerator GetEnumerator() + { + this.EnsureNotDisposed(); + + return this.NonGenericGetEnumerator(); + } /// IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator(); + /// + /// Throws if the image frame is disposed. + /// + protected void EnsureNotDisposed() + { + if(this.isDisposed) + { + throw new ObjectDisposedException("Trying to execute an operation on a disposed image frame."); + } + } + + /// + /// Internal routine for freeing managed resources called from + /// + protected abstract void DisposeManaged(); + /// /// Implements . /// diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index 36c3ee481f..b51e4dae53 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -335,7 +335,8 @@ namespace SixLabors.ImageSharp } } - internal void Dispose() + /// + protected override void DisposeManaged() { foreach (ImageFrame f in this.frames) { From 127e9ddcdd37e7030febb17efbb323526c05bd01 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 03:13:16 +0300 Subject: [PATCH 10/48] All ImageFrameCollection public properties & method now check if object was disposed --- .../ImageFrameCollection{TPixel}.cs | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index b51e4dae53..c1eae02806 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -67,7 +67,17 @@ namespace SixLabors.ImageSharp /// /// Gets the root frame. /// - public new ImageFrame RootFrame => this.frames.Count > 0 ? this.frames[0] : null; + public new ImageFrame RootFrame + { + get + { + this.EnsureNotDisposed(); + + // frame collection would always contain at least 1 frame + // the only exception is when collection is disposed what is checked via EnsureNotDisposed() call + return this.frames[0]; + } + } /// protected override ImageFrame NonGenericRootFrame => this.RootFrame; @@ -80,20 +90,30 @@ namespace SixLabors.ImageSharp /// /// The index. /// The at the specified index. - public new ImageFrame this[int index] => this.frames[index]; - - /// - public override int IndexOf(ImageFrame frame) + public new ImageFrame this[int index] { - return frame is ImageFrame specific ? this.IndexOf(specific) : -1; + get + { + this.EnsureNotDisposed(); + + return this.frames[index]; + } } + /// + public override int IndexOf(ImageFrame frame) => frame is ImageFrame specific ? this.IndexOf(specific) : -1; + /// /// Determines the index of a specific in the . /// /// The to locate in the . /// The index of item if found in the list; otherwise, -1. - public int IndexOf(ImageFrame frame) => this.frames.IndexOf(frame); + public int IndexOf(ImageFrame frame) + { + this.EnsureNotDisposed(); + + return this.frames.IndexOf(frame); + } /// /// Clones and inserts the into the at the specified . @@ -104,6 +124,8 @@ namespace SixLabors.ImageSharp /// The cloned . public ImageFrame InsertFrame(int index, ImageFrame source) { + this.EnsureNotDisposed(); + this.ValidateFrame(source); ImageFrame clonedFrame = source.Clone(this.parent.GetConfiguration()); this.frames.Insert(index, clonedFrame); @@ -117,6 +139,8 @@ namespace SixLabors.ImageSharp /// The cloned . public ImageFrame AddFrame(ImageFrame source) { + this.EnsureNotDisposed(); + this.ValidateFrame(source); ImageFrame clonedFrame = source.Clone(this.parent.GetConfiguration()); this.frames.Add(clonedFrame); @@ -131,6 +155,8 @@ namespace SixLabors.ImageSharp /// The new . public ImageFrame AddFrame(ReadOnlySpan source) { + this.EnsureNotDisposed(); + var frame = ImageFrame.LoadPixelData( this.parent.GetConfiguration(), source, @@ -149,6 +175,7 @@ namespace SixLabors.ImageSharp public ImageFrame AddFrame(TPixel[] source) { Guard.NotNull(source, nameof(source)); + return this.AddFrame(source.AsSpan()); } @@ -159,6 +186,8 @@ namespace SixLabors.ImageSharp /// Cannot remove last frame. public override void RemoveFrame(int index) { + this.EnsureNotDisposed(); + if (index == 0 && this.Count == 1) { throw new InvalidOperationException("Cannot remove last frame."); @@ -180,7 +209,12 @@ namespace SixLabors.ImageSharp /// /// true if the contains the specified frame; otherwise, false. /// - public bool Contains(ImageFrame frame) => this.frames.Contains(frame); + public bool Contains(ImageFrame frame) + { + this.EnsureNotDisposed(); + + return this.frames.Contains(frame); + } /// /// Moves an from to . @@ -189,6 +223,8 @@ namespace SixLabors.ImageSharp /// The index to move the frame to. public override void MoveFrame(int sourceIndex, int destinationIndex) { + this.EnsureNotDisposed(); + if (sourceIndex == destinationIndex) { return; @@ -208,6 +244,8 @@ namespace SixLabors.ImageSharp /// The new with the specified frame. public new Image ExportFrame(int index) { + this.EnsureNotDisposed(); + ImageFrame frame = this[index]; if (this.Count == 1 && this.frames.Contains(frame)) @@ -228,6 +266,8 @@ namespace SixLabors.ImageSharp /// The new with the specified frame. public new Image CloneFrame(int index) { + this.EnsureNotDisposed(); + ImageFrame frame = this[index]; ImageFrame clonedFrame = frame.Clone(); return new Image(this.parent.GetConfiguration(), this.parent.Metadata.DeepClone(), new[] { clonedFrame }); @@ -241,6 +281,8 @@ namespace SixLabors.ImageSharp /// public new ImageFrame CreateFrame() { + this.EnsureNotDisposed(); + var frame = new ImageFrame( this.parent.GetConfiguration(), this.RootFrame.Width, From 3f8bd3d2e67913d2aa3500928dca44d6ed7fd35b Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 03:20:03 +0300 Subject: [PATCH 11/48] Added internal accessor for root frame --- src/ImageSharp/ImageFrameCollection{TPixel}.cs | 9 +++++++++ src/ImageSharp/Image{TPixel}.cs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index c1eae02806..023da0d347 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -79,6 +79,15 @@ namespace SixLabors.ImageSharp } } + /// + /// Gets root frame accessor in unsafe manner without any checks. + /// + /// + /// This property is most likely to be called from for indexing pixels. + /// already checks if it was disposed before querying for root frame. + /// + internal ImageFrame RootFrameUnsafe => this.frames[0]; + /// protected override ImageFrame NonGenericRootFrame => this.RootFrame; diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 1fc77dc1f0..3805446fe5 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -158,7 +158,7 @@ namespace SixLabors.ImageSharp /// /// Gets the root frame. /// - private IPixelSource PixelSource => this.frames.RootFrame; + private IPixelSource PixelSource => this.frames.RootFrameUnsafe; /// /// Gets or sets the pixel at the specified position. From 009e9357bd13776b8cf2f8f90dfa9332f87e8d84 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 03:59:58 +0300 Subject: [PATCH 12/48] Added tests for issues#1628 --- .../ImageFrameCollectionTests.Generic.cs | 42 +++++++++++++++++++ .../ImageFrameCollectionTests.NonGeneric.cs | 35 ++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs index ecbc331b28..c889fa76b2 100644 --- a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs +++ b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs @@ -323,6 +323,48 @@ namespace SixLabors.ImageSharp.Tests var frame = new ImageFrame(Configuration.Default, 10, 10); Assert.False(this.Image.Frames.Contains(frame)); } + + [Fact] + public void DisposeCall_NoThrowIfCalledMultiple() + { + var image = new Image(Configuration.Default, 10, 10); + var frameCollection = image.Frames as ImageFrameCollection; + + image.Dispose(); // this should invalidate underlying collection as well + frameCollection.Dispose(); + } + + [Fact] + public void PublicProperties_ThrowIfDisposed() + { + var image = new Image(Configuration.Default, 10, 10); + var frameCollection = image.Frames as ImageFrameCollection; + + image.Dispose(); // this should invalidate underlying collection as well + + Assert.Throws(() => { var prop = frameCollection.RootFrame; }); + } + + [Fact] + public void PublicMethods_ThrowIfDisposed() + { + var image = new Image(Configuration.Default, 10, 10); + var frameCollection = image.Frames as ImageFrameCollection; + + image.Dispose(); // this should invalidate underlying collection as well + + Assert.Throws(() => { var res = frameCollection.AddFrame(default); }); + Assert.Throws(() => { var res = frameCollection.CloneFrame(default); }); + Assert.Throws(() => { var res = frameCollection.Contains(default); }); + Assert.Throws(() => { var res = frameCollection.CreateFrame(); }); + Assert.Throws(() => { var res = frameCollection.CreateFrame(default); }); + Assert.Throws(() => { var res = frameCollection.ExportFrame(default); }); + Assert.Throws(() => { var res = frameCollection.GetEnumerator(); }); + Assert.Throws(() => { var prop = frameCollection.IndexOf(default); }); + Assert.Throws(() => { var prop = frameCollection.InsertFrame(default, default); }); + Assert.Throws(() => { frameCollection.RemoveFrame(default); }); + Assert.Throws(() => { frameCollection.MoveFrame(default, default); }); + } } } } diff --git a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs index 92109ed479..7ff6b9b085 100644 --- a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs +++ b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs @@ -263,6 +263,41 @@ namespace SixLabors.ImageSharp.Tests Assert.False(this.Image.Frames.Contains(frame)); } + [Fact] + public void PublicProperties_ThrowIfDisposed() + { + var image = new Image(Configuration.Default, 10, 10); + var frameCollection = image.Frames; + + image.Dispose(); // this should invalidate underlying collection as well + + Assert.Throws(() => { var prop = frameCollection.RootFrame; }); + } + + [Fact] + public void PublicMethods_ThrowIfDisposed() + { + var image = new Image(Configuration.Default, 10, 10); + var frameCollection = image.Frames; + + image.Dispose(); // this should invalidate underlying collection as well + + Assert.Throws(() => { var res = frameCollection.AddFrame((ImageFrame)null); }); + Assert.Throws(() => { var res = frameCollection.AddFrame((Rgba32[])null); }); + Assert.Throws(() => { var res = frameCollection.AddFrame((ImageFrame)null); }); + Assert.Throws(() => { var res = frameCollection.AddFrame(stackalloc Rgba32[0]); }); + Assert.Throws(() => { var res = frameCollection.CloneFrame(default); }); + Assert.Throws(() => { var res = frameCollection.Contains(default); }); + Assert.Throws(() => { var res = frameCollection.CreateFrame(); }); + Assert.Throws(() => { var res = frameCollection.CreateFrame(default); }); + Assert.Throws(() => { var res = frameCollection.ExportFrame(default); }); + Assert.Throws(() => { var res = frameCollection.GetEnumerator(); }); + Assert.Throws(() => { var prop = frameCollection.IndexOf(default); }); + Assert.Throws(() => { var prop = frameCollection.InsertFrame(default, default); }); + Assert.Throws(() => { frameCollection.RemoveFrame(default); }); + Assert.Throws(() => { frameCollection.MoveFrame(default, default); }); + } + /// /// Integration test for end-to end API validation. /// From f0f0c088abdf974f4c2e0a09425b5481ae2791d1 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 05:27:25 +0300 Subject: [PATCH 13/48] Fixed couple of invalid tests for ImageFrameCollection --- .../Image/ImageFrameCollectionTests.NonGeneric.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs index 7ff6b9b085..15838f6902 100644 --- a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs +++ b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.NonGeneric.cs @@ -279,13 +279,14 @@ namespace SixLabors.ImageSharp.Tests { var image = new Image(Configuration.Default, 10, 10); var frameCollection = image.Frames; + var rgba32Array = new Rgba32[0]; image.Dispose(); // this should invalidate underlying collection as well Assert.Throws(() => { var res = frameCollection.AddFrame((ImageFrame)null); }); - Assert.Throws(() => { var res = frameCollection.AddFrame((Rgba32[])null); }); + Assert.Throws(() => { var res = frameCollection.AddFrame(rgba32Array); }); Assert.Throws(() => { var res = frameCollection.AddFrame((ImageFrame)null); }); - Assert.Throws(() => { var res = frameCollection.AddFrame(stackalloc Rgba32[0]); }); + Assert.Throws(() => { var res = frameCollection.AddFrame(rgba32Array.AsSpan()); }); Assert.Throws(() => { var res = frameCollection.CloneFrame(default); }); Assert.Throws(() => { var res = frameCollection.Contains(default); }); Assert.Throws(() => { var res = frameCollection.CreateFrame(); }); From a71ce1913f6066b3cf9769f37cbea063c57c3e23 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 05:28:28 +0300 Subject: [PATCH 14/48] ImageFrameCollection.Contains first checks if it was disposed first --- src/ImageSharp/ImageFrameCollection{TPixel}.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index 023da0d347..92722494b1 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -208,8 +208,12 @@ namespace SixLabors.ImageSharp } /// - public override bool Contains(ImageFrame frame) => - frame is ImageFrame specific && this.Contains(specific); + public override bool Contains(ImageFrame frame) + { + this.EnsureNotDisposed(); + + return frame is ImageFrame specific && this.frames.Contains(specific); + } /// /// Determines whether the contains the . From f56105393b2e9300dfba5cf4979578f7a99c176b Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 14 May 2021 19:14:53 +0300 Subject: [PATCH 15/48] Fixed a couple of failing tests --- src/ImageSharp/ImageFrameCollection.cs | 7 ++++++- src/ImageSharp/ImageFrameCollection{TPixel}.cs | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/ImageFrameCollection.cs b/src/ImageSharp/ImageFrameCollection.cs index 53ab2bf7c1..ed36c16539 100644 --- a/src/ImageSharp/ImageFrameCollection.cs +++ b/src/ImageSharp/ImageFrameCollection.cs @@ -70,7 +70,12 @@ namespace SixLabors.ImageSharp /// The to clone and insert into the . /// Frame must have the same dimensions as the image. /// The cloned . - public ImageFrame InsertFrame(int index, ImageFrame source) => this.NonGenericInsertFrame(index, source); + public ImageFrame InsertFrame(int index, ImageFrame source) + { + this.EnsureNotDisposed(); + + return this.NonGenericInsertFrame(index, source); + } /// /// Clones the frame and appends the clone to the end of the collection. diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index 92722494b1..13ae092c42 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -110,7 +110,12 @@ namespace SixLabors.ImageSharp } /// - public override int IndexOf(ImageFrame frame) => frame is ImageFrame specific ? this.IndexOf(specific) : -1; + public override int IndexOf(ImageFrame frame) + { + this.EnsureNotDisposed(); + + return frame is ImageFrame specific ? this.frames.IndexOf(specific) : -1; + } /// /// Determines the index of a specific in the . From 1c8dcefd6dda4fd1677e0f2d8a64a2edc0086d46 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 21 May 2021 20:16:21 +0300 Subject: [PATCH 16/48] Renamed private Image.PixelSourse to PixelSourceUnsafe --- src/ImageSharp/Image{TPixel}.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 3805446fe5..9e3ad2636f 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -158,7 +158,7 @@ namespace SixLabors.ImageSharp /// /// Gets the root frame. /// - private IPixelSource PixelSource => this.frames.RootFrameUnsafe; + private IPixelSource PixelSourceUnsafe => this.frames.RootFrameUnsafe; /// /// Gets or sets the pixel at the specified position. @@ -175,7 +175,7 @@ namespace SixLabors.ImageSharp this.EnsureNotDisposed(); this.VerifyCoords(x, y); - return this.PixelSource.PixelBuffer.GetElementUnsafe(x, y); + return this.PixelSourceUnsafe.PixelBuffer.GetElementUnsafe(x, y); } [MethodImpl(InliningOptions.ShortMethod)] @@ -184,7 +184,7 @@ namespace SixLabors.ImageSharp this.EnsureNotDisposed(); this.VerifyCoords(x, y); - this.PixelSource.PixelBuffer.GetElementUnsafe(x, y) = value; + this.PixelSourceUnsafe.PixelBuffer.GetElementUnsafe(x, y) = value; } } @@ -202,7 +202,7 @@ namespace SixLabors.ImageSharp this.EnsureNotDisposed(); - return this.PixelSource.PixelBuffer.GetRowSpan(rowIndex); + return this.PixelSourceUnsafe.PixelBuffer.GetRowSpan(rowIndex); } /// From e787ffa518b2e20406bbe41a9a0e611fa28f87b2 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 21 May 2021 20:23:10 +0300 Subject: [PATCH 17/48] Implemented dispose method according to common convention. --- src/ImageSharp/Image.cs | 6 ++++-- src/ImageSharp/ImageFrameCollection.cs | 6 ++++-- src/ImageSharp/ImageFrameCollection{TPixel}.cs | 13 ++++++++----- src/ImageSharp/Image{TPixel}.cs | 8 +++++++- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index a3b425233b..724fa4f96b 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -87,7 +87,8 @@ namespace SixLabors.ImageSharp return; } - this.DisposeManaged(); + this.Dispose(true); + GC.SuppressFinalize(this); this.isDisposed = true; } @@ -150,7 +151,8 @@ namespace SixLabors.ImageSharp /// /// Internal routine for freeing managed resources called from /// - protected abstract void DisposeManaged(); + /// /// Whether to dispose of managed objects. + protected abstract void Dispose(bool disposing); /// /// Throws if the image is disposed. diff --git a/src/ImageSharp/ImageFrameCollection.cs b/src/ImageSharp/ImageFrameCollection.cs index ed36c16539..16d4285782 100644 --- a/src/ImageSharp/ImageFrameCollection.cs +++ b/src/ImageSharp/ImageFrameCollection.cs @@ -174,7 +174,8 @@ namespace SixLabors.ImageSharp return; } - this.DisposeManaged(); + this.Dispose(true); + GC.SuppressFinalize(this); this.isDisposed = true; } @@ -204,7 +205,8 @@ namespace SixLabors.ImageSharp /// /// Internal routine for freeing managed resources called from /// - protected abstract void DisposeManaged(); + /// /// /// Whether to dispose of managed objects. + protected abstract void Dispose(bool disposing); /// /// Implements . diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index 13ae092c42..da024c9176 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -396,14 +396,17 @@ namespace SixLabors.ImageSharp } /// - protected override void DisposeManaged() + protected override void Dispose(bool disposing) { - foreach (ImageFrame f in this.frames) + if (disposing) { - f.Dispose(); - } + foreach (ImageFrame f in this.frames) + { + f.Dispose(); + } - this.frames.Clear(); + this.frames.Clear(); + } } private ImageFrame CopyNonCompatibleFrame(ImageFrame source) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 9e3ad2636f..e42022729f 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -268,7 +268,13 @@ namespace SixLabors.ImageSharp } /// - protected override void DisposeManaged() => this.frames.Dispose(); + protected override void Dispose(bool disposing) + { + if (disposing) + { + this.frames.Dispose(); + } + } /// public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; From d54ff0e084aa823464f4f64c0ef32f30471b5c79 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 21 May 2021 20:25:31 +0300 Subject: [PATCH 18/48] Fixed disposable resouce leak in unit test. --- tests/ImageSharp.Tests/Image/ImageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.cs b/tests/ImageSharp.Tests/Image/ImageTests.cs index b6d78a356d..1296f26c47 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.cs @@ -201,8 +201,8 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Save_ObjectDisposedException() { + using var stream = new MemoryStream(); var image = new Image(this.configuration, 10, 10); - var stream = new MemoryStream(); var encoder = new JpegEncoder(); image.Dispose(); From 5704403030f8781da651c261fc4a4b50360c675c Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 21 May 2021 20:52:38 +0300 Subject: [PATCH 19/48] Implemented ThrowObjectDisposedException for the ThrowHelper, replaced raw throws with this in Image/ImageFrameCollection --- src/ImageSharp/Image.cs | 2 +- src/ImageSharp/ImageFrameCollection.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 724fa4f96b..fe72ec5c00 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -161,7 +161,7 @@ namespace SixLabors.ImageSharp { if (this.isDisposed) { - throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); + ThrowHelper.ThrowObjectDisposedException(this.GetType()); } } diff --git a/src/ImageSharp/ImageFrameCollection.cs b/src/ImageSharp/ImageFrameCollection.cs index 16d4285782..8c8edcd7a5 100644 --- a/src/ImageSharp/ImageFrameCollection.cs +++ b/src/ImageSharp/ImageFrameCollection.cs @@ -196,9 +196,9 @@ namespace SixLabors.ImageSharp /// protected void EnsureNotDisposed() { - if(this.isDisposed) + if (this.isDisposed) { - throw new ObjectDisposedException("Trying to execute an operation on a disposed image frame."); + ThrowHelper.ThrowObjectDisposedException(this.GetType()); } } From 5ceba7116cadfb29af12275b57dc9a8997a3cf99 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 13:42:14 +0100 Subject: [PATCH 20/48] Ensure cdfMinSpan is cleared before use. --- .../AdaptiveHistogramEqualizationProcessor.cs | 9 +--- ...eHistogramEqualizationProcessor{TPixel}.cs | 13 ++++-- .../HistogramEqualizationProcessor.cs | 44 ++++--------------- .../HistogramEqualizationProcessor{TPixel}.cs | 1 + .../HistogramEqualizationTests.cs | 31 +++++++++++++ .../Issue1640_L16_TestPattern5120x9234.png | 3 ++ 6 files changed, 56 insertions(+), 45 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png diff --git a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor.cs index 56593acb84..9b28a8fdd8 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor.cs @@ -22,10 +22,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization bool clipHistogram, int clipLimit, int numberOfTiles) - : base(luminanceLevels, clipHistogram, clipLimit) - { - this.NumberOfTiles = numberOfTiles; - } + : base(luminanceLevels, clipHistogram, clipLimit) => this.NumberOfTiles = numberOfTiles; /// /// Gets the number of tiles the image is split into (horizontal and vertically) for the adaptive histogram equalization. @@ -34,8 +31,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization /// public override IImageProcessor CreatePixelSpecificProcessor(Configuration configuration, Image source, Rectangle sourceRectangle) - { - return new AdaptiveHistogramEqualizationProcessor( + => new AdaptiveHistogramEqualizationProcessor( configuration, this.LuminanceLevels, this.ClipHistogram, @@ -43,6 +39,5 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization this.NumberOfTiles, source, sourceRectangle); - } } } diff --git a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs index 14687426d0..317db83b44 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs @@ -459,10 +459,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization private readonly Configuration configuration; private readonly MemoryAllocator memoryAllocator; - // Used for storing the minimum value for each CDF entry. + /// + /// Used for storing the minimum value for each CDF entry. + /// private readonly Buffer2D cdfMinBuffer2D; - // Used for storing the LUT for each CDF entry. + /// + /// Used for storing the LUT for each CDF entry. + /// private readonly Buffer2D cdfLutBuffer2D; private readonly int pixelsInTile; private readonly int sourceWidth; @@ -596,6 +600,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization int y = this.tileYStartPositions[index].y; int endY = Math.Min(y + this.tileHeight, this.sourceHeight); Span cdfMinSpan = this.cdfMinBuffer2D.GetRowSpan(cdfY); + cdfMinSpan.Clear(); using IMemoryOwner histogramBuffer = this.allocator.Allocate(this.luminanceLevels); Span histogram = histogramBuffer.GetSpan(); @@ -614,7 +619,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization for (int dx = x; dx < xlimit; dx++) { int luminance = GetLuminance(rowSpan[dx], this.luminanceLevels); - histogram[luminance]++; + + // This is safe. The index maxes out to the span length. + Unsafe.Add(ref histogramBase, luminance)++; } } diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs index 60686f4014..f93334beb0 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs @@ -49,44 +49,18 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization /// /// The . /// The . - public static HistogramEqualizationProcessor FromOptions(HistogramEqualizationOptions options) + public static HistogramEqualizationProcessor FromOptions(HistogramEqualizationOptions options) => options.Method switch { - HistogramEqualizationProcessor processor; + HistogramEqualizationMethod.Global + => new GlobalHistogramEqualizationProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), - switch (options.Method) - { - case HistogramEqualizationMethod.Global: - processor = new GlobalHistogramEqualizationProcessor( - options.LuminanceLevels, - options.ClipHistogram, - options.ClipLimit); - break; + HistogramEqualizationMethod.AdaptiveTileInterpolation + => new AdaptiveHistogramEqualizationProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit, options.NumberOfTiles), - case HistogramEqualizationMethod.AdaptiveTileInterpolation: - processor = new AdaptiveHistogramEqualizationProcessor( - options.LuminanceLevels, - options.ClipHistogram, - options.ClipLimit, - options.NumberOfTiles); - break; + HistogramEqualizationMethod.AdaptiveSlidingWindow + => new AdaptiveHistogramEqualizationSlidingWindowProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit, options.NumberOfTiles), - case HistogramEqualizationMethod.AdaptiveSlidingWindow: - processor = new AdaptiveHistogramEqualizationSlidingWindowProcessor( - options.LuminanceLevels, - options.ClipHistogram, - options.ClipLimit, - options.NumberOfTiles); - break; - - default: - processor = new GlobalHistogramEqualizationProcessor( - options.LuminanceLevels, - options.ClipHistogram, - options.ClipLimit); - break; - } - - return processor; - } + _ => new GlobalHistogramEqualizationProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), + }; } } diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor{TPixel}.cs index 59df3058d9..9227cb0c01 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor{TPixel}.cs @@ -142,6 +142,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization [MethodImpl(InliningOptions.ShortMethod)] public static int GetLuminance(TPixel sourcePixel, int luminanceLevels) { + // TODO: We need a bulk per span equivalent. var vector = sourcePixel.ToVector4(); return ColorNumerics.GetBT709Luminance(ref vector, luminanceLevels); } diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index ab3a1d7603..a24910f9d1 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -141,6 +141,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization /// See: https://github.com/SixLabors/ImageSharp/pull/984 /// /// The pixel type of the image. + /// The test image provider. [Theory] [WithTestPatternImages(110, 110, PixelTypes.Rgb24)] [WithTestPatternImages(170, 170, PixelTypes.Rgb24)] @@ -162,5 +163,35 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization image.CompareToReferenceOutput(ValidatorComparer, provider); } } + + [Theory] + [WithTestPatternImages(5120, 9234, PixelTypes.L16)] + public unsafe void Issue1640(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + // https://github.com/SixLabors/ImageSharp/discussions/1640 + for (int i = 0; i < 2; i++) + { + var options = new HistogramEqualizationOptions + { + Method = HistogramEqualizationMethod.AdaptiveTileInterpolation, + LuminanceLevels = 4096, + ClipHistogram = false, + ClipLimit = 350, + NumberOfTiles = 8 + }; + + Image processed = image.Clone(ctx => + { + ctx.HistogramEqualization(options); + ctx.Resize(image.Width / 4, image.Height / 4, KnownResamplers.Bicubic); + }); + + processed.DebugSave(provider); + processed.CompareToReferenceOutput(ValidatorComparer, provider); + } + } } } diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png new file mode 100644 index 0000000000..b3bbcf7941 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2e6bff82eaedcd43932a5bd11d1feeea2143f00ab2ee5fe0654a403bba9ba2de +size 424844 From 0110ff23ef534b3e278e2deb6e7f7ef9656b49c9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 13:48:10 +0100 Subject: [PATCH 21/48] 64 bit only. Huge image --- .../Processing/Normalization/HistogramEqualizationTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index a24910f9d1..e48504e7bc 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -169,6 +169,11 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization public unsafe void Issue1640(TestImageProvider provider) where TPixel : unmanaged, IPixel { + if (!TestEnvironment.Is64BitProcess) + { + return; + } + using Image image = provider.GetImage(); // https://github.com/SixLabors/ImageSharp/discussions/1640 From 2ce4e2166c7efbd221ed711e73adfe156df91f71 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 14:39:02 +0100 Subject: [PATCH 22/48] Fix using --- .../Processing/Normalization/HistogramEqualizationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index e48504e7bc..902b4086a7 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -188,7 +188,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization NumberOfTiles = 8 }; - Image processed = image.Clone(ctx => + using Image processed = image.Clone(ctx => { ctx.HistogramEqualization(options); ctx.Resize(image.Width / 4, image.Height / 4, KnownResamplers.Bicubic); From 8834788490c1ba053840594b618948875e9b4828 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 14:44:55 +0100 Subject: [PATCH 23/48] Revert unsafe indexer --- .../AdaptiveHistogramEqualizationProcessor{TPixel}.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs index 317db83b44..91ed9f5de4 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs @@ -619,9 +619,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization for (int dx = x; dx < xlimit; dx++) { int luminance = GetLuminance(rowSpan[dx], this.luminanceLevels); - - // This is safe. The index maxes out to the span length. - Unsafe.Add(ref histogramBase, luminance)++; + histogram[luminance]++; } } From a8d269f35ad90b8f613e95573a03f3cc96790933 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 15:20:25 +0100 Subject: [PATCH 24/48] Better test --- .../HistogramEqualizationTests.cs | 41 ++++++++++--------- .../Issue1640_L16_TestPattern5120x9234.png | 3 -- 2 files changed, 22 insertions(+), 22 deletions(-) delete mode 100644 tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index 902b4086a7..85b7530247 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -174,29 +174,32 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization return; } - using Image image = provider.GetImage(); - // https://github.com/SixLabors/ImageSharp/discussions/1640 - for (int i = 0; i < 2; i++) + // Test using isolated memory to ensure clean buffers for reference + provider.Configuration = Configuration.CreateDefaultInstance(); + var options = new HistogramEqualizationOptions { - var options = new HistogramEqualizationOptions - { - Method = HistogramEqualizationMethod.AdaptiveTileInterpolation, - LuminanceLevels = 4096, - ClipHistogram = false, - ClipLimit = 350, - NumberOfTiles = 8 - }; + Method = HistogramEqualizationMethod.AdaptiveTileInterpolation, + LuminanceLevels = 4096, + ClipHistogram = false, + ClipLimit = 350, + NumberOfTiles = 8 + }; - using Image processed = image.Clone(ctx => - { - ctx.HistogramEqualization(options); - ctx.Resize(image.Width / 4, image.Height / 4, KnownResamplers.Bicubic); - }); + using Image image = provider.GetImage(); + using Image referenceResult = image.Clone(ctx => + { + ctx.HistogramEqualization(options); + ctx.Resize(image.Width / 4, image.Height / 4, KnownResamplers.Bicubic); + }); - processed.DebugSave(provider); - processed.CompareToReferenceOutput(ValidatorComparer, provider); - } + using Image processed = image.Clone(ctx => + { + ctx.HistogramEqualization(options); + ctx.Resize(image.Width / 4, image.Height / 4, KnownResamplers.Bicubic); + }); + + ValidatorComparer.VerifySimilarity(referenceResult, processed); } } } diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png deleted file mode 100644 index b3bbcf7941..0000000000 --- a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/Issue1640_L16_TestPattern5120x9234.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:2e6bff82eaedcd43932a5bd11d1feeea2143f00ab2ee5fe0654a403bba9ba2de -size 424844 From 9886969a305511119cdd31bf12376f652b26339e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 28 May 2021 16:24:11 +0100 Subject: [PATCH 25/48] Skip flaky test --- tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs index 9a1d423a6d..a03ceefaff 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs @@ -309,7 +309,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.Equal(values.Entries, actual.Entries); } - [Theory] + [Theory(Skip = "TODO: Too Flaky")] [InlineData(JpegSubsample.Ratio420, 0)] [InlineData(JpegSubsample.Ratio420, 3)] [InlineData(JpegSubsample.Ratio420, 10)] From b6f00c1597ee053f28612b5185b7b1218a4f050d Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 21:27:20 +0100 Subject: [PATCH 26/48] Stop using timing to test cancellation token compliance. --- .../Formats/Jpg/JpegDecoderTests.cs | 83 +++++----- .../Formats/Jpg/JpegEncoderTests.cs | 39 ++--- .../Image/ImageTests.SaveAsync.cs | 20 ++- .../AsyncLocalSwitchableFilesystem.cs | 51 +++++++ .../TestUtilities/PausedStream.cs | 142 ++++++++++++++++++ 5 files changed, 269 insertions(+), 66 deletions(-) create mode 100644 tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs create mode 100644 tests/ImageSharp.Tests/TestUtilities/PausedStream.cs diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 27d70fd188..1b561c20d7 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -12,6 +12,7 @@ using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.Formats.Jpg.Utils; +using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; @@ -126,61 +127,53 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.IsType(ex.InnerException); } - [Theory] - [InlineData(TestImages.Jpeg.Baseline.Jpeg420Small, 0)] - [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 1)] - [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 15)] - [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 30)] - [InlineData(TestImages.Jpeg.Issues.BadRstProgressive518, 1)] - [InlineData(TestImages.Jpeg.Issues.BadRstProgressive518, 15)] - [InlineData(TestImages.Jpeg.Issues.BadRstProgressive518, 30)] - public async Task Decode_IsCancellable(string fileName, int cancellationDelayMs) + [Fact] + public async Task Decode_IsCancellable() { - // Decoding these huge files took 300ms on i7-8650U in 2020. 30ms should be safe for cancellation delay. - string hugeFile = Path.Combine( - TestEnvironment.InputImagesDirectoryFullPath, - fileName); - - const int numberOfRuns = 5; + var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); + using var pausedStream = new PausedStream(file); + var cts = new CancellationTokenSource(); - for (int i = 0; i < numberOfRuns; i++) + var testTask = Task.Run(async () => { - var cts = new CancellationTokenSource(); - if (cancellationDelayMs == 0) - { - cts.Cancel(); - } - else - { - cts.CancelAfter(cancellationDelayMs); - } + AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - try - { - using Image image = await Image.LoadAsync(hugeFile, cts.Token); - } - catch (TaskCanceledException) + return await Assert.ThrowsAsync(async () => { - // Successfully observed a cancellation - return; - } - } + using Image image = await Image.LoadAsync("someFakeFile", cts.Token); + }); + }); + + await pausedStream.FirstWaitReached; + cts.Cancel(); - throw new Exception($"No cancellation happened out of {numberOfRuns} runs!"); + // allow testTask to try and continue now we know we have started but canceled + pausedStream.Release(); + + await testTask; } - [Theory(Skip = "Identify is too fast, doesn't work reliably.")] - [InlineData(TestImages.Jpeg.Baseline.Exif)] - [InlineData(TestImages.Jpeg.Progressive.Bad.ExifUndefType)] - public async Task Identify_IsCancellable(string fileName) + [Fact] + public async Task Identify_IsCancellable() { - string file = Path.Combine( - TestEnvironment.InputImagesDirectoryFullPath, - fileName); - + var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); + using var pausedStream = new PausedStream(file); var cts = new CancellationTokenSource(); - cts.CancelAfter(TimeSpan.FromTicks(1)); - await Assert.ThrowsAsync(() => Image.IdentifyAsync(file, cts.Token)); + + var testTask = Task.Run(async () => + { + AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); + + return await Assert.ThrowsAsync(async () => await Image.IdentifyAsync("someFakeFile", cts.Token)); + }); + + await pausedStream.FirstWaitReached; + cts.Cancel(); + + // allow testTask to try and continue now we know we have started but canceled + pausedStream.Release(); + + await testTask; } // DEBUG ONLY! diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs index 9a1d423a6d..d7e77eabec 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs @@ -13,6 +13,7 @@ using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.Metadata.Profiles.Iptc; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; +using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; @@ -310,28 +311,30 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg } [Theory] - [InlineData(JpegSubsample.Ratio420, 0)] - [InlineData(JpegSubsample.Ratio420, 3)] - [InlineData(JpegSubsample.Ratio420, 10)] - [InlineData(JpegSubsample.Ratio444, 0)] - [InlineData(JpegSubsample.Ratio444, 3)] - [InlineData(JpegSubsample.Ratio444, 10)] - public async Task Encode_IsCancellable(JpegSubsample subsample, int cancellationDelayMs) + [InlineData(JpegSubsample.Ratio420)] + [InlineData(JpegSubsample.Ratio444)] + public async Task Encode_IsCancellable(JpegSubsample subsample) { - using var image = new Image(5000, 5000); - using var stream = new MemoryStream(); + using var pausedStream = new PausedStream(new MemoryStream()); var cts = new CancellationTokenSource(); - if (cancellationDelayMs == 0) - { - cts.Cancel(); - } - else + + var testTask = Task.Run(async () => { - cts.CancelAfter(cancellationDelayMs); - } + using var image = new Image(5000, 5000); + return await Assert.ThrowsAsync(async () => + { + var encoder = new JpegEncoder() { Subsample = subsample }; + await image.SaveAsync(pausedStream, encoder, cts.Token); + }); + }); + + await pausedStream.FirstWaitReached; + cts.Cancel(); + + // allow testTask to try and continue now we know we have started but canceled + pausedStream.Release(); - var encoder = new JpegEncoder() { Subsample = subsample }; - await Assert.ThrowsAsync(() => image.SaveAsync(stream, encoder, cts.Token)); + await testTask; } } } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs index 825bd55e47..f34b74f899 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs @@ -139,11 +139,25 @@ namespace SixLabors.ImageSharp.Tests var encoder = new PngEncoder() { CompressionLevel = PngCompressionLevel.BestCompression }; using var stream = new MemoryStream(); var asyncStream = new AsyncStreamWrapper(stream, () => false); + var pausedStream = new PausedStream(asyncStream); + var cts = new CancellationTokenSource(); - cts.CancelAfter(TimeSpan.FromTicks(1)); - await Assert.ThrowsAnyAsync(() => - image.SaveAsync(asyncStream, encoder, cts.Token)); + var testTask = Task.Run(async () => + { + AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); + + using var image = new Image(5000, 5000); + return await Assert.ThrowsAsync(async () => await image.SaveAsync(pausedStream, encoder, cts.Token)); + }); + + await pausedStream.FirstWaitReached; + cts.Cancel(); + + // allow testTask to try and continue now we know we have started but canceled + pausedStream.Release(); + + await testTask; } } } diff --git a/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs b/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs new file mode 100644 index 0000000000..d53af7a43a --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs @@ -0,0 +1,51 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System.IO; +using System.Threading; +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Tests.TestUtilities +{ + public class AsyncLocalSwitchableFilesystem : IFileSystem + { + private static readonly LocalFileSystem LocalFile = new LocalFileSystem(); + + private static readonly AsyncLocalSwitchableFilesystem Instance = new AsyncLocalSwitchableFilesystem(); + + internal static void ConfigureDefaultFileSystem(IFileSystem fileSystem) + { + Configuration.Default.FileSystem = Instance; + Instance.FileSystem = fileSystem; + } + + internal static void ConfigureFileSystemStream(Stream stream) + { + Configuration.Default.FileSystem = Instance; + Instance.FileSystem = new SingleStreamFileSystem(stream); + } + + private readonly AsyncLocal asyncLocal = new AsyncLocal(); + + private IFileSystem FileSystem + { + get => this.asyncLocal.Value ?? LocalFile; + set => this.asyncLocal.Value = value; + } + + public Stream Create(string path) => this.FileSystem.Create(path); + + public Stream OpenRead(string path) => this.FileSystem.OpenRead(path); + + public class SingleStreamFileSystem : IFileSystem + { + private readonly Stream stream; + + public SingleStreamFileSystem(Stream stream) => this.stream = stream; + + Stream IFileSystem.Create(string path) => this.stream; + + Stream IFileSystem.OpenRead(string path) => this.stream; + } + } +} diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs new file mode 100644 index 0000000000..20c4cf0e35 --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -0,0 +1,142 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace SixLabors.ImageSharp.Tests.TestUtilities +{ + public class PausedStream : Stream + { + private readonly SemaphoreSlim slim = new SemaphoreSlim(0); + + private readonly CancellationTokenSource cancelationTokenSource = new CancellationTokenSource(); + private readonly TaskCompletionSource waitReached = new TaskCompletionSource(); + + private readonly Stream innerStream; + + public Task FirstWaitReached => this.waitReached.Task; + + public void Release() + { + this.slim.Release(); + this.cancelationTokenSource.Cancel(); + } + + public void Next() => this.slim.Release(); + + private void Wait() + { + this.waitReached.TrySetResult(null); + + if (this.cancelationTokenSource.IsCancellationRequested) + { + return; + } + + try + { + this.slim.Wait(this.cancelationTokenSource.Token); + } + catch (OperationCanceledException) + { + // ignore this as its just used to unlock any waits in progress + } + } + + private async Task Await(Func action) + { + await Task.Yield(); + this.Wait(); + await action(); + } + + private async Task Await(Func> action) + { + await Task.Yield(); + this.Wait(); + return await action(); + } + + private T Await(Func action) + { + this.Wait(); + return action(); + } + + private void Await(Action action) + { + this.Wait(); + action(); + } + + public PausedStream(byte[] data) + : this(new MemoryStream(data)) + { + } + + public PausedStream(string filePath) + : this(File.OpenRead(filePath)) + { + } + + public PausedStream(Stream innerStream) => this.innerStream = innerStream; + + public override bool CanTimeout => this.innerStream.CanTimeout; + + public override void Close() => this.Await(() => this.innerStream.Close()); + + public override void CopyTo(Stream destination, int bufferSize) => this.Await(() => this.innerStream.CopyTo(destination, bufferSize)); + + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => this.Await(() => this.innerStream.CopyToAsync(destination, bufferSize, cancellationToken)); + + public override bool CanRead => this.innerStream.CanRead; + + public override bool CanSeek => this.innerStream.CanSeek; + + public override bool CanWrite => this.innerStream.CanWrite; + + public override long Length => this.Await(() => this.innerStream.Length); + + public override long Position { get => this.Await(() => this.innerStream.Position); set => this.Await(() => this.innerStream.Position = value); } + + public override void Flush() => this.Await(() => this.innerStream.Flush()); + + public override int Read(byte[] buffer, int offset, int count) => this.Await(() => this.innerStream.Read(buffer, offset, count)); + + public override long Seek(long offset, SeekOrigin origin) => this.Await(() => this.innerStream.Seek(offset, origin)); + + public override void SetLength(long value) => this.Await(() => this.innerStream.SetLength(value)); + + public override void Write(byte[] buffer, int offset, int count) => this.Await(() => this.innerStream.Write(buffer, offset, count)); + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => this.Await(() => this.innerStream.ReadAsync(buffer, offset, count, cancellationToken)); + + public override int Read(Span buffer) + { + this.Wait(); + return this.innerStream.Read(buffer); + } + + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) => this.Await(() => this.innerStream.ReadAsync(buffer, cancellationToken)); + + public override void Write(ReadOnlySpan buffer) + { + this.Wait(); + this.innerStream.Write(buffer); + } + + public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => this.Await(() => this.innerStream.WriteAsync(buffer, offset, count, cancellationToken)); + + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) => this.Await(() => this.innerStream.WriteAsync(buffer, cancellationToken)); + + public override void WriteByte(byte value) => this.Await(() => this.innerStream.WriteByte(value)); + + public override int ReadByte() => this.Await(() => this.innerStream.ReadByte()); + + protected override void Dispose(bool disposing) => this.innerStream.Dispose(); + public override ValueTask DisposeAsync() => this.innerStream.DisposeAsync(); + } +} From 2a8b1da925f13753f181966895702ac413a64e35 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 21:32:34 +0100 Subject: [PATCH 27/48] style cop my old foe --- tests/ImageSharp.Tests/TestUtilities/PausedStream.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs index 20c4cf0e35..bba4c61dc1 100644 --- a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -137,6 +137,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities public override int ReadByte() => this.Await(() => this.innerStream.ReadByte()); protected override void Dispose(bool disposing) => this.innerStream.Dispose(); + public override ValueTask DisposeAsync() => this.innerStream.DisposeAsync(); } } From 5ccfec917e891e73904f9d541a864ae1ee4f416a Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 21:56:58 +0100 Subject: [PATCH 28/48] allow finer grained details logic on when to release the pause. --- .../Formats/Jpg/JpegDecoderTests.cs | 56 +++++++++---------- .../Formats/Jpg/JpegEncoderTests.cs | 33 ++++++----- .../Image/ImageTests.SaveAsync.cs | 19 ++----- .../TestUtilities/PausedStream.cs | 10 ++-- 4 files changed, 57 insertions(+), 61 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 1b561c20d7..478b0f3ffa 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -127,53 +127,53 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.IsType(ex.InnerException); } - [Fact] - public async Task Decode_IsCancellable() + [Theory] + [InlineData(0)] + [InlineData(0.5)] + [InlineData(0.9)] + public async Task Decode_IsCancellable(int percentageOfStreamReadToCancel) { + var cts = new CancellationTokenSource(); var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); using var pausedStream = new PausedStream(file); - var cts = new CancellationTokenSource(); - - var testTask = Task.Run(async () => + pausedStream.OnWaiting(s => { - AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - - return await Assert.ThrowsAsync(async () => + if (s.Position >= s.Length * percentageOfStreamReadToCancel) { - using Image image = await Image.LoadAsync("someFakeFile", cts.Token); - }); + cts.Cancel(); + pausedStream.Release(); + } + else + { + // allows this/next wait to unblock + pausedStream.Next(); + } }); - await pausedStream.FirstWaitReached; - cts.Cancel(); - - // allow testTask to try and continue now we know we have started but canceled - pausedStream.Release(); + AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - await testTask; + await Assert.ThrowsAsync(async () => + { + using Image image = await Image.LoadAsync("someFakeFile", cts.Token); + }); } [Fact] public async Task Identify_IsCancellable() { - var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); - using var pausedStream = new PausedStream(file); var cts = new CancellationTokenSource(); - var testTask = Task.Run(async () => + var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); + using var pausedStream = new PausedStream(file); + pausedStream.OnWaiting(s => { - AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - - return await Assert.ThrowsAsync(async () => await Image.IdentifyAsync("someFakeFile", cts.Token)); + cts.Cancel(); + pausedStream.Release(); }); - await pausedStream.FirstWaitReached; - cts.Cancel(); - - // allow testTask to try and continue now we know we have started but canceled - pausedStream.Release(); + AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - await testTask; + await Assert.ThrowsAsync(async () => await Image.IdentifyAsync("someFakeFile", cts.Token)); } // DEBUG ONLY! diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs index d7e77eabec..3c48865c71 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs @@ -315,26 +315,29 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [InlineData(JpegSubsample.Ratio444)] public async Task Encode_IsCancellable(JpegSubsample subsample) { - using var pausedStream = new PausedStream(new MemoryStream()); var cts = new CancellationTokenSource(); - - var testTask = Task.Run(async () => + using var pausedStream = new PausedStream(new MemoryStream()); + pausedStream.OnWaiting(s => { - using var image = new Image(5000, 5000); - return await Assert.ThrowsAsync(async () => + // after some writing + if (s.Position >= 500) + { + cts.Cancel(); + pausedStream.Release(); + } + else { - var encoder = new JpegEncoder() { Subsample = subsample }; - await image.SaveAsync(pausedStream, encoder, cts.Token); - }); + // allows this/next wait to unblock + pausedStream.Next(); + } }); - await pausedStream.FirstWaitReached; - cts.Cancel(); - - // allow testTask to try and continue now we know we have started but canceled - pausedStream.Release(); - - await testTask; + using var image = new Image(5000, 5000); + await Assert.ThrowsAsync(async () => + { + var encoder = new JpegEncoder() { Subsample = subsample }; + await image.SaveAsync(pausedStream, encoder, cts.Token); + }); } } } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs index f34b74f899..8bb121349f 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs @@ -139,25 +139,16 @@ namespace SixLabors.ImageSharp.Tests var encoder = new PngEncoder() { CompressionLevel = PngCompressionLevel.BestCompression }; using var stream = new MemoryStream(); var asyncStream = new AsyncStreamWrapper(stream, () => false); - var pausedStream = new PausedStream(asyncStream); - var cts = new CancellationTokenSource(); - var testTask = Task.Run(async () => + var pausedStream = new PausedStream(asyncStream); + pausedStream.OnWaiting(s => { - AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - - using var image = new Image(5000, 5000); - return await Assert.ThrowsAsync(async () => await image.SaveAsync(pausedStream, encoder, cts.Token)); + cts.Cancel(); + pausedStream.Release(); }); - await pausedStream.FirstWaitReached; - cts.Cancel(); - - // allow testTask to try and continue now we know we have started but canceled - pausedStream.Release(); - - await testTask; + await Assert.ThrowsAsync(async () => await image.SaveAsync(pausedStream, encoder, cts.Token)); } } } diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs index bba4c61dc1..c6902b06a2 100644 --- a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -13,11 +13,13 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities private readonly SemaphoreSlim slim = new SemaphoreSlim(0); private readonly CancellationTokenSource cancelationTokenSource = new CancellationTokenSource(); - private readonly TaskCompletionSource waitReached = new TaskCompletionSource(); private readonly Stream innerStream; + private Action onWaitingCallback; - public Task FirstWaitReached => this.waitReached.Task; + public void OnWaiting(Action onWaitingCallback) => this.onWaitingCallback = onWaitingCallback; + + public void OnWaiting(Action onWaitingCallback) => this.OnWaiting(_ => onWaitingCallback()); public void Release() { @@ -29,13 +31,13 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities private void Wait() { - this.waitReached.TrySetResult(null); - if (this.cancelationTokenSource.IsCancellationRequested) { return; } + this.onWaitingCallback?.Invoke(this.innerStream); + try { this.slim.Wait(this.cancelationTokenSource.Token); From 0dd0f0f01ca31e725e47c99cab5e1b94efebdfde Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 22:09:56 +0100 Subject: [PATCH 29/48] skipping `DisposeAsync()` as it not in netcoreapp2.1 its not used anyway in our code so doesn't matter. --- tests/ImageSharp.Tests/TestUtilities/PausedStream.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs index c6902b06a2..5887c533de 100644 --- a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -139,7 +139,5 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities public override int ReadByte() => this.Await(() => this.innerStream.ReadByte()); protected override void Dispose(bool disposing) => this.innerStream.Dispose(); - - public override ValueTask DisposeAsync() => this.innerStream.DisposeAsync(); } } From 654901555d2d79b9d8f18372a296aec23b8638c0 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 22:21:35 +0100 Subject: [PATCH 30/48] skip some overrides on full framework --- .../TestUtilities/PausedStream.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs index 5887c533de..b6ab039b17 100644 --- a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -90,8 +90,6 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities public override void Close() => this.Await(() => this.innerStream.Close()); - public override void CopyTo(Stream destination, int bufferSize) => this.Await(() => this.innerStream.CopyTo(destination, bufferSize)); - public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => this.Await(() => this.innerStream.CopyToAsync(destination, bufferSize, cancellationToken)); public override bool CanRead => this.innerStream.CanRead; @@ -116,6 +114,17 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => this.Await(() => this.innerStream.ReadAsync(buffer, offset, count, cancellationToken)); + public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => this.Await(() => this.innerStream.WriteAsync(buffer, offset, count, cancellationToken)); + + public override void WriteByte(byte value) => this.Await(() => this.innerStream.WriteByte(value)); + + public override int ReadByte() => this.Await(() => this.innerStream.ReadByte()); + + protected override void Dispose(bool disposing) => this.innerStream.Dispose(); + +#if NETCOREAPP + public override void CopyTo(Stream destination, int bufferSize) => this.Await(() => this.innerStream.CopyTo(destination, bufferSize)); + public override int Read(Span buffer) { this.Wait(); @@ -130,14 +139,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities this.innerStream.Write(buffer); } - public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => this.Await(() => this.innerStream.WriteAsync(buffer, offset, count, cancellationToken)); - public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) => this.Await(() => this.innerStream.WriteAsync(buffer, cancellationToken)); - - public override void WriteByte(byte value) => this.Await(() => this.innerStream.WriteByte(value)); - - public override int ReadByte() => this.Await(() => this.innerStream.ReadByte()); - - protected override void Dispose(bool disposing) => this.innerStream.Dispose(); +#endif } } From e66e31947c6c50699e5dcce2263f550494c13e43 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 22:27:56 +0100 Subject: [PATCH 31/48] rename semaphore --- tests/ImageSharp.Tests/TestUtilities/PausedStream.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs index b6ab039b17..4d3646301f 100644 --- a/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs +++ b/tests/ImageSharp.Tests/TestUtilities/PausedStream.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities { public class PausedStream : Stream { - private readonly SemaphoreSlim slim = new SemaphoreSlim(0); + private readonly SemaphoreSlim semaphore = new SemaphoreSlim(0); private readonly CancellationTokenSource cancelationTokenSource = new CancellationTokenSource(); @@ -23,11 +23,11 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities public void Release() { - this.slim.Release(); + this.semaphore.Release(); this.cancelationTokenSource.Cancel(); } - public void Next() => this.slim.Release(); + public void Next() => this.semaphore.Release(); private void Wait() { @@ -40,7 +40,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities try { - this.slim.Wait(this.cancelationTokenSource.Token); + this.semaphore.Wait(this.cancelationTokenSource.Token); } catch (OperationCanceledException) { From 695cfd3ef70e8a4a224999f39941856dd79a1321 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 28 May 2021 22:56:30 +0100 Subject: [PATCH 32/48] use overloads taking configuration --- .../Formats/Jpg/JpegDecoderTests.cs | 11 ++-- .../AsyncLocalSwitchableFilesystem.cs | 51 ------------------- .../TestUtilities/SingleStreamFileSystem.cs | 19 +++++++ 3 files changed, 25 insertions(+), 56 deletions(-) delete mode 100644 tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs create mode 100644 tests/ImageSharp.Tests/TestUtilities/SingleStreamFileSystem.cs diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 478b0f3ffa..67df6a8814 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -150,11 +150,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg } }); - AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); - + var config = Configuration.CreateDefaultInstance(); + config.FileSystem = new SingleStreamFileSystem(pausedStream); await Assert.ThrowsAsync(async () => { - using Image image = await Image.LoadAsync("someFakeFile", cts.Token); + using Image image = await Image.LoadAsync(config, "someFakeFile", cts.Token); }); } @@ -171,9 +171,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg pausedStream.Release(); }); - AsyncLocalSwitchableFilesystem.ConfigureFileSystemStream(pausedStream); + var config = Configuration.CreateDefaultInstance(); + config.FileSystem = new SingleStreamFileSystem(pausedStream); - await Assert.ThrowsAsync(async () => await Image.IdentifyAsync("someFakeFile", cts.Token)); + await Assert.ThrowsAsync(async () => await Image.IdentifyAsync(config, "someFakeFile", cts.Token)); } // DEBUG ONLY! diff --git a/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs b/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs deleted file mode 100644 index d53af7a43a..0000000000 --- a/tests/ImageSharp.Tests/TestUtilities/AsyncLocalSwitchableFilesystem.cs +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -using System.IO; -using System.Threading; -using SixLabors.ImageSharp.IO; - -namespace SixLabors.ImageSharp.Tests.TestUtilities -{ - public class AsyncLocalSwitchableFilesystem : IFileSystem - { - private static readonly LocalFileSystem LocalFile = new LocalFileSystem(); - - private static readonly AsyncLocalSwitchableFilesystem Instance = new AsyncLocalSwitchableFilesystem(); - - internal static void ConfigureDefaultFileSystem(IFileSystem fileSystem) - { - Configuration.Default.FileSystem = Instance; - Instance.FileSystem = fileSystem; - } - - internal static void ConfigureFileSystemStream(Stream stream) - { - Configuration.Default.FileSystem = Instance; - Instance.FileSystem = new SingleStreamFileSystem(stream); - } - - private readonly AsyncLocal asyncLocal = new AsyncLocal(); - - private IFileSystem FileSystem - { - get => this.asyncLocal.Value ?? LocalFile; - set => this.asyncLocal.Value = value; - } - - public Stream Create(string path) => this.FileSystem.Create(path); - - public Stream OpenRead(string path) => this.FileSystem.OpenRead(path); - - public class SingleStreamFileSystem : IFileSystem - { - private readonly Stream stream; - - public SingleStreamFileSystem(Stream stream) => this.stream = stream; - - Stream IFileSystem.Create(string path) => this.stream; - - Stream IFileSystem.OpenRead(string path) => this.stream; - } - } -} diff --git a/tests/ImageSharp.Tests/TestUtilities/SingleStreamFileSystem.cs b/tests/ImageSharp.Tests/TestUtilities/SingleStreamFileSystem.cs new file mode 100644 index 0000000000..ddd1ec7506 --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/SingleStreamFileSystem.cs @@ -0,0 +1,19 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System.IO; +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Tests.TestUtilities +{ + internal class SingleStreamFileSystem : IFileSystem + { + private readonly Stream stream; + + public SingleStreamFileSystem(Stream stream) => this.stream = stream; + + Stream IFileSystem.Create(string path) => this.stream; + + Stream IFileSystem.OpenRead(string path) => this.stream; + } +} From 881bb51f217bc722fd847f1c3fe1357b90b8f90f Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 1 Jun 2021 01:12:44 +0200 Subject: [PATCH 33/48] Make sure encoding 4bit paletted tiff rows are byte aligned --- .../Formats/Tiff/TiffDecoderCore.cs | 2 +- .../Tiff/Writers/TiffPaletteWriter{TPixel}.cs | 33 ++++++++++++++----- .../Formats/Tiff/TiffDecoderTests.cs | 23 +++++++++++-- .../Formats/Tiff/TiffEncoderTests.cs | 6 ++-- tests/ImageSharp.Tests/TestImages.cs | 2 ++ .../Input/Tiff/flower-minisblack-04.tiff | 3 ++ .../Images/Input/Tiff/flower-palette-04.tiff | 3 ++ 7 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 tests/Images/Input/Tiff/flower-minisblack-04.tiff create mode 100644 tests/Images/Input/Tiff/flower-palette-04.tiff diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index b7b7640072..50882c0072 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -202,7 +202,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff if (this.PlanarConfiguration == TiffPlanarConfiguration.Chunky) { - DebugGuard.IsTrue(plane == -1, "Excepted Chunky planar."); + DebugGuard.IsTrue(plane == -1, "Expected Chunky planar."); bitsPerPixel = this.BitsPerPixel; } else diff --git a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs index 712578f81a..fe614c55ed 100644 --- a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs @@ -55,23 +55,38 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers /// protected override void EncodeStrip(int y, int height, TiffBaseCompressor compressor) { - Span pixels = GetStripPixels(((IPixelSource)this.quantizedImage).PixelBuffer, y, height); + Span indexedPixels = GetStripPixels(((IPixelSource)this.quantizedImage).PixelBuffer, y, height); if (this.BitsPerPixel == 4) { - using IMemoryOwner rows4bitBuffer = this.MemoryAllocator.Allocate(pixels.Length / 2); + int width = this.Image.Width; + int excess = (width % 2) * height; + int rows4BitBufferLength = indexedPixels.Length + excess; + using IMemoryOwner rows4bitBuffer = this.MemoryAllocator.Allocate(rows4BitBufferLength); Span rows4bit = rows4bitBuffer.GetSpan(); - int idx = 0; - for (int i = 0; i < rows4bit.Length; i++) + int idxPixels = 0; + int idx4bitRows = 0; + int halfWidth = width / 2; + for (int row = 0; row < height; row++) { - rows4bit[i] = (byte)((pixels[idx] << 4) | (pixels[idx + 1] & 0xF)); - idx += 2; + for (int x = 0; x < halfWidth; x++) + { + rows4bit[idx4bitRows] = (byte)((indexedPixels[idxPixels] << 4) | (indexedPixels[idxPixels + 1] & 0xF)); + idxPixels += 2; + idx4bitRows++; + } + + // Make sure rows are byte-aligned. + if (width % 2 != 0) + { + rows4bit[idx4bitRows++] = (byte)(indexedPixels[idxPixels++] << 4); + } } - compressor.CompressStrip(rows4bit, height); + compressor.CompressStrip(rows4bit.Slice(0, idx4bitRows), height); } else { - compressor.CompressStrip(pixels, height); + compressor.CompressStrip(indexedPixels, height); } } @@ -91,7 +106,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers PixelOperations.Instance.ToRgb48(this.Configuration, quantizedColors, quantizedColorRgb48); // It can happen that the quantized colors are less than the expected maximum per channel. - var diffToMaxColors = this.maxColors - quantizedColors.Length; + int diffToMaxColors = this.maxColors - quantizedColors.Length; // In a TIFF ColorMap, all the Red values come first, followed by the Green values, // then the Blue values. Convert the quantized palette to this format. diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index c35311a2a8..1a72046fb3 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -4,7 +4,7 @@ // ReSharper disable InconsistentNaming using System; using System.IO; - +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Tiff; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; @@ -37,6 +37,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [InlineData(RgbUncompressed, 24, 256, 256, 300, 300, PixelResolutionUnit.PixelsPerInch)] [InlineData(SmallRgbDeflate, 24, 32, 32, 96, 96, PixelResolutionUnit.PixelsPerInch)] [InlineData(Calliphora_GrayscaleUncompressed, 8, 804, 1198, 96, 96, PixelResolutionUnit.PixelsPerInch)] + [InlineData(Flower4BitPalette, 4, 73, 43, 72, 72, PixelResolutionUnit.PixelsPerInch)] public void Identify(string imagePath, int expectedPixelSize, int expectedWidth, int expectedHeight, double expectedHResolution, double expectedVResolution, PixelResolutionUnit expectedResolutionUnit) { var testFile = TestFile.Create(imagePath); @@ -91,6 +92,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_WithPalette(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] + [WithFile(Rgb4BitPalette, PixelTypes.Rgba32)] + [WithFile(Flower4BitPalette, PixelTypes.Rgba32)] + [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_4Bit_WithPalette(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + if (TestEnvironment.IsWindows) + { + TestTiffDecoder(provider, new SystemDrawingReferenceDecoder(), useExactComparer: false, 0.01f); + } + } + [Theory] [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)] [WithFile(RgbDeflateMultistrip, PixelTypes.Rgba32)] @@ -155,12 +169,15 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff image.CompareToOriginalMultiFrame(provider, ImageComparer.Exact, ReferenceDecoder); } - private static void TestTiffDecoder(TestImageProvider provider) + private static void TestTiffDecoder(TestImageProvider provider, IImageDecoder referenceDecoder = null, bool useExactComparer = true, float compareTolerance = 0.001f) where TPixel : unmanaged, IPixel { using Image image = provider.GetImage(TiffDecoder); image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact, ReferenceDecoder); + image.CompareToOriginal( + provider, + useExactComparer ? ImageComparer.Exact : ImageComparer.Tolerant(compareTolerance), + referenceDecoder ?? ReferenceDecoder); } } } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 546508ca54..105514c982 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -296,10 +296,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [Theory] [WithFile(Rgb4BitPalette, PixelTypes.Rgba32)] + [WithFile(Flower4BitPalette, PixelTypes.Rgba32)] + [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)] public void TiffEncoder_EncodeColorPalette_With4Bit_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel => //// Note: The magick reference decoder does not support 4 bit tiff's, so we use our TIFF decoder instead. - TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.001f, imageDecoder: new TiffDecoder()); + TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.003f, imageDecoder: new TiffDecoder()); [Theory] [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)] @@ -460,7 +462,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff TiffCompression compression = TiffCompression.None, TiffPredictor predictor = TiffPredictor.None, bool useExactComparer = true, - float compareTolerance = 0.01f, + float compareTolerance = 0.001f, IImageDecoder imageDecoder = null) where TPixel : unmanaged, IPixel { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 49d0e759cd..09394d4ea0 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -558,6 +558,8 @@ namespace SixLabors.ImageSharp.Tests public const string RgbPalette = "Tiff/rgb_palette.tiff"; public const string Rgb4BitPalette = "Tiff/bike_colorpalette_4bit.tiff"; public const string RgbPaletteDeflate = "Tiff/rgb_palette_deflate.tiff"; + public const string Flower4BitPalette = "Tiff/flower-palette-04.tiff"; + public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; diff --git a/tests/Images/Input/Tiff/flower-minisblack-04.tiff b/tests/Images/Input/Tiff/flower-minisblack-04.tiff new file mode 100644 index 0000000000..e6d1e13360 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-minisblack-04.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:18991fca75a89b3d15c7f93dee0454e3943920b595ba16145ebc1fd8bd45b1f5 +size 1905 diff --git a/tests/Images/Input/Tiff/flower-palette-04.tiff b/tests/Images/Input/Tiff/flower-palette-04.tiff new file mode 100644 index 0000000000..8594a0b00a --- /dev/null +++ b/tests/Images/Input/Tiff/flower-palette-04.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:700ec8103b4197c415ba90d983a7d5f471f155fd5b1c952d86ee9becba898a1a +size 2010 From e8a9e54eef0582ab728bffe9f100d4c52dec2403 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 1 Jun 2021 11:14:59 +0200 Subject: [PATCH 34/48] Fix length of 4 bit row buffer --- .../Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs index fe614c55ed..b851122a63 100644 --- a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers { int width = this.Image.Width; int excess = (width % 2) * height; - int rows4BitBufferLength = indexedPixels.Length + excess; + int rows4BitBufferLength = (width / 2 * height) + excess; using IMemoryOwner rows4bitBuffer = this.MemoryAllocator.Allocate(rows4BitBufferLength); Span rows4bit = rows4bitBuffer.GetSpan(); int idxPixels = 0; From d5c5d678ab5213559acc1d6024d954e544679e3a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 1 Jun 2021 16:23:28 +0200 Subject: [PATCH 35/48] Review changes --- .../Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs index b851122a63..d1a3dd1ea3 100644 --- a/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs @@ -59,13 +59,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers if (this.BitsPerPixel == 4) { int width = this.Image.Width; - int excess = (width % 2) * height; - int rows4BitBufferLength = (width / 2 * height) + excess; + int halfWidth = width >> 1; + int excess = (width & 1) * height; // (width % 2) * height + int rows4BitBufferLength = (halfWidth * height) + excess; using IMemoryOwner rows4bitBuffer = this.MemoryAllocator.Allocate(rows4BitBufferLength); Span rows4bit = rows4bitBuffer.GetSpan(); int idxPixels = 0; int idx4bitRows = 0; - int halfWidth = width / 2; for (int row = 0; row < height; row++) { for (int x = 0; x < halfWidth; x++) From c6f5a8aaa01369794eac5c4958718f5d6f018595 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 1 Jun 2021 23:48:48 +0200 Subject: [PATCH 36/48] Add support for decoding 12 bits per pixel tiff's --- .../Formats/Tiff/Constants/TiffConstants.cs | 7 ++- .../Rgb444TiffColor{TPixel}.cs | 60 +++++++++++++++++++ .../RgbPlanarTiffColor{TPixel}.cs | 1 - .../TiffColorDecoderFactory{TPixel}.cs | 10 ++++ .../TiffColorType.cs | 5 ++ .../Formats/Tiff/TiffBitsPerPixel.cs | 7 +++ .../Formats/Tiff/TiffBitsPerSample.cs | 5 ++ .../Tiff/TiffBitsPerSampleExtensions.cs | 9 +++ .../Formats/Tiff/TiffDecoderCore.cs | 2 +- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 17 +++++- .../Formats/Tiff/TiffEncoderCore.cs | 6 +- .../Tiff/TiffEncoderEntriesCollector.cs | 11 +++- .../Formats/Tiff/TiffDecoderTests.cs | 12 ++++ .../Formats/Tiff/TiffMetadataTests.cs | 5 +- tests/ImageSharp.Tests/TestImages.cs | 2 + .../Input/Tiff/flower-rgb-contig-04.tiff | 3 + .../Input/Tiff/flower-rgb-planar-04.tiff | 3 + 17 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 src/ImageSharp/Formats/Tiff/PhotometricInterpretation/Rgb444TiffColor{TPixel}.cs create mode 100644 tests/Images/Input/Tiff/flower-rgb-contig-04.tiff create mode 100644 tests/Images/Input/Tiff/flower-rgb-planar-04.tiff diff --git a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs index a30890a69e..988b1242ae 100644 --- a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs +++ b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs @@ -96,10 +96,15 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Constants public static readonly ushort[] BitsPerSample8Bit = { 8 }; /// - /// The bits per sample for images with 8 bits for each color channel. + /// The bits per sample for color images with 8 bits for each color channel. /// public static readonly ushort[] BitsPerSampleRgb8Bit = { 8, 8, 8 }; + /// + /// The bits per sample for color images with 4 bits for each color channel. + /// + public static readonly ushort[] BitsPerSampleRgb4Bit = { 4, 4, 4 }; + /// /// The list of mimetypes that equate to a tiff. /// diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/Rgb444TiffColor{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/Rgb444TiffColor{TPixel}.cs new file mode 100644 index 0000000000..d8c48942f8 --- /dev/null +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/Rgb444TiffColor{TPixel}.cs @@ -0,0 +1,60 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; + +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation +{ + /// + /// Implements the 'RGB' photometric interpretation for 4 bits per color channel images. + /// + internal class Rgb444TiffColor : TiffBaseColorDecoder + where TPixel : unmanaged, IPixel + { + /// + public override void Decode(ReadOnlySpan data, Buffer2D pixels, int left, int top, int width, int height) + { + var color = default(TPixel); + + int offset = 0; + + var bgra = default(Bgra4444); + for (int y = top; y < top + height; y++) + { + Span pixelRow = pixels.GetRowSpan(y); + + for (int x = left; x < left + width; x += 2) + { + byte r = (byte)((data[offset] & 0xF0) >> 4); + byte g = (byte)(data[offset] & 0xF); + offset++; + byte b = (byte)((data[offset] & 0xF0) >> 4); + + bgra.PackedValue = ToBgraPackedValue(b, g, r); + color.FromScaledVector4(bgra.ToScaledVector4()); + pixelRow[x] = color; + if (x + 1 >= pixelRow.Length) + { + offset++; + break; + } + + r = (byte)(data[offset] & 0xF); + offset++; + g = (byte)((data[offset] & 0xF0) >> 4); + b = (byte)(data[offset] & 0xF); + offset++; + + bgra.PackedValue = ToBgraPackedValue(b, g, r); + color.FromScaledVector4(bgra.ToScaledVector4()); + pixelRow[x + 1] = color; + } + } + } + + private static ushort ToBgraPackedValue(byte b, byte g, byte r) => (ushort)(b | (g << 4) | (r << 8) | (0xF << 12)); + } +} diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/RgbPlanarTiffColor{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/RgbPlanarTiffColor{TPixel}.cs index e45dd44bdc..b40158fcee 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/RgbPlanarTiffColor{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/RgbPlanarTiffColor{TPixel}.cs @@ -27,7 +27,6 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation private readonly ushort bitsPerSampleB; public RgbPlanarTiffColor(ushort[] bitsPerSample) - /* : base(bitsPerSample, null) */ { this.bitsPerSampleR = bitsPerSample[0]; this.bitsPerSampleG = bitsPerSample[1]; diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs index 0a7941dfbc..d78b06aa7f 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs @@ -57,6 +57,16 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample); + case TiffColorType.Rgb444: + DebugGuard.IsTrue( + bitsPerSample.Length == 3 + && bitsPerSample[0] == 4 + && bitsPerSample[1] == 4 + && bitsPerSample[2] == 4, + "bitsPerSample"); + DebugGuard.IsTrue(colorMap == null, "colorMap"); + return new Rgb444TiffColor(); + case TiffColorType.Rgb888: DebugGuard.IsTrue( bitsPerSample.Length == 3 diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs index 484d231633..089dc31ade 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs @@ -63,6 +63,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation /// Rgb888, + /// + /// RGB color image with 4 bits for each channel. + /// + Rgb444, + /// /// RGB Full Color. Planar configuration of data. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs index 35a9a590bb..289637fc32 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs @@ -23,6 +23,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Bit8 = 8, + /// + /// 14 bits per pixel. 4 bit for each color channel. + /// + /// Note: The TiffEncoder does not yet support 4 bits per color channel and will default to 24 bits per pixel. + /// + Bit12 = 12, + /// /// 24 bits per pixel. One byte for each color channel. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs index bc74cbc5fb..992a5ad6eb 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs @@ -28,6 +28,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Bit8 = 8, + /// + /// Twelve bits per sample, each channel has 4 bits. + /// + Bit12 = 12, + /// /// 24 bits per sample, each color channel has 8 Bits. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 5c4c374bef..32ef547ba4 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -23,6 +23,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffConstants.BitsPerSample4Bit; case TiffBitsPerSample.Bit8: return TiffConstants.BitsPerSample8Bit; + case TiffBitsPerSample.Bit12: + return TiffConstants.BitsPerSampleRgb4Bit; case TiffBitsPerSample.Bit24: return TiffConstants.BitsPerSampleRgb8Bit; @@ -48,6 +50,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffBitsPerSample.Bit24; } + if (bitsPerSample[0] == TiffConstants.BitsPerSampleRgb4Bit[0] && + bitsPerSample[1] == TiffConstants.BitsPerSampleRgb4Bit[1] && + bitsPerSample[2] == TiffConstants.BitsPerSampleRgb4Bit[2]) + { + return TiffBitsPerSample.Bit12; + } + break; case 1: diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index 50882c0072..fadb4f7c2e 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -294,7 +294,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff int top = rowsPerStrip * stripIndex; if (top + stripHeight > frame.Height) { - // Make sure we ignore any strips that are not needed for the image (if too many are present) + // Make sure we ignore any strips that are not needed for the image (if too many are present). break; } diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index b5f3e7cf1e..1b48cd08f6 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.Linq; using SixLabors.ImageSharp.Formats.Tiff.Compression; using SixLabors.ImageSharp.Formats.Tiff.Constants; @@ -179,7 +180,18 @@ namespace SixLabors.ImageSharp.Formats.Tiff if (options.PlanarConfiguration == TiffPlanarConfiguration.Chunky) { - options.ColorType = options.BitsPerSample == TiffBitsPerSample.Bit24 ? TiffColorType.Rgb888 : TiffColorType.Rgb; + switch (options.BitsPerSample) + { + case TiffBitsPerSample.Bit24: + options.ColorType = TiffColorType.Rgb888; + break; + case TiffBitsPerSample.Bit12: + options.ColorType = TiffColorType.Rgb444; + break; + default: + TiffThrowHelper.ThrowNotSupported("Bits per sample is nut supported."); + break; + } } else { @@ -274,8 +286,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff TiffBitsPerPixel.Bit1 => TiffBitsPerSample.Bit1, TiffBitsPerPixel.Bit4 => TiffBitsPerSample.Bit4, TiffBitsPerPixel.Bit8 => TiffBitsPerSample.Bit8, + TiffBitsPerPixel.Bit12 => TiffBitsPerSample.Bit12, TiffBitsPerPixel.Bit24 => TiffBitsPerSample.Bit24, - _ => TiffBitsPerSample.Bit24, + _ => throw new NotSupportedException("The bits per pixel are not supported"), }; } } diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index 74c516f63b..6b5ca0086f 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -306,7 +306,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff case TiffBitsPerPixel.Bit1: if (compression == TiffCompression.Ccitt1D || compression == TiffCompression.CcittGroup3Fax || compression == TiffCompression.CcittGroup4Fax) { - // The normal PhotometricInterpretation for bilevel CCITT compressed data is WhiteIsZero. + // The “normal” PhotometricInterpretation for bilevel CCITT compressed data is WhiteIsZero. this.SetEncoderOptions(bitsPerPixel, TiffPhotometricInterpretation.WhiteIsZero, compression, TiffPredictor.None); break; } @@ -319,6 +319,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff case TiffBitsPerPixel.Bit8: this.SetEncoderOptions(bitsPerPixel, photometricInterpretation ?? TiffPhotometricInterpretation.BlackIsZero, compression, predictor); break; + case TiffBitsPerPixel.Bit12: + // Encoding 12 bits per pixel is not yet supported. Default to 24 bits. + this.SetEncoderOptions(TiffBitsPerPixel.Bit24, TiffPhotometricInterpretation.Rgb, compression, TiffPredictor.None); + break; default: this.SetEncoderOptions(bitsPerPixel, TiffPhotometricInterpretation.Rgb, compression, predictor); break; diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs index 09605bc690..9bc0792c40 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs @@ -66,8 +66,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff Value = SoftwareValue }; - this.collector.Add(width); - this.collector.Add(height); + this.collector.AddOrReplace(width); + this.collector.AddOrReplace(height); this.ProcessResolution(image.Metadata, rootFrameExifProfile); this.ProcessProfiles(image.Metadata, rootFrameExifProfile, rootFrameXmpBytes); @@ -227,7 +227,6 @@ namespace SixLabors.ImageSharp.Formats.Tiff exifProfile.RemoveValue(ExifTag.IccProfile); } - TiffMetadata tiffMetadata = imageMetadata.GetTiffMetadata(); if (xmpProfile != null) { var xmp = new ExifByteArray(ExifTagValue.XMP, ExifDataType.Byte) @@ -252,6 +251,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff public void Process(TiffEncoderCore encoder) { + var planarConfig = new ExifShort(ExifTagValue.PlanarConfiguration) + { + Value = (ushort)TiffPlanarConfiguration.Chunky + }; + var samplesPerPixel = new ExifLong(ExifTagValue.SamplesPerPixel) { Value = GetSamplesPerPixel(encoder) @@ -274,6 +278,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff Value = (ushort)encoder.PhotometricInterpretation }; + this.collector.AddOrReplace(planarConfig); this.collector.AddOrReplace(samplesPerPixel); this.collector.AddOrReplace(bitPerSample); this.collector.AddOrReplace(compression); diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 1a72046fb3..2144ddfdb3 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -105,6 +105,18 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff } } + [Theory] + [WithFile(FlowerRgb444Contiguous, PixelTypes.Rgba32)] + [WithFile(FlowerRgb444Planar, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_12Bit(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + if (TestEnvironment.IsWindows) + { + TestTiffDecoder(provider, new SystemDrawingReferenceDecoder()); + } + } + [Theory] [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)] [WithFile(RgbDeflateMultistrip, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 3aded7b0e3..68244b3b12 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -288,10 +288,13 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal("This is Изготовитель камеры", exifProfileInput.GetValue(ExifTag.Make).Value); Assert.Equal("This is Авторские права", exifProfileInput.GetValue(ExifTag.Copyright).Value); - Assert.Equal(exifProfileInput.Values.Count, encodedImageExifProfile.Values.Count); Assert.Equal(exifProfileInput.GetValue(ExifTag.ImageDescription).Value, encodedImageExifProfile.GetValue(ExifTag.ImageDescription).Value); Assert.Equal(exifProfileInput.GetValue(ExifTag.Make).Value, encodedImageExifProfile.GetValue(ExifTag.Make).Value); Assert.Equal(exifProfileInput.GetValue(ExifTag.Copyright).Value, encodedImageExifProfile.GetValue(ExifTag.Copyright).Value); + + // Note that the encoded profile has PlanarConfiguration explicitly set, which is missing in the original image profile. + Assert.Equal((ushort)TiffPlanarConfiguration.Chunky, encodedImageExifProfile.GetValue(ExifTag.PlanarConfiguration).Value); + Assert.Equal(exifProfileInput.Values.Count + 1, encodedImageExifProfile.Values.Count); } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 09394d4ea0..d1c29489fb 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -560,6 +560,8 @@ namespace SixLabors.ImageSharp.Tests public const string RgbPaletteDeflate = "Tiff/rgb_palette_deflate.tiff"; public const string Flower4BitPalette = "Tiff/flower-palette-04.tiff"; public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff"; + public const string FlowerRgb444Contiguous = "Tiff/flower-rgb-contig-04.tiff"; + public const string FlowerRgb444Planar = "Tiff/flower-rgb-planar-04.tiff"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; diff --git a/tests/Images/Input/Tiff/flower-rgb-contig-04.tiff b/tests/Images/Input/Tiff/flower-rgb-contig-04.tiff new file mode 100644 index 0000000000..d9a141f29a --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-contig-04.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:96c4c1dfc23a0d9e5c6189717647fa117b08aac9a40c63e3945d3e674df4c3c6 +size 5049 diff --git a/tests/Images/Input/Tiff/flower-rgb-planar-04.tiff b/tests/Images/Input/Tiff/flower-rgb-planar-04.tiff new file mode 100644 index 0000000000..7a2270e486 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-planar-04.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ca4434aa1a8c52654b20596c7c428c9016e089de75c29dc6ddcd32708874005c +size 5117 From bc723d308bce60736b7de8041547ef7bb7fc61f9 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 2 Jun 2021 10:38:54 +0200 Subject: [PATCH 37/48] Add 4 bit and 2 bit depth to the valid bit depth for the magick reference decoder --- .../TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index 4e2866be1f..885d12e774 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs MemoryGroup framePixels = frame.PixelBuffer.FastMemoryGroup; using IUnsafePixelCollection pixels = magicFrame.GetPixelsUnsafe(); - if (magicFrame.Depth == 8 || magicFrame.Depth == 1) + if (magicFrame.Depth == 8 || magicFrame.Depth == 4 || magicFrame.Depth == 2 || magicFrame.Depth == 1) { byte[] data = pixels.ToByteArray(PixelMapping.RGBA); From 5e0f75f1197e256b03de24dcb5834bb7470397d1 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 2 Jun 2021 10:39:13 +0200 Subject: [PATCH 38/48] Dont skip tests on linux, use magick decoder --- .../Formats/Tiff/TiffDecoderTests.cs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 2144ddfdb3..c58977813a 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -97,25 +97,13 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [WithFile(Flower4BitPalette, PixelTypes.Rgba32)] [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_4Bit_WithPalette(TestImageProvider provider) - where TPixel : unmanaged, IPixel - { - if (TestEnvironment.IsWindows) - { - TestTiffDecoder(provider, new SystemDrawingReferenceDecoder(), useExactComparer: false, 0.01f); - } - } + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, ReferenceDecoder, useExactComparer: false, 0.01f); [Theory] [WithFile(FlowerRgb444Contiguous, PixelTypes.Rgba32)] [WithFile(FlowerRgb444Planar, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_12Bit(TestImageProvider provider) - where TPixel : unmanaged, IPixel - { - if (TestEnvironment.IsWindows) - { - TestTiffDecoder(provider, new SystemDrawingReferenceDecoder()); - } - } + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); [Theory] [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)] From cc081b0de7071c84d984318078cc4de8c1321529 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 2 Jun 2021 14:09:32 +0200 Subject: [PATCH 39/48] Use magick decoder for 4bit test, add test for encoding option with 12 bpp --- .../Formats/Tiff/TiffEncoderTests.cs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 105514c982..61bccc008d 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -77,6 +77,26 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal(TiffCompression.None, frameMetaData.Compression); } + [Theory] + [InlineData(TiffBitsPerPixel.Bit12)] + public void EncoderOptions_UnsupportedBitPerPixel_DefaultTo24Bits(TiffBitsPerPixel bitsPerPixel) + { + // arrange + var tiffEncoder = new TiffEncoder { BitsPerPixel = bitsPerPixel }; + using Image input = new Image(10, 10); + using var memStream = new MemoryStream(); + + // act + input.Save(memStream, tiffEncoder); + + // assert + memStream.Position = 0; + using var output = Image.Load(memStream); + + TiffFrameMetadata frameMetaData = output.Frames.RootFrame.Metadata.GetTiffMetadata(); + Assert.Equal(TiffBitsPerPixel.Bit24, frameMetaData.BitsPerPixel); + } + [Theory] [InlineData(null, TiffCompression.Deflate, TiffBitsPerPixel.Bit24, TiffCompression.Deflate)] [InlineData(TiffPhotometricInterpretation.Rgb, TiffCompression.Deflate, TiffBitsPerPixel.Bit24, TiffCompression.Deflate)] @@ -300,8 +320,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)] public void TiffEncoder_EncodeColorPalette_With4Bit_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel => - //// Note: The magick reference decoder does not support 4 bit tiff's, so we use our TIFF decoder instead. - TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.003f, imageDecoder: new TiffDecoder()); + TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.003f); [Theory] [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)] From 65808ae55f1bc069434bacbf1964895720b9b1d0 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 2 Jun 2021 15:34:39 +0100 Subject: [PATCH 40/48] Fix throwhelper --- src/ImageSharp/Image.cs | 10 +++++++--- src/ImageSharp/ImageFrameCollection.cs | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index fe72ec5c00..22b9ce8e5c 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using SixLabors.ImageSharp.Advanced; @@ -149,9 +150,9 @@ namespace SixLabors.ImageSharp protected void UpdateSize(Size size) => this.size = size; /// - /// Internal routine for freeing managed resources called from + /// Disposes the object and frees resources for the Garbage Collector. /// - /// /// Whether to dispose of managed objects. + /// Whether to dispose of managed and unmanaged objects. protected abstract void Dispose(bool disposing); /// @@ -161,7 +162,7 @@ namespace SixLabors.ImageSharp { if (this.isDisposed) { - ThrowHelper.ThrowObjectDisposedException(this.GetType()); + ThrowObjectDisposedException(this.GetType()); } } @@ -182,6 +183,9 @@ namespace SixLabors.ImageSharp /// The token to monitor for cancellation requests. internal abstract Task AcceptAsync(IImageVisitorAsync visitor, CancellationToken cancellationToken); + [MethodImpl(InliningOptions.ColdPath)] + private static void ThrowObjectDisposedException(Type type) => throw new ObjectDisposedException(type.Name); + private class EncodeVisitor : IImageVisitor, IImageVisitorAsync { private readonly IImageEncoder encoder; diff --git a/src/ImageSharp/ImageFrameCollection.cs b/src/ImageSharp/ImageFrameCollection.cs index 8c8edcd7a5..07ba8c87f3 100644 --- a/src/ImageSharp/ImageFrameCollection.cs +++ b/src/ImageSharp/ImageFrameCollection.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp { @@ -198,14 +199,14 @@ namespace SixLabors.ImageSharp { if (this.isDisposed) { - ThrowHelper.ThrowObjectDisposedException(this.GetType()); + ThrowObjectDisposedException(this.GetType()); } } /// - /// Internal routine for freeing managed resources called from + /// Disposes the object and frees resources for the Garbage Collector. /// - /// /// /// Whether to dispose of managed objects. + /// Whether to dispose of managed and unmanaged objects. protected abstract void Dispose(bool disposing); /// @@ -262,5 +263,8 @@ namespace SixLabors.ImageSharp /// The background color. /// The new frame. protected abstract ImageFrame NonGenericCreateFrame(Color backgroundColor); + + [MethodImpl(InliningOptions.ColdPath)] + private static void ThrowObjectDisposedException(Type type) => throw new ObjectDisposedException(type.Name); } } From afee88123c36c11b506673d709d11db366c0e18c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 2 Jun 2021 17:17:19 +0100 Subject: [PATCH 41/48] Make frames resonly --- src/ImageSharp/Image.cs | 2 +- src/ImageSharp/Image{TPixel}.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 22b9ce8e5c..ce6aa69b58 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -99,7 +99,7 @@ namespace SixLabors.ImageSharp /// /// The stream to save the image to. /// The encoder to save the image with. - /// Thrown if the stream or encoder is null. + /// Thrown if the stream or encoder is null. public void Save(Stream stream, IImageEncoder encoder) { Guard.NotNull(stream, nameof(stream)); diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index e42022729f..b43ff0422b 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -23,7 +23,7 @@ namespace SixLabors.ImageSharp public sealed class Image : Image where TPixel : unmanaged, IPixel { - private ImageFrameCollection frames; + private readonly ImageFrameCollection frames; /// /// Initializes a new instance of the class From 85ef0fe2ca9e674abea787b7d9f8258b7d257e6e Mon Sep 17 00:00:00 2001 From: Brian Popow <38701097+brianpopow@users.noreply.github.com> Date: Wed, 2 Jun 2021 20:25:08 +0200 Subject: [PATCH 42/48] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs | 4 ++-- src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 32ef547ba4..4910cf9527 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -50,9 +50,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffBitsPerSample.Bit24; } - if (bitsPerSample[0] == TiffConstants.BitsPerSampleRgb4Bit[0] && + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb4Bit[2] && bitsPerSample[1] == TiffConstants.BitsPerSampleRgb4Bit[1] && - bitsPerSample[2] == TiffConstants.BitsPerSampleRgb4Bit[2]) + bitsPerSample[0] == TiffConstants.BitsPerSampleRgb4Bit[0]) { return TiffBitsPerSample.Bit12; } diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 1b48cd08f6..b38ff68c18 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -189,7 +189,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff options.ColorType = TiffColorType.Rgb444; break; default: - TiffThrowHelper.ThrowNotSupported("Bits per sample is nut supported."); + TiffThrowHelper.ThrowNotSupported("Bits per sample is not supported."); break; } } From 3a6a5e9201a7ba00bfc57e8ed4ba6fd732518286 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 2 Jun 2021 20:27:08 +0200 Subject: [PATCH 43/48] Flip order of comparing BitsPerSample --- src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 4910cf9527..0687b0104a 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -43,9 +43,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff switch (bitsPerSample.Length) { case 3: - if (bitsPerSample[0] == TiffConstants.BitsPerSampleRgb8Bit[0] && + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb8Bit[2] && bitsPerSample[1] == TiffConstants.BitsPerSampleRgb8Bit[1] && - bitsPerSample[2] == TiffConstants.BitsPerSampleRgb8Bit[2]) + bitsPerSample[0] == TiffConstants.BitsPerSampleRgb8Bit[0]) { return TiffBitsPerSample.Bit24; } From 580723fc0aac40443d52a4636acadeb7b3e9b000 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 3 Jun 2021 10:34:52 +0200 Subject: [PATCH 44/48] Add test for encode and reload planar tiff --- src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs | 2 +- tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs | 6 ++++++ tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs index 289637fc32..4b65f88b8c 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs @@ -24,7 +24,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff Bit8 = 8, /// - /// 14 bits per pixel. 4 bit for each color channel. + /// 12 bits per pixel. 4 bit for each color channel. /// /// Note: The TiffEncoder does not yet support 4 bits per color channel and will default to 24 bits per pixel. /// diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 61bccc008d..dd3ef133e6 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -248,6 +248,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal(expectedCompression, frameMetaData.Compression); } + // This makes sure, that when decoding a planar tiff, the planar configuration is not carried over to the encoded image. + [Theory] + [WithFile(FlowerRgb444Planar, PixelTypes.Rgba32)] + public void TiffEncoder_EncodePlanar_AndReload_Works(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit24, TiffPhotometricInterpretation.Rgb, imageDecoder: new TiffDecoder()); + [Theory] [WithFile(Calliphora_RgbUncompressed, PixelTypes.Rgba32)] public void TiffEncoder_EncodeRgb_Works(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 68244b3b12..ab350f720e 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -293,7 +293,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal(exifProfileInput.GetValue(ExifTag.Copyright).Value, encodedImageExifProfile.GetValue(ExifTag.Copyright).Value); // Note that the encoded profile has PlanarConfiguration explicitly set, which is missing in the original image profile. - Assert.Equal((ushort)TiffPlanarConfiguration.Chunky, encodedImageExifProfile.GetValue(ExifTag.PlanarConfiguration).Value); + Assert.Equal((ushort)TiffPlanarConfiguration.Chunky, encodedImageExifProfile.GetValue(ExifTag.PlanarConfiguration)?.Value); Assert.Equal(exifProfileInput.Values.Count + 1, encodedImageExifProfile.Values.Count); } } From 42d5d9ee912f8d5d1b8307cc5d916fddc7a89387 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 3 Jun 2021 12:10:27 +0200 Subject: [PATCH 45/48] Add support for decoding 6 bit per pixel tiff's --- src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs | 9 +++++++-- .../TiffColorDecoderFactory{TPixel}.cs | 10 ++++++++++ .../Tiff/PhotometricInterpretation/TiffColorType.cs | 9 +++++++-- src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs | 9 ++++++++- src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs | 5 +++++ .../Formats/Tiff/TiffBitsPerSampleExtensions.cs | 9 +++++++++ .../Formats/Tiff/TiffDecoderOptionsParser.cs | 4 ++++ src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 6 ++++++ .../ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Tiff/flower-rgb-contig-02.tiff | 3 +++ tests/Images/Input/Tiff/flower-rgb-planar-02.tiff | 3 +++ 13 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 tests/Images/Input/Tiff/flower-rgb-contig-02.tiff create mode 100644 tests/Images/Input/Tiff/flower-rgb-planar-02.tiff diff --git a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs index 988b1242ae..f56488a52f 100644 --- a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs +++ b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs @@ -96,15 +96,20 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Constants public static readonly ushort[] BitsPerSample8Bit = { 8 }; /// - /// The bits per sample for color images with 8 bits for each color channel. + /// The bits per sample for color images with 2 bits for each color channel. /// - public static readonly ushort[] BitsPerSampleRgb8Bit = { 8, 8, 8 }; + public static readonly ushort[] BitsPerSampleRgb2Bit = { 2, 2, 2 }; /// /// The bits per sample for color images with 4 bits for each color channel. /// public static readonly ushort[] BitsPerSampleRgb4Bit = { 4, 4, 4 }; + /// + /// The bits per sample for color images with 8 bits for each color channel. + /// + public static readonly ushort[] BitsPerSampleRgb8Bit = { 8, 8, 8 }; + /// /// The list of mimetypes that equate to a tiff. /// diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs index d78b06aa7f..2c59fdf137 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs @@ -57,6 +57,16 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample); + case TiffColorType.Rgb222: + DebugGuard.IsTrue( + bitsPerSample.Length == 3 + && bitsPerSample[0] == 2 + && bitsPerSample[1] == 2 + && bitsPerSample[2] == 2, + "bitsPerSample"); + DebugGuard.IsTrue(colorMap == null, "colorMap"); + return new RgbTiffColor(bitsPerSample); + case TiffColorType.Rgb444: DebugGuard.IsTrue( bitsPerSample.Length == 3 diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs index 089dc31ade..1d6535fd71 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs @@ -59,15 +59,20 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation Rgb, /// - /// RGB Full Color. Optimized implementation for 8-bit images. + /// RGB color image with 2 bits for each channel. /// - Rgb888, + Rgb222, /// /// RGB color image with 4 bits for each channel. /// Rgb444, + /// + /// RGB Full Color. Optimized implementation for 8-bit images. + /// + Rgb888, + /// /// RGB Full Color. Planar configuration of data. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs index 4b65f88b8c..0dee1105bb 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs @@ -18,6 +18,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Bit4 = 4, + /// + /// 6 bits per pixel. 2 bit for each color channel. + /// + /// Note: The TiffEncoder does not yet support 2 bits per color channel and will default to 24 bits per pixel instead. + /// + Bit6 = 6, + /// /// 8 bits per pixel, grayscale or color palette images. /// @@ -26,7 +33,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// /// 12 bits per pixel. 4 bit for each color channel. /// - /// Note: The TiffEncoder does not yet support 4 bits per color channel and will default to 24 bits per pixel. + /// Note: The TiffEncoder does not yet support 4 bits per color channel and will default to 24 bits per pixel instead. /// Bit12 = 12, diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs index 992a5ad6eb..0a4962b530 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs @@ -28,6 +28,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Bit8 = 8, + /// + /// Six bits per sample, each channel has 2 bits. + /// + Bit6 = 6, + /// /// Twelve bits per sample, each channel has 4 bits. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 0687b0104a..923e355f40 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -21,6 +21,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffConstants.BitsPerSample1Bit; case TiffBitsPerSample.Bit4: return TiffConstants.BitsPerSample4Bit; + case TiffBitsPerSample.Bit6: + return TiffConstants.BitsPerSampleRgb2Bit; case TiffBitsPerSample.Bit8: return TiffConstants.BitsPerSample8Bit; case TiffBitsPerSample.Bit12: @@ -57,6 +59,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffBitsPerSample.Bit12; } + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb2Bit[2] && + bitsPerSample[1] == TiffConstants.BitsPerSampleRgb2Bit[1] && + bitsPerSample[0] == TiffConstants.BitsPerSampleRgb2Bit[0]) + { + return TiffBitsPerSample.Bit6; + } + break; case 1: diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index b38ff68c18..1c2ee2443d 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -188,6 +188,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff case TiffBitsPerSample.Bit12: options.ColorType = TiffColorType.Rgb444; break; + case TiffBitsPerSample.Bit6: + options.ColorType = TiffColorType.Rgb222; + break; default: TiffThrowHelper.ThrowNotSupported("Bits per sample is not supported."); break; @@ -285,6 +288,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff { TiffBitsPerPixel.Bit1 => TiffBitsPerSample.Bit1, TiffBitsPerPixel.Bit4 => TiffBitsPerSample.Bit4, + TiffBitsPerPixel.Bit6 => TiffBitsPerSample.Bit6, TiffBitsPerPixel.Bit8 => TiffBitsPerSample.Bit8, TiffBitsPerPixel.Bit12 => TiffBitsPerSample.Bit12, TiffBitsPerPixel.Bit24 => TiffBitsPerSample.Bit24, diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index 6b5ca0086f..b61a0c0e1a 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -319,8 +319,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff case TiffBitsPerPixel.Bit8: this.SetEncoderOptions(bitsPerPixel, photometricInterpretation ?? TiffPhotometricInterpretation.BlackIsZero, compression, predictor); break; + case TiffBitsPerPixel.Bit6: case TiffBitsPerPixel.Bit12: - // Encoding 12 bits per pixel is not yet supported. Default to 24 bits. + // Encoding 12 and 6 bits per pixel is not yet supported. Default to 24 bits. this.SetEncoderOptions(TiffBitsPerPixel.Bit24, TiffPhotometricInterpretation.Rgb, compression, TiffPredictor.None); break; default: diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index c58977813a..ad27ed0fcc 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -99,6 +99,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_4Bit_WithPalette(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, ReferenceDecoder, useExactComparer: false, 0.01f); + [Theory] + [WithFile(FlowerRgb222Contiguous, PixelTypes.Rgba32)] + [WithFile(FlowerRgb222Planar, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_6Bit(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] [WithFile(FlowerRgb444Contiguous, PixelTypes.Rgba32)] [WithFile(FlowerRgb444Planar, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index dd3ef133e6..1b3104e728 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -79,6 +79,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [Theory] [InlineData(TiffBitsPerPixel.Bit12)] + [InlineData(TiffBitsPerPixel.Bit6)] public void EncoderOptions_UnsupportedBitPerPixel_DefaultTo24Bits(TiffBitsPerPixel bitsPerPixel) { // arrange diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index d1c29489fb..3eff09c75f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -562,6 +562,8 @@ namespace SixLabors.ImageSharp.Tests public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff"; public const string FlowerRgb444Contiguous = "Tiff/flower-rgb-contig-04.tiff"; public const string FlowerRgb444Planar = "Tiff/flower-rgb-planar-04.tiff"; + public const string FlowerRgb222Contiguous = "Tiff/flower-rgb-contig-02.tiff"; + public const string FlowerRgb222Planar = "Tiff/flower-rgb-planar-02.tiff"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; diff --git a/tests/Images/Input/Tiff/flower-rgb-contig-02.tiff b/tests/Images/Input/Tiff/flower-rgb-contig-02.tiff new file mode 100644 index 0000000000..a2d253dbd5 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-contig-02.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fbcd225c0db343f0cc984c35609b81f6413ebc1ba2ce2494d3607db375e969ff +size 2685 diff --git a/tests/Images/Input/Tiff/flower-rgb-planar-02.tiff b/tests/Images/Input/Tiff/flower-rgb-planar-02.tiff new file mode 100644 index 0000000000..8b301a534d --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-planar-02.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:21c4ede6382d8c72cb8e6f7939203d5111b362646a9727d95a2f63310ec8e5b3 +size 2795 From 8e6fad805cef36a305a332b517d5ba7a13e6a5e9 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 3 Jun 2021 12:35:30 +0200 Subject: [PATCH 46/48] Add support for decoding 30 bit per pixel tiff's --- src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs | 5 +++++ .../TiffColorDecoderFactory{TPixel}.cs | 10 ++++++++++ .../Tiff/PhotometricInterpretation/TiffColorType.cs | 5 +++++ src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs | 7 +++++++ src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs | 5 +++++ .../Formats/Tiff/TiffBitsPerSampleExtensions.cs | 9 +++++++++ .../Formats/Tiff/TiffDecoderOptionsParser.cs | 5 +++++ src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 6 ++++++ .../ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 2 ++ .../ReferenceCodecs/MagickReferenceDecoder.cs | 2 +- tests/Images/Input/Tiff/flower-rgb-contig-10.tiff | 3 +++ tests/Images/Input/Tiff/flower-rgb-planar-10.tiff | 3 +++ 14 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Tiff/flower-rgb-contig-10.tiff create mode 100644 tests/Images/Input/Tiff/flower-rgb-planar-10.tiff diff --git a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs index f56488a52f..2327528b0e 100644 --- a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs +++ b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs @@ -110,6 +110,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Constants /// public static readonly ushort[] BitsPerSampleRgb8Bit = { 8, 8, 8 }; + /// + /// The bits per sample for color images with 10 bits for each color channel. + /// + public static readonly ushort[] BitsPerSampleRgb10Bit = { 10, 10, 10 }; + /// /// The list of mimetypes that equate to a tiff. /// diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs index 2c59fdf137..9ebf48620a 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs @@ -87,6 +87,16 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation DebugGuard.IsTrue(colorMap == null, "colorMap"); return new Rgb888TiffColor(); + case TiffColorType.Rgb101010: + DebugGuard.IsTrue( + bitsPerSample.Length == 3 + && bitsPerSample[0] == 10 + && bitsPerSample[1] == 10 + && bitsPerSample[2] == 10, + "bitsPerSample"); + DebugGuard.IsTrue(colorMap == null, "colorMap"); + return new RgbTiffColor(bitsPerSample); + case TiffColorType.PaletteColor: DebugGuard.NotNull(bitsPerSample, "bitsPerSample"); DebugGuard.NotNull(colorMap, "colorMap"); diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs index 1d6535fd71..afa86b1430 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs @@ -73,6 +73,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation /// Rgb888, + /// + /// RGB color image with 10 bits for each channel. + /// + Rgb101010, + /// /// RGB Full Color. Planar configuration of data. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs index 0dee1105bb..dc1ef0fd6e 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs @@ -41,5 +41,12 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// 24 bits per pixel. One byte for each color channel. /// Bit24 = 24, + + /// + /// 30 bits per pixel. 10 bit for each color channel. + /// + /// Note: The TiffEncoder does not yet support 10 bits per color channel and will default to 24 bits per pixel instead. + /// + Bit30 = 30, } } diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs index 0a4962b530..02378ded30 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs @@ -42,5 +42,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// 24 bits per sample, each color channel has 8 Bits. /// Bit24 = 24, + + /// + /// Thirty bits per sample, each channel has 10 bits. + /// + Bit30 = 30, } } diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 923e355f40..51a5a53a7a 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -29,6 +29,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffConstants.BitsPerSampleRgb4Bit; case TiffBitsPerSample.Bit24: return TiffConstants.BitsPerSampleRgb8Bit; + case TiffBitsPerSample.Bit30: + return TiffConstants.BitsPerSampleRgb10Bit; default: return Array.Empty(); @@ -45,6 +47,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff switch (bitsPerSample.Length) { case 3: + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb10Bit[2] && + bitsPerSample[1] == TiffConstants.BitsPerSampleRgb10Bit[1] && + bitsPerSample[0] == TiffConstants.BitsPerSampleRgb10Bit[0]) + { + return TiffBitsPerSample.Bit30; + } + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb8Bit[2] && bitsPerSample[1] == TiffConstants.BitsPerSampleRgb8Bit[1] && bitsPerSample[0] == TiffConstants.BitsPerSampleRgb8Bit[0]) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 1c2ee2443d..cf6ac6dc75 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -182,6 +182,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff { switch (options.BitsPerSample) { + case TiffBitsPerSample.Bit30: + options.ColorType = TiffColorType.Rgb101010; + break; + case TiffBitsPerSample.Bit24: options.ColorType = TiffColorType.Rgb888; break; @@ -292,6 +296,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff TiffBitsPerPixel.Bit8 => TiffBitsPerSample.Bit8, TiffBitsPerPixel.Bit12 => TiffBitsPerSample.Bit12, TiffBitsPerPixel.Bit24 => TiffBitsPerSample.Bit24, + TiffBitsPerPixel.Bit30 => TiffBitsPerSample.Bit30, _ => throw new NotSupportedException("The bits per pixel are not supported"), }; } diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index b61a0c0e1a..edfa215cc5 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -321,7 +321,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff break; case TiffBitsPerPixel.Bit6: case TiffBitsPerPixel.Bit12: - // Encoding 12 and 6 bits per pixel is not yet supported. Default to 24 bits. + case TiffBitsPerPixel.Bit30: + // Encoding 30, 12 and 6 bits per pixel is not yet supported. Default to 24 bits. this.SetEncoderOptions(TiffBitsPerPixel.Bit24, TiffPhotometricInterpretation.Rgb, compression, TiffPredictor.None); break; default: diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index ad27ed0fcc..0dd8e0e22e 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -111,6 +111,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_12Bit(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] + [WithFile(FlowerRgb101010Contiguous, PixelTypes.Rgba32)] + [WithFile(FlowerRgb101010Planar, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_30Bit(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)] [WithFile(RgbDeflateMultistrip, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 1b3104e728..19cfc42e4f 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -78,6 +78,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff } [Theory] + [InlineData(TiffBitsPerPixel.Bit30)] [InlineData(TiffBitsPerPixel.Bit12)] [InlineData(TiffBitsPerPixel.Bit6)] public void EncoderOptions_UnsupportedBitPerPixel_DefaultTo24Bits(TiffBitsPerPixel bitsPerPixel) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 3eff09c75f..05045d06a4 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -560,6 +560,8 @@ namespace SixLabors.ImageSharp.Tests public const string RgbPaletteDeflate = "Tiff/rgb_palette_deflate.tiff"; public const string Flower4BitPalette = "Tiff/flower-palette-04.tiff"; public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff"; + public const string FlowerRgb101010Contiguous = "Tiff/flower-rgb-contig-10.tiff"; + public const string FlowerRgb101010Planar = "Tiff/flower-rgb-planar-10.tiff"; public const string FlowerRgb444Contiguous = "Tiff/flower-rgb-contig-04.tiff"; public const string FlowerRgb444Planar = "Tiff/flower-rgb-planar-04.tiff"; public const string FlowerRgb222Contiguous = "Tiff/flower-rgb-contig-02.tiff"; diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index 885d12e774..30c2214b5a 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs MemoryGroup framePixels = frame.PixelBuffer.FastMemoryGroup; using IUnsafePixelCollection pixels = magicFrame.GetPixelsUnsafe(); - if (magicFrame.Depth == 8 || magicFrame.Depth == 4 || magicFrame.Depth == 2 || magicFrame.Depth == 1) + if (magicFrame.Depth == 8 || magicFrame.Depth == 4 || magicFrame.Depth == 2 || magicFrame.Depth == 1 || magicFrame.Depth == 10) { byte[] data = pixels.ToByteArray(PixelMapping.RGBA); diff --git a/tests/Images/Input/Tiff/flower-rgb-contig-10.tiff b/tests/Images/Input/Tiff/flower-rgb-contig-10.tiff new file mode 100644 index 0000000000..2b271c8004 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-contig-10.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:68168ea1c2e50e674a7c5c41e5b055c881adf8cb940d0fd033a927a7ebdd7b6f +size 12117 diff --git a/tests/Images/Input/Tiff/flower-rgb-planar-10.tiff b/tests/Images/Input/Tiff/flower-rgb-planar-10.tiff new file mode 100644 index 0000000000..be0acd6465 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-planar-10.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7f53948d4a36c80f45d70a315d2e76514ec41cabe982c06dbbd0d47e671120e2 +size 12211 From deed7485253358e25319875da649c0807cb42a1b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 3 Jun 2021 13:20:18 +0200 Subject: [PATCH 47/48] Add support for decoding 10 bit per channel rgb tiff's --- src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs | 5 +++++ .../TiffColorDecoderFactory{TPixel}.cs | 10 ++++++++++ .../Tiff/PhotometricInterpretation/TiffColorType.cs | 5 +++++ src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs | 7 +++++++ src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs | 5 +++++ .../Formats/Tiff/TiffBitsPerSampleExtensions.cs | 9 +++++++++ .../Formats/Tiff/TiffDecoderOptionsParser.cs | 5 +++++ src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 6 ++++++ .../ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 2 ++ .../ReferenceCodecs/MagickReferenceDecoder.cs | 7 ++----- tests/Images/Input/Tiff/flower-rgb-contig-14.tiff | 3 +++ tests/Images/Input/Tiff/flower-rgb-planar-14.tiff | 3 +++ 14 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 tests/Images/Input/Tiff/flower-rgb-contig-14.tiff create mode 100644 tests/Images/Input/Tiff/flower-rgb-planar-14.tiff diff --git a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs index 2327528b0e..6fe412b925 100644 --- a/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs +++ b/src/ImageSharp/Formats/Tiff/Constants/TiffConstants.cs @@ -115,6 +115,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Constants /// public static readonly ushort[] BitsPerSampleRgb10Bit = { 10, 10, 10 }; + /// + /// The bits per sample for color images with 14 bits for each color channel. + /// + public static readonly ushort[] BitsPerSampleRgb14Bit = { 14, 14, 14 }; + /// /// The list of mimetypes that equate to a tiff. /// diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs index 9ebf48620a..924415850d 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs @@ -97,6 +97,16 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample); + case TiffColorType.Rgb141414: + DebugGuard.IsTrue( + bitsPerSample.Length == 3 + && bitsPerSample[0] == 14 + && bitsPerSample[1] == 14 + && bitsPerSample[2] == 14, + "bitsPerSample"); + DebugGuard.IsTrue(colorMap == null, "colorMap"); + return new RgbTiffColor(bitsPerSample); + case TiffColorType.PaletteColor: DebugGuard.NotNull(bitsPerSample, "bitsPerSample"); DebugGuard.NotNull(colorMap, "colorMap"); diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs index afa86b1430..22d8199533 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorType.cs @@ -78,6 +78,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation /// Rgb101010, + /// + /// RGB color image with 14 bits for each channel. + /// + Rgb141414, + /// /// RGB Full Color. Planar configuration of data. /// diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs index dc1ef0fd6e..ab9f3cbec0 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerPixel.cs @@ -48,5 +48,12 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Note: The TiffEncoder does not yet support 10 bits per color channel and will default to 24 bits per pixel instead. /// Bit30 = 30, + + /// + /// 42 bits per pixel. 14 bit for each color channel. + /// + /// Note: The TiffEncoder does not yet support 14 bits per color channel and will default to 24 bits per pixel instead. + /// + Bit42 = 42, } } diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs index 02378ded30..088ef5d6f8 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSample.cs @@ -47,5 +47,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Thirty bits per sample, each channel has 10 bits. /// Bit30 = 30, + + /// + /// Forty two bits per sample, each channel has 14 bits. + /// + Bit42 = 42, } } diff --git a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs index 51a5a53a7a..ca0f0befcc 100644 --- a/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs +++ b/src/ImageSharp/Formats/Tiff/TiffBitsPerSampleExtensions.cs @@ -31,6 +31,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff return TiffConstants.BitsPerSampleRgb8Bit; case TiffBitsPerSample.Bit30: return TiffConstants.BitsPerSampleRgb10Bit; + case TiffBitsPerSample.Bit42: + return TiffConstants.BitsPerSampleRgb14Bit; default: return Array.Empty(); @@ -47,6 +49,13 @@ namespace SixLabors.ImageSharp.Formats.Tiff switch (bitsPerSample.Length) { case 3: + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb14Bit[2] && + bitsPerSample[1] == TiffConstants.BitsPerSampleRgb14Bit[1] && + bitsPerSample[0] == TiffConstants.BitsPerSampleRgb14Bit[0]) + { + return TiffBitsPerSample.Bit42; + } + if (bitsPerSample[2] == TiffConstants.BitsPerSampleRgb10Bit[2] && bitsPerSample[1] == TiffConstants.BitsPerSampleRgb10Bit[1] && bitsPerSample[0] == TiffConstants.BitsPerSampleRgb10Bit[0]) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index cf6ac6dc75..eeac6a33c2 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -182,6 +182,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff { switch (options.BitsPerSample) { + case TiffBitsPerSample.Bit42: + options.ColorType = TiffColorType.Rgb141414; + break; + case TiffBitsPerSample.Bit30: options.ColorType = TiffColorType.Rgb101010; break; @@ -297,6 +301,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff TiffBitsPerPixel.Bit12 => TiffBitsPerSample.Bit12, TiffBitsPerPixel.Bit24 => TiffBitsPerSample.Bit24, TiffBitsPerPixel.Bit30 => TiffBitsPerSample.Bit30, + TiffBitsPerPixel.Bit42 => TiffBitsPerSample.Bit42, _ => throw new NotSupportedException("The bits per pixel are not supported"), }; } diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index edfa215cc5..d5137c4357 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -322,7 +322,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff case TiffBitsPerPixel.Bit6: case TiffBitsPerPixel.Bit12: case TiffBitsPerPixel.Bit30: - // Encoding 30, 12 and 6 bits per pixel is not yet supported. Default to 24 bits. + case TiffBitsPerPixel.Bit42: + // Encoding 42, 30, 12 and 6 bits per pixel is not yet supported. Default to 24 bits. this.SetEncoderOptions(TiffBitsPerPixel.Bit24, TiffPhotometricInterpretation.Rgb, compression, TiffPredictor.None); break; default: diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 0dd8e0e22e..02b7f97d94 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -117,6 +117,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_30Bit(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] + [WithFile(FlowerRgb141414Contiguous, PixelTypes.Rgba32)] + [WithFile(FlowerRgb141414Planar, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_42Bit(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)] [WithFile(RgbDeflateMultistrip, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 19cfc42e4f..7c386a6a9a 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -78,6 +78,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff } [Theory] + [InlineData(TiffBitsPerPixel.Bit42)] [InlineData(TiffBitsPerPixel.Bit30)] [InlineData(TiffBitsPerPixel.Bit12)] [InlineData(TiffBitsPerPixel.Bit6)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 05045d06a4..9471a63937 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -560,6 +560,8 @@ namespace SixLabors.ImageSharp.Tests public const string RgbPaletteDeflate = "Tiff/rgb_palette_deflate.tiff"; public const string Flower4BitPalette = "Tiff/flower-palette-04.tiff"; public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff"; + public const string FlowerRgb141414Contiguous = "Tiff/flower-rgb-contig-14.tiff"; + public const string FlowerRgb141414Planar = "Tiff/flower-rgb-planar-14.tiff"; public const string FlowerRgb101010Contiguous = "Tiff/flower-rgb-contig-10.tiff"; public const string FlowerRgb101010Planar = "Tiff/flower-rgb-planar-10.tiff"; public const string FlowerRgb444Contiguous = "Tiff/flower-rgb-contig-04.tiff"; diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index 30c2214b5a..dffbeac497 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -25,10 +25,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs { } - public MagickReferenceDecoder(bool validate) - { - this.validate = validate; - } + public MagickReferenceDecoder(bool validate) => this.validate = validate; public static MagickReferenceDecoder Instance { get; } = new MagickReferenceDecoder(); @@ -93,7 +90,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs FromRgba32Bytes(configuration, data, framePixels); } - else if (magicFrame.Depth == 16) + else if (magicFrame.Depth == 16 || magicFrame.Depth == 14) { ushort[] data = pixels.ToShortArray(PixelMapping.RGBA); Span bytes = MemoryMarshal.Cast(data.AsSpan()); diff --git a/tests/Images/Input/Tiff/flower-rgb-contig-14.tiff b/tests/Images/Input/Tiff/flower-rgb-contig-14.tiff new file mode 100644 index 0000000000..d4d6a9492d --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-contig-14.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a419a8e2f89321501ca8ad70d2a19d37a7bf3a8c2f45c809acc30be59139ae29 +size 16855 diff --git a/tests/Images/Input/Tiff/flower-rgb-planar-14.tiff b/tests/Images/Input/Tiff/flower-rgb-planar-14.tiff new file mode 100644 index 0000000000..2d517268e9 --- /dev/null +++ b/tests/Images/Input/Tiff/flower-rgb-planar-14.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d28f021d40f53a011053f9644400fee2d29c02f97b4101fec899251125dbb18e +size 16855 From 036b95bd7a49ba105febee78c88198fa0e07ba08 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 3 Jun 2021 14:44:20 +0200 Subject: [PATCH 48/48] Flip order of comparing bitsPerSample --- .../TiffColorDecoderFactory{TPixel}.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs index 924415850d..5555eb537c 100644 --- a/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs +++ b/src/ImageSharp/Formats/Tiff/PhotometricInterpretation/TiffColorDecoderFactory{TPixel}.cs @@ -60,9 +60,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation case TiffColorType.Rgb222: DebugGuard.IsTrue( bitsPerSample.Length == 3 - && bitsPerSample[0] == 2 + && bitsPerSample[2] == 2 && bitsPerSample[1] == 2 - && bitsPerSample[2] == 2, + && bitsPerSample[0] == 2, "bitsPerSample"); DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample); @@ -70,9 +70,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation case TiffColorType.Rgb444: DebugGuard.IsTrue( bitsPerSample.Length == 3 - && bitsPerSample[0] == 4 + && bitsPerSample[2] == 4 && bitsPerSample[1] == 4 - && bitsPerSample[2] == 4, + && bitsPerSample[0] == 4, "bitsPerSample"); DebugGuard.IsTrue(colorMap == null, "colorMap"); return new Rgb444TiffColor(); @@ -80,9 +80,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation case TiffColorType.Rgb888: DebugGuard.IsTrue( bitsPerSample.Length == 3 - && bitsPerSample[0] == 8 + && bitsPerSample[2] == 8 && bitsPerSample[1] == 8 - && bitsPerSample[2] == 8, + && bitsPerSample[0] == 8, "bitsPerSample"); DebugGuard.IsTrue(colorMap == null, "colorMap"); return new Rgb888TiffColor(); @@ -90,9 +90,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation case TiffColorType.Rgb101010: DebugGuard.IsTrue( bitsPerSample.Length == 3 - && bitsPerSample[0] == 10 + && bitsPerSample[2] == 10 && bitsPerSample[1] == 10 - && bitsPerSample[2] == 10, + && bitsPerSample[0] == 10, "bitsPerSample"); DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample); @@ -100,9 +100,9 @@ namespace SixLabors.ImageSharp.Formats.Tiff.PhotometricInterpretation case TiffColorType.Rgb141414: DebugGuard.IsTrue( bitsPerSample.Length == 3 - && bitsPerSample[0] == 14 + && bitsPerSample[2] == 14 && bitsPerSample[1] == 14 - && bitsPerSample[2] == 14, + && bitsPerSample[0] == 14, "bitsPerSample"); DebugGuard.IsTrue(colorMap == null, "colorMap"); return new RgbTiffColor(bitsPerSample);