From 2999ef15733676f773edbe747fbc106cf92185d8 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 24 Dec 2016 08:06:09 +0100 Subject: [PATCH] fixed PixelArea pooling + added reusing of PixelArea-s in JpegEncoderCore --- src/ImageSharp/Formats/Jpg/JpegEncoderCore.cs | 166 +++++++++--------- src/ImageSharp/Image/PixelArea{TColor}.cs | 39 ++-- 2 files changed, 114 insertions(+), 91 deletions(-) diff --git a/src/ImageSharp/Formats/Jpg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpg/JpegEncoderCore.cs index 4444e50bf..d6c387911 100644 --- a/src/ImageSharp/Formats/Jpg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpg/JpegEncoderCore.cs @@ -395,45 +395,45 @@ namespace ImageSharp.Formats /// The luminance block. /// The red chroma block. /// The blue chroma block. + /// Temporal provided by the caller private static void ToYCbCr( PixelAccessor pixels, int x, int y, Block8x8F* yBlock, Block8x8F* cbBlock, - Block8x8F* crBlock) + Block8x8F* crBlock, + PixelArea rgbBytes) where TColor : struct, IPackedPixel, IEquatable { float* yBlockRaw = (float*)yBlock; float* cbBlockRaw = (float*)cbBlock; float* crBlockRaw = (float*)crBlock; - using (PixelArea rgbBytes = new PixelArea(8, 8, ComponentOrder.XYZ)) - { - pixels.CopyRGBBytesStretchedTo(rgbBytes, y, x); + rgbBytes.Reset(); + pixels.CopyRGBBytesStretchedTo(rgbBytes, y, x); - byte* data = (byte*)rgbBytes.DataPointer; + byte* data = (byte*)rgbBytes.DataPointer; - for (int j = 0; j < 8; j++) + for (int j = 0; j < 8; j++) + { + int j8 = j * 8; + for (int i = 0; i < 8; i++) { - int j8 = j * 8; - for (int i = 0; i < 8; i++) - { - Vector3 v = new Vector3(data[0], data[1], data[2]); + Vector3 v = new Vector3(data[0], data[1], data[2]); - // Convert returned bytes into the YCbCr color space. Assume RGBA - float yy = (0.299F * v.X) + (0.587F * v.Y) + (0.114F * v.Z); - float cb = 128 + ((-0.168736F * v.X) - (0.331264F * v.Y) + (0.5F * v.Z)); - float cr = 128 + ((0.5F * v.X) - (0.418688F * v.Y) - (0.081312F * v.Z)); + // Convert returned bytes into the YCbCr color space. Assume RGBA + float yy = (0.299F * v.X) + (0.587F * v.Y) + (0.114F * v.Z); + float cb = 128 + ((-0.168736F * v.X) - (0.331264F * v.Y) + (0.5F * v.Z)); + float cr = 128 + ((0.5F * v.X) - (0.418688F * v.Y) - (0.081312F * v.Z)); - int index = j8 + i; + int index = j8 + i; - yBlockRaw[index] = yy; - cbBlockRaw[index] = cb; - crBlockRaw[index] = cr; + yBlockRaw[index] = yy; + cbBlockRaw[index] = cb; + crBlockRaw[index] = cr; - data += 3; - } + data += 3; } } } @@ -844,6 +844,7 @@ namespace ImageSharp.Formats private void Encode444(PixelAccessor pixels) where TColor : struct, IPackedPixel, IEquatable { + // TODO: Need a JpegEncoderCoreCore class or struct to hold all this mess: Block8x8F b = default(Block8x8F); Block8x8F cb = default(Block8x8F); Block8x8F cr = default(Block8x8F); @@ -859,36 +860,39 @@ namespace ImageSharp.Formats // ReSharper disable once InconsistentNaming float prevDCY = 0, prevDCCb = 0, prevDCCr = 0; - for (int y = 0; y < pixels.Height; y += 8) + using (PixelArea rgbBytes = new PixelArea(8, 8, ComponentOrder.XYZ, true)) { - for (int x = 0; x < pixels.Width; x += 8) + for (int y = 0; y < pixels.Height; y += 8) { - ToYCbCr(pixels, x, y, &b, &cb, &cr); - - prevDCY = this.WriteBlock( - QuantIndex.Luminance, - prevDCY, - &b, - &temp1, - &temp2, - &onStackLuminanceQuantTable, - unzig.Data); - prevDCCb = this.WriteBlock( - QuantIndex.Chrominance, - prevDCCb, - &cb, - &temp1, - &temp2, - &onStackChrominanceQuantTable, - unzig.Data); - prevDCCr = this.WriteBlock( - QuantIndex.Chrominance, - prevDCCr, - &cr, - &temp1, - &temp2, - &onStackChrominanceQuantTable, - unzig.Data); + for (int x = 0; x < pixels.Width; x += 8) + { + ToYCbCr(pixels, x, y, &b, &cb, &cr, rgbBytes); + + prevDCY = this.WriteBlock( + QuantIndex.Luminance, + prevDCY, + &b, + &temp1, + &temp2, + &onStackLuminanceQuantTable, + unzig.Data); + prevDCCb = this.WriteBlock( + QuantIndex.Chrominance, + prevDCCb, + &cb, + &temp1, + &temp2, + &onStackChrominanceQuantTable, + unzig.Data); + prevDCCr = this.WriteBlock( + QuantIndex.Chrominance, + prevDCCr, + &cr, + &temp1, + &temp2, + &onStackChrominanceQuantTable, + unzig.Data); + } } } } @@ -912,6 +916,7 @@ namespace ImageSharp.Formats private void Encode420(PixelAccessor pixels) where TColor : struct, IPackedPixel, IEquatable { + // TODO: Need a JpegEncoderCoreCore class or struct to hold all this mess: Block8x8F b = default(Block8x8F); BlockQuad cb = default(BlockQuad); @@ -930,45 +935,48 @@ namespace ImageSharp.Formats // ReSharper disable once InconsistentNaming float prevDCY = 0, prevDCCb = 0, prevDCCr = 0; - for (int y = 0; y < pixels.Height; y += 16) + using (var rgbBytes = new PixelArea(8, 8, ComponentOrder.XYZ, true)) { - for (int x = 0; x < pixels.Width; x += 16) + for (int y = 0; y < pixels.Height; y += 16) { - for (int i = 0; i < 4; i++) + for (int x = 0; x < pixels.Width; x += 16) { - int xOff = (i & 1) * 8; - int yOff = (i & 2) * 4; - - ToYCbCr(pixels, x + xOff, y + yOff, &b, cbPtr + i, crPtr + i); - - prevDCY = this.WriteBlock( - QuantIndex.Luminance, - prevDCY, + for (int i = 0; i < 4; i++) + { + int xOff = (i & 1) * 8; + int yOff = (i & 2) * 4; + + ToYCbCr(pixels, x + xOff, y + yOff, &b, cbPtr + i, crPtr + i, rgbBytes); + + prevDCY = this.WriteBlock( + QuantIndex.Luminance, + prevDCY, + &b, + &temp1, + &temp2, + &onStackLuminanceQuantTable, + unzig.Data); + } + + Block8x8F.Scale16X16To8X8(&b, cbPtr); + prevDCCb = this.WriteBlock( + QuantIndex.Chrominance, + prevDCCb, &b, &temp1, &temp2, - &onStackLuminanceQuantTable, + &onStackChrominanceQuantTable, + unzig.Data); + Block8x8F.Scale16X16To8X8(&b, crPtr); + prevDCCr = this.WriteBlock( + QuantIndex.Chrominance, + prevDCCr, + &b, + &temp1, + &temp2, + &onStackChrominanceQuantTable, unzig.Data); } - - Block8x8F.Scale16X16To8X8(&b, cbPtr); - prevDCCb = this.WriteBlock( - QuantIndex.Chrominance, - prevDCCb, - &b, - &temp1, - &temp2, - &onStackChrominanceQuantTable, - unzig.Data); - Block8x8F.Scale16X16To8X8(&b, crPtr); - prevDCCr = this.WriteBlock( - QuantIndex.Chrominance, - prevDCCr, - &b, - &temp1, - &temp2, - &onStackChrominanceQuantTable, - unzig.Data); } } } diff --git a/src/ImageSharp/Image/PixelArea{TColor}.cs b/src/ImageSharp/Image/PixelArea{TColor}.cs index f8e46a6cc..51e788d19 100644 --- a/src/ImageSharp/Image/PixelArea{TColor}.cs +++ b/src/ImageSharp/Image/PixelArea{TColor}.cs @@ -28,6 +28,11 @@ namespace ImageSharp /// private IntPtr dataPointer; + /// + /// True if was rented from by the constructor + /// + private bool isBufferRented; + /// /// A value indicating whether this instance of the given entity has been disposed. /// @@ -39,11 +44,6 @@ namespace ImageSharp /// private bool isDisposed; - /// - /// True if was allocated by the constructor (rented from ) - /// - private bool isBufferOwner; - /// /// Initializes a new instance of the class. /// @@ -90,8 +90,9 @@ namespace ImageSharp /// The width. /// The height. /// The component order. - public PixelArea(int width, int height, ComponentOrder componentOrder) - : this(width, height, componentOrder, 0) + /// True if the buffer should be rented from ArrayPool + public PixelArea(int width, int height, ComponentOrder componentOrder, bool usePool = false) + : this(width, height, componentOrder, 0, usePool) { } @@ -123,14 +124,27 @@ namespace ImageSharp /// The height. /// The component order. /// The number of bytes to pad each row. - public PixelArea(int width, int height, ComponentOrder componentOrder, int padding) + /// True if the buffer should be rented from ArrayPool + public PixelArea(int width, int height, ComponentOrder componentOrder, int padding, bool usePool = false) { this.Width = width; this.Height = height; this.ComponentOrder = componentOrder; this.RowByteCount = (width * GetComponentCount(componentOrder)) + padding; - this.Bytes = BytesPool.Rent(this.RowByteCount * height); - this.isBufferOwner = true; + + var bufferSize = this.RowByteCount * height; + + if (usePool) + { + this.Bytes = BytesPool.Rent(bufferSize); + this.isBufferRented = true; + Array.Clear(this.Bytes, 0, bufferSize); + } + else + { + this.Bytes = new byte[bufferSize]; + } + this.pixelsHandle = GCHandle.Alloc(this.Bytes, GCHandleType.Pinned); // TODO: Why is Resharper warning us about an impure method call? @@ -184,6 +198,7 @@ namespace ImageSharp /// /// Gets the pool used to rent , when it's not coming from an external source /// + // ReSharper disable once StaticMemberInGenericType private static ArrayPool BytesPool => ArrayPool.Shared; /// @@ -217,7 +232,7 @@ namespace ImageSharp /// internal void Reset() { - Unsafe.InitBlock(this.PixelBase, 0, (uint)this.Bytes.Length); + Unsafe.InitBlock(this.PixelBase, 0, (uint)(this.RowByteCount * this.Height)); } /// @@ -274,7 +289,7 @@ namespace ImageSharp this.pixelsHandle.Free(); } - if (disposing && this.isBufferOwner) + if (disposing && this.isBufferRented) { BytesPool.Return(this.Bytes); }