diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs index 8efbf92b64..95829e8109 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs @@ -234,9 +234,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters public ComponentValues Slice(int start, int length) { Span c0 = this.Component0.Slice(start, length); - Span c1 = this.ComponentCount > 1 ? this.Component1.Slice(start, length) : Span.Empty; - Span c2 = this.ComponentCount > 2 ? this.Component2.Slice(start, length) : Span.Empty; - Span c3 = this.ComponentCount > 3 ? this.Component3.Slice(start, length) : Span.Empty; + Span c1 = this.Component1.Length > 0 ? this.Component1.Slice(start, length) : Span.Empty; + Span c2 = this.Component2.Length > 0 ? this.Component2.Slice(start, length) : Span.Empty; + Span c3 = this.Component3.Length > 0 ? this.Component3.Slice(start, length) : Span.Empty; return new ComponentValues(this.ComponentCount, c0, c1, c2, c3); } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 313a132b81..313457ea26 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -22,7 +22,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private JpegColorConverter colorConverter; - private IMemoryOwner rgbaBuffer; + // private IMemoryOwner rgbaBuffer; + private IMemoryOwner rgbBuffer; + + private IMemoryOwner paddedProxyPixelRow; private Buffer2D pixelBuffer; @@ -40,23 +43,20 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private bool Converted => this.pixelRowCounter >= this.pixelBuffer.Height; - public Buffer2D PixelBuffer + public Buffer2D GetPixelBuffer() { - get + if (!this.Converted) { - if (!this.Converted) - { - int steps = (int)Math.Ceiling(this.pixelBuffer.Height / (float)this.pixelRowsPerStep); + int steps = (int) Math.Ceiling(this.pixelBuffer.Height / (float) this.pixelRowsPerStep); - for (int step = 0; step < steps; step++) - { - this.cancellationToken.ThrowIfCancellationRequested(); - this.ConvertNextStride(step); - } + for (int step = 0; step < steps; step++) + { + this.cancellationToken.ThrowIfCancellationRequested(); + this.ConvertNextStride(step); } - - return this.pixelBuffer; } + + return this.pixelBuffer; } /// @@ -72,7 +72,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.pixelRowsPerStep = this.blockRowsPerStep * blockPixelHeight; // pixel buffer for resulting image - this.pixelBuffer = allocator.Allocate2D(frame.PixelWidth, frame.PixelHeight, AllocationOptions.Clean); + this.pixelBuffer = allocator.Allocate2D(frame.PixelWidth, frame.PixelHeight); + this.paddedProxyPixelRow = allocator.Allocate(frame.PixelWidth + 3); // component processors from spectral to Rgba32 var postProcessorBufferSize = new Size(c0.SizeInBlocks.Width * 8, this.pixelRowsPerStep); @@ -83,7 +84,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } // single 'stride' rgba32 buffer for conversion between spectral and TPixel - this.rgbaBuffer = allocator.Allocate(frame.PixelWidth); + // this.rgbaBuffer = allocator.Allocate(frame.PixelWidth); + this.rgbBuffer = allocator.Allocate(frame.PixelWidth * 3); // color converter from Rgba32 to TPixel this.colorConverter = this.GetColorConverter(frame, jpegData); @@ -115,7 +117,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } - this.rgbaBuffer?.Dispose(); + this.rgbBuffer?.Dispose(); + this.paddedProxyPixelRow?.Dispose(); } private void ConvertNextStride(int spectralStep) @@ -129,17 +132,38 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder buffers[i] = this.componentProcessors[i].ColorBuffer; } + int width = this.pixelBuffer.Width; + for (int yy = this.pixelRowCounter; yy < maxY; yy++) { int y = yy - this.pixelRowCounter; var values = new JpegColorConverter.ComponentValues(buffers, y); - this.colorConverter.ConvertToRgba(values, this.rgbaBuffer.GetSpan()); - Span destRow = this.pixelBuffer.GetRowSpan(yy); + this.colorConverter.ConvertToRgbInplace(values); + values = values.Slice(0, width); // slice away Jpeg padding - // TODO: Investigate if slicing is actually necessary - PixelOperations.Instance.FromVector4Destructive(this.configuration, this.rgbaBuffer.GetSpan().Slice(0, destRow.Length), destRow); + Span r = this.rgbBuffer.Slice(0, width); + Span g = this.rgbBuffer.Slice(width, width); + Span b = this.rgbBuffer.Slice(width * 2, width); + + SimdUtils.NormalizedFloatToByteSaturate(values.Component0, r); + SimdUtils.NormalizedFloatToByteSaturate(values.Component1, g); + SimdUtils.NormalizedFloatToByteSaturate(values.Component2, b); + + // PackFromRgbPlanes expects the destination to be padded, so try to get padded span containing extra elements from the next row. + // If we can't get such a padded row because we are on a MemoryGroup boundary or at the last row, + // pack pixels to a temporary, padded proxy buffer, then copy the relevant values to the destination row. + if (this.pixelBuffer.TryGetPaddedRowSpan(yy, 3, out Span destRow)) + { + PixelOperations.Instance.PackFromRgbPlanes(this.configuration, r, g, b, destRow); + } + else + { + Span proxyRow = this.paddedProxyPixelRow.GetSpan(); + PixelOperations.Instance.PackFromRgbPlanes(this.configuration, r, g, b, proxyRow); + proxyRow.Slice(0, width).CopyTo(this.pixelBuffer.GetRowSpan(yy)); + } } this.pixelRowCounter += this.pixelRowsPerStep; diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index a0f69bb7bf..9a9e5eb799 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -185,7 +185,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.InitIptcProfile(); this.InitDerivedMetadataProperties(); - return new Image(this.Configuration, spectralConverter.PixelBuffer, this.Metadata); + return new Image(this.Configuration, spectralConverter.GetPixelBuffer(), this.Metadata); } /// diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index e764c014d3..9a0607584e 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors scanDecoder.ResetInterval = 0; jpegDecoder.ParseStream(stream, scanDecoder, CancellationToken.None); - CopyImageBytesToBuffer(buffer, spectralConverter.PixelBuffer); + CopyImageBytesToBuffer(buffer, spectralConverter.GetPixelBuffer()); } else { diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 21c19f5d52..4f7aa3419c 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -116,6 +116,36 @@ namespace SixLabors.ImageSharp.Memory : this.GetRowMemorySlow(y).Span; } + internal bool TryGetPaddedRowSpan(int y, int padding, out Span paddedSpan) + { + DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y)); + DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); + + int stride = this.Width + padding; + if (this.cachedMemory.Length > 0) + { + paddedSpan = this.cachedMemory.Span.Slice(y * this.Width); + if (paddedSpan.Length < stride) + { + return false; + } + + paddedSpan = paddedSpan.Slice(0, stride); + return true; + } + + Memory memory = this.FastMemoryGroup.GetRemainingSliceOfBuffer(y * (long)this.Width); + + if (memory.Length < stride) + { + paddedSpan = default; + return false; + } + + paddedSpan = memory.Span.Slice(0, stride); + return true; + } + [MethodImpl(InliningOptions.ShortMethod)] internal ref T GetElementUnsafe(int x, int y) { @@ -202,6 +232,7 @@ namespace SixLabors.ImageSharp.Memory [MethodImpl(InliningOptions.ColdPath)] private Memory GetRowMemorySlow(int y) => this.FastMemoryGroup.GetBoundedSlice(y * (long)this.Width, this.Width); + [MethodImpl(InliningOptions.ColdPath)] private Memory DangerousGetSingleMemorySlow() => this.FastMemoryGroup.Single(); diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs index da42b30ad8..319c72af52 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs @@ -61,6 +61,35 @@ namespace SixLabors.ImageSharp.Memory return memory.Slice(bufferStart, length); } + /// + /// Returns the slice of the buffer starting at global index that goes until the end of the buffer. + /// + internal static Memory GetRemainingSliceOfBuffer(this IMemoryGroup group, long start) + where T : struct + { + Guard.NotNull(group, nameof(group)); + Guard.IsTrue(group.IsValid, nameof(group), "Group must be valid!"); + Guard.MustBeLessThan(start, group.TotalLength, nameof(start)); + + int bufferIdx = (int)(start / group.BufferLength); + + if (bufferIdx < 0) + { + throw new ArgumentOutOfRangeException(nameof(start)); + } + + if (bufferIdx >= group.Count) + { + throw new ArgumentOutOfRangeException(nameof(start)); + } + + int bufferStart = (int)(start % group.BufferLength); + + Memory memory = group[bufferIdx]; + + return memory.Slice(bufferStart); + } + internal static void CopyTo(this IMemoryGroup source, Span target) where T : struct { diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs index f345f58bcd..0f1ea6b815 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs @@ -32,9 +32,7 @@ namespace SixLabors.ImageSharp.PixelFormats { Guard.NotNull(configuration, nameof(configuration)); int count = redChannel.Length; - Guard.IsTrue(greenChannel.Length == count, nameof(greenChannel), "Channels must be of same size!"); - Guard.IsTrue(blueChannel.Length == count, nameof(blueChannel), "Channels must be of same size!"); - Guard.IsTrue(destination.Length > count + 2, nameof(destination), "'destination' must contain a padding of 3 elements!"); + GuardPackFromRgbPlanes(greenChannel, blueChannel, destination, count); SimdUtils.PackFromRgbPlanes(configuration, redChannel, greenChannel, blueChannel, destination); } diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs index 9633059774..d937da98fd 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs @@ -67,9 +67,7 @@ namespace SixLabors.ImageSharp.PixelFormats { Guard.NotNull(configuration, nameof(configuration)); int count = redChannel.Length; - Guard.IsTrue(greenChannel.Length == count, nameof(greenChannel), "Channels must be of same size!"); - Guard.IsTrue(blueChannel.Length == count, nameof(blueChannel), "Channels must be of same size!"); - Guard.IsTrue(destination.Length > count, nameof(destination), "'destination' span should not be shorter than the source channels!"); + GuardPackFromRgbPlanes(greenChannel, blueChannel, destination, count); SimdUtils.PackFromRgbPlanes(configuration, redChannel, greenChannel, blueChannel, destination); } diff --git a/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs b/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs index c5450538e4..f748a4b574 100644 --- a/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs +++ b/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs @@ -181,11 +181,7 @@ namespace SixLabors.ImageSharp.PixelFormats Guard.NotNull(configuration, nameof(configuration)); int count = redChannel.Length; - Guard.IsTrue(greenChannel.Length == count, nameof(greenChannel), "Channels must be of same size!"); - Guard.IsTrue(blueChannel.Length == count, nameof(blueChannel), "Channels must be of same size!"); - Guard.IsTrue(destination.Length > count + 2, nameof(destination), "'destination' must contain a padding of 3 elements!"); - - Guard.DestinationShouldNotBeTooShort(redChannel, destination, nameof(destination)); + GuardPackFromRgbPlanes(greenChannel, blueChannel, destination, count); Rgb24 rgb24 = default; ref byte r = ref MemoryMarshal.GetReference(redChannel); @@ -201,5 +197,13 @@ namespace SixLabors.ImageSharp.PixelFormats Unsafe.Add(ref d, i).FromRgb24(rgb24); } } + + [MethodImpl(InliningOptions.ShortMethod)] + internal static void GuardPackFromRgbPlanes(ReadOnlySpan greenChannel, ReadOnlySpan blueChannel, Span destination, int count) + { + Guard.IsTrue(greenChannel.Length == count, nameof(greenChannel), "Channels must be of same size!"); + Guard.IsTrue(blueChannel.Length == count, nameof(blueChannel), "Channels must be of same size!"); + Guard.IsTrue(destination.Length > count + 2, nameof(destination), "'destination' must contain a padding of 3 elements!"); + } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/SpectralToPixelConversionTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/SpectralToPixelConversionTests.cs index 353ae39f0f..f0f92d763e 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/SpectralToPixelConversionTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/SpectralToPixelConversionTests.cs @@ -53,7 +53,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg provider.Utility.TestName = JpegDecoderTests.DecodeBaselineJpegOutputName; // Comparison - using (Image image = new Image(Configuration.Default, converter.PixelBuffer, new ImageMetadata())) + using (Image image = new Image(Configuration.Default, converter.GetPixelBuffer(), new ImageMetadata())) using (Image referenceImage = provider.GetReferenceOutputImage(appendPixelTypeToFileName: false)) { ImageSimilarityReport report = ImageComparer.Exact.CompareImagesOrFrames(referenceImage, image); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 015b3617bc..4e097f3f13 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -118,6 +118,32 @@ namespace SixLabors.ImageSharp.Tests.Memory } } + [Theory] + [InlineData(10, 0, 0, 0)] + [InlineData(10, 0, 2, 0)] + [InlineData(10, 1, 2, 0)] + [InlineData(10, 1, 3, 0)] + [InlineData(10, 1, 5, -1)] + [InlineData(10, 2, 2, -1)] + [InlineData(10, 3, 2, 1)] + [InlineData(10, 4, 2, -1)] + [InlineData(30, 3, 2, 0)] + [InlineData(30, 4, 1, -1)] + public void TryGetPaddedRowSpanY(int bufferCapacity, int y, int padding, int expectedBufferIndex) + { + this.MemoryAllocator.BufferCapacityInBytes = bufferCapacity; + using Buffer2D buffer = this.MemoryAllocator.Allocate2D(3, 5); + + bool expectSuccess = expectedBufferIndex >= 0; + bool success = buffer.TryGetPaddedRowSpan(y, padding, out Span paddedSpan); + Xunit.Assert.Equal(expectSuccess, success); + if (success) + { + int expectedSubBufferOffset = (3 * y) - (expectedBufferIndex * buffer.FastMemoryGroup.BufferLength); + Assert.SpanPointsTo(paddedSpan, buffer.FastMemoryGroup[expectedBufferIndex], expectedSubBufferOffset); + } + } + public static TheoryData GetRowSpanY_OutOfRange_Data = new TheoryData() { { Big, 10, 8, -1 },