From 64870f50166fff49be84fa81a79cbc39db5b1d68 Mon Sep 17 00:00:00 2001 From: jasonjyu Date: Tue, 31 Oct 2017 17:00:05 -0400 Subject: [PATCH 01/27] Removed commented code. --- tests/ImageSharp.Benchmarks/Color/RgbToYCbCr.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tests/ImageSharp.Benchmarks/Color/RgbToYCbCr.cs b/tests/ImageSharp.Benchmarks/Color/RgbToYCbCr.cs index 4048b25692..c4a77acc26 100644 --- a/tests/ImageSharp.Benchmarks/Color/RgbToYCbCr.cs +++ b/tests/ImageSharp.Benchmarks/Color/RgbToYCbCr.cs @@ -183,17 +183,6 @@ namespace SixLabors.ImageSharp.Benchmarks Vector3 vectorCb = VectorCb * vectorRgb; Vector3 vectorCr = VectorCr * vectorRgb; - // Should be better in theory, but came out to be worse: :( - // Vector3 c = new Vector3(0, 128, 128); - // Vector3 xx = new Vector3(vectorY.X, vectorCb.X, vectorCr.X); - // Vector3 yy = new Vector3(vectorY.Y, -vectorCb.Y, -vectorCr.Y); - // Vector3 zz = new Vector3(vectorY.Z, vectorCb.Z, -vectorCr.Z); - - // c += xx + yy + zz; - // *yPtr++ = c.X; - // *cbPtr++ = c.Y; - // *crPtr++ = c.Z; - *yPtr++ = vectorY.X + vectorY.Y + vectorY.Z; *cbPtr++ = 128 + (vectorCb.X - vectorCb.Y + vectorCb.Z); *crPtr++ = 128 + (vectorCr.X - vectorCr.Y - vectorCr.Z); @@ -375,4 +364,4 @@ namespace SixLabors.ImageSharp.Benchmarks } } } -} \ No newline at end of file +} From eecffcce6cae92c1e549a68efd952d467bf974ec Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 4 Dec 2017 23:49:05 +1100 Subject: [PATCH 02/27] Use clean buffers. Fix #390 --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 34 ++++++++------------ src/ImageSharp/Formats/Gif/LzwDecoder.cs | 2 +- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 453197b0ca..80ee3d130e 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -7,6 +7,7 @@ using System.IO; using System.Runtime.CompilerServices; using System.Text; using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.MetaData; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; @@ -38,7 +39,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The global color table. /// - private byte[] globalColorTable; + private Buffer globalColorTable; /// /// The global color table length @@ -123,10 +124,10 @@ namespace SixLabors.ImageSharp.Formats.Gif if (this.logicalScreenDescriptor.GlobalColorTableFlag) { this.globalColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize * 3; - this.globalColorTable = ArrayPool.Shared.Rent(this.globalColorTableLength); + this.globalColorTable = Buffer.CreateClean(this.globalColorTableLength); // Read the global color table from the stream - stream.Read(this.globalColorTable, 0, this.globalColorTableLength); + stream.Read(this.globalColorTable.Array, 0, this.globalColorTableLength); } // Loop though the respective gif parts and read the data. @@ -175,10 +176,7 @@ namespace SixLabors.ImageSharp.Formats.Gif } finally { - if (this.globalColorTable != null) - { - ArrayPool.Shared.Return(this.globalColorTable); - } + this.globalColorTable?.Dispose(); } return this.image; @@ -309,19 +307,19 @@ namespace SixLabors.ImageSharp.Formats.Gif { GifImageDescriptor imageDescriptor = this.ReadImageDescriptor(); - byte[] localColorTable = null; - byte[] indices = null; + Buffer localColorTable = null; + Buffer indices = null; try { // Determine the color table for this frame. If there is a local one, use it otherwise use the global color table. if (imageDescriptor.LocalColorTableFlag) { int length = imageDescriptor.LocalColorTableSize * 3; - localColorTable = ArrayPool.Shared.Rent(length); - this.currentStream.Read(localColorTable, 0, length); + localColorTable = Buffer.CreateClean(length); + this.currentStream.Read(localColorTable.Array, 0, length); } - indices = ArrayPool.Shared.Rent(imageDescriptor.Width * imageDescriptor.Height); + indices = Buffer.CreateClean(imageDescriptor.Width * imageDescriptor.Height); this.ReadFrameIndices(imageDescriptor, indices); this.ReadFrameColors(indices, localColorTable ?? this.globalColorTable, imageDescriptor); @@ -331,12 +329,8 @@ namespace SixLabors.ImageSharp.Formats.Gif } finally { - if (localColorTable != null) - { - ArrayPool.Shared.Return(localColorTable); - } - - ArrayPool.Shared.Return(indices); + localColorTable?.Dispose(); + indices?.Dispose(); } } @@ -346,7 +340,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The . /// The pixel array to write to. [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void ReadFrameIndices(GifImageDescriptor imageDescriptor, byte[] indices) + private void ReadFrameIndices(GifImageDescriptor imageDescriptor, Span indices) { int dataSize = this.currentStream.ReadByte(); using (var lzwDecoder = new LzwDecoder(this.currentStream)) @@ -361,7 +355,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The indexed pixels. /// The color table containing the available colors. /// The - private void ReadFrameColors(byte[] indices, byte[] colorTable, GifImageDescriptor descriptor) + private void ReadFrameColors(Span indices, Span colorTable, GifImageDescriptor descriptor) { int imageWidth = this.logicalScreenDescriptor.Width; int imageHeight = this.logicalScreenDescriptor.Height; diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index b8f12f930a..3284dad657 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -83,7 +83,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The height of the pixel index array. /// Size of the data. /// The pixel array to decode to. - public void DecodePixels(int width, int height, int dataSize, byte[] pixels) + public void DecodePixels(int width, int height, int dataSize, Span pixels) { Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize)); From 02ba124667906c9ec81070373ee016d3076ba32b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 12:27:56 +1100 Subject: [PATCH 03/27] Use ref --- src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index 44311e080b..d58a960b53 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -179,9 +179,8 @@ namespace SixLabors.ImageSharp.Quantizers float b = Volume(this.colorCube[k], this.vmb) / weight; float a = Volume(this.colorCube[k], this.vma) / weight; - var color = default(TPixel); + ref TPixel color = ref this.palette[k]; color.PackFromVector4(new Vector4(r, g, b, a) / 255F); - this.palette[k] = color; } } } From b219fda083b0ba0537e65e572e22f178e284c0f8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 14:00:29 +1100 Subject: [PATCH 04/27] Fix quantized output --- .../Quantizers/WuQuantizer{TPixel}.cs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index d58a960b53..6397864067 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -196,19 +196,21 @@ namespace SixLabors.ImageSharp.Quantizers var rgba = default(Rgba32); pixel.ToRgba32(ref rgba); - int r = rgba.R >> 2; // 8 - IndexBits - int g = rgba.G >> 2; - int b = rgba.B >> 2; - int a = rgba.A >> 5; // 8 - IndexAlphaBits - - int ind = GetPaletteIndex(r + 1, g + 1, b + 1, a + 1); - - this.vwt[ind]++; - this.vmr[ind] += r; - this.vmg[ind] += g; - this.vmb[ind] += b; - this.vma[ind] += a; - this.m2[ind] += (r * r) + (g * g) + (b * b) + (a * a); + int r = rgba.R >> (8 - IndexBits); + int g = rgba.G >> (8 - IndexBits); + int b = rgba.B >> (8 - IndexBits); + int a = rgba.A >> (8 - IndexAlphaBits); + + int index = GetPaletteIndex(r + 1, g + 1, b + 1, a + 1); + + this.vwt[index]++; + this.vmr[index] += rgba.R; + this.vmg[index] += rgba.G; + this.vmb[index] += rgba.B; + this.vma[index] += rgba.A; + + var vector = new Vector4(rgba.R, rgba.G, rgba.B, rgba.A); + this.m2[index] += Vector4.Dot(vector, vector); } /// @@ -617,14 +619,12 @@ namespace SixLabors.ImageSharp.Quantizers float halfA = baseA + Top(cube, direction, i, this.vma); float halfW = baseW + Top(cube, direction, i, this.vwt); - float temp; - if (MathF.Abs(halfW) < Constants.Epsilon) { continue; } - temp = ((halfR * halfR) + (halfG * halfG) + (halfB * halfB) + (halfA * halfA)) / halfW; + float temp = ((halfR * halfR) + (halfG * halfG) + (halfB * halfB) + (halfA * halfA)) / halfW; halfR = wholeR - halfR; halfG = wholeG - halfG; From a7ffeffcec5c1f410bb787f9b5dfd3ffecf73990 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 14:22:23 +1100 Subject: [PATCH 05/27] Minor optimization --- src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index 6397864067..629bc1431b 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -174,13 +174,13 @@ namespace SixLabors.ImageSharp.Quantizers if (MathF.Abs(weight) > Constants.Epsilon) { - float r = Volume(this.colorCube[k], this.vmr) / weight; - float g = Volume(this.colorCube[k], this.vmg) / weight; - float b = Volume(this.colorCube[k], this.vmb) / weight; - float a = Volume(this.colorCube[k], this.vma) / weight; + float r = Volume(this.colorCube[k], this.vmr); + float g = Volume(this.colorCube[k], this.vmg); + float b = Volume(this.colorCube[k], this.vmb); + float a = Volume(this.colorCube[k], this.vma); ref TPixel color = ref this.palette[k]; - color.PackFromVector4(new Vector4(r, g, b, a) / 255F); + color.PackFromVector4(new Vector4(r, g, b, a) / weight / 255F); } } } From 31b4a6420d43b5f06ce31d124bf26c335ce2ade2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 14:35:51 +1100 Subject: [PATCH 06/27] Use struct for Box --- src/ImageSharp/Quantizers/Box.cs | 5 +- .../Quantizers/WuQuantizer{TPixel}.cs | 93 +++++++++---------- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/src/ImageSharp/Quantizers/Box.cs b/src/ImageSharp/Quantizers/Box.cs index 4a1e17753f..cd1936b653 100644 --- a/src/ImageSharp/Quantizers/Box.cs +++ b/src/ImageSharp/Quantizers/Box.cs @@ -5,9 +5,8 @@ namespace SixLabors.ImageSharp.Quantizers { /// /// Represents a box color cube. - /// TODO: This should be a struct for performance /// - internal sealed class Box + internal struct Box { /// /// Gets or sets the min red value, exclusive. @@ -54,4 +53,4 @@ namespace SixLabors.ImageSharp.Quantizers /// public int Volume { get; set; } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index 629bc1431b..11d9bd5dbd 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -168,16 +168,16 @@ namespace SixLabors.ImageSharp.Quantizers this.palette = new TPixel[this.colors]; for (int k = 0; k < this.colors; k++) { - this.Mark(this.colorCube[k], (byte)k); + this.Mark(ref this.colorCube[k], (byte)k); - float weight = Volume(this.colorCube[k], this.vwt); + float weight = Volume(ref this.colorCube[k], this.vwt); if (MathF.Abs(weight) > Constants.Epsilon) { - float r = Volume(this.colorCube[k], this.vmr); - float g = Volume(this.colorCube[k], this.vmg); - float b = Volume(this.colorCube[k], this.vmb); - float a = Volume(this.colorCube[k], this.vma); + float r = Volume(ref this.colorCube[k], this.vmr); + float g = Volume(ref this.colorCube[k], this.vmg); + float b = Volume(ref this.colorCube[k], this.vmb); + float a = Volume(ref this.colorCube[k], this.vma); ref TPixel color = ref this.palette[k]; color.PackFromVector4(new Vector4(r, g, b, a) / weight / 255F); @@ -302,7 +302,7 @@ namespace SixLabors.ImageSharp.Quantizers /// The cube. /// The moment. /// The result. - private static float Volume(Box cube, long[] moment) + private static float Volume(ref Box cube, long[] moment) { return moment[GetPaletteIndex(cube.R1, cube.G1, cube.B1, cube.A1)] - moment[GetPaletteIndex(cube.R1, cube.G1, cube.B1, cube.A0)] @@ -329,7 +329,7 @@ namespace SixLabors.ImageSharp.Quantizers /// The direction. /// The moment. /// The result. - private static long Bottom(Box cube, int direction, long[] moment) + private static long Bottom(ref Box cube, int direction, long[] moment) { switch (direction) { @@ -390,7 +390,7 @@ namespace SixLabors.ImageSharp.Quantizers /// The position. /// The moment. /// The result. - private static long Top(Box cube, int direction, int position, long[] moment) + private static long Top(ref Box cube, int direction, int position, long[] moment) { switch (direction) { @@ -554,12 +554,12 @@ namespace SixLabors.ImageSharp.Quantizers /// /// The cube. /// The . - private float Variance(Box cube) + private float Variance(ref Box cube) { - float dr = Volume(cube, this.vmr); - float dg = Volume(cube, this.vmg); - float db = Volume(cube, this.vmb); - float da = Volume(cube, this.vma); + float dr = Volume(ref cube, this.vmr); + float dg = Volume(ref cube, this.vmg); + float db = Volume(ref cube, this.vmb); + float da = Volume(ref cube, this.vma); float xx = this.m2[GetPaletteIndex(cube.R1, cube.G1, cube.B1, cube.A1)] @@ -579,7 +579,8 @@ namespace SixLabors.ImageSharp.Quantizers - this.m2[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A1)] + this.m2[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A0)]; - return xx - (((dr * dr) + (dg * dg) + (db * db) + (da * da)) / Volume(cube, this.vwt)); + // TODO: Vector.Dot + return xx - (((dr * dr) + (dg * dg) + (db * db) + (da * da)) / Volume(ref cube, this.vwt)); } /// @@ -600,24 +601,24 @@ namespace SixLabors.ImageSharp.Quantizers /// The whole alpha. /// The whole weight. /// The . - private float Maximize(Box cube, int direction, int first, int last, out int cut, float wholeR, float wholeG, float wholeB, float wholeA, float wholeW) + private float Maximize(ref Box cube, int direction, int first, int last, out int cut, float wholeR, float wholeG, float wholeB, float wholeA, float wholeW) { - long baseR = Bottom(cube, direction, this.vmr); - long baseG = Bottom(cube, direction, this.vmg); - long baseB = Bottom(cube, direction, this.vmb); - long baseA = Bottom(cube, direction, this.vma); - long baseW = Bottom(cube, direction, this.vwt); + long baseR = Bottom(ref cube, direction, this.vmr); + long baseG = Bottom(ref cube, direction, this.vmg); + long baseB = Bottom(ref cube, direction, this.vmb); + long baseA = Bottom(ref cube, direction, this.vma); + long baseW = Bottom(ref cube, direction, this.vwt); float max = 0F; cut = -1; for (int i = first; i < last; i++) { - float halfR = baseR + Top(cube, direction, i, this.vmr); - float halfG = baseG + Top(cube, direction, i, this.vmg); - float halfB = baseB + Top(cube, direction, i, this.vmb); - float halfA = baseA + Top(cube, direction, i, this.vma); - float halfW = baseW + Top(cube, direction, i, this.vwt); + float halfR = baseR + Top(ref cube, direction, i, this.vmr); + float halfG = baseG + Top(ref cube, direction, i, this.vmg); + float halfB = baseB + Top(ref cube, direction, i, this.vmb); + float halfA = baseA + Top(ref cube, direction, i, this.vma); + float halfW = baseW + Top(ref cube, direction, i, this.vwt); if (MathF.Abs(halfW) < Constants.Epsilon) { @@ -655,18 +656,18 @@ namespace SixLabors.ImageSharp.Quantizers /// The first set. /// The second set. /// Returns a value indicating whether the box has been split. - private bool Cut(Box set1, Box set2) + private bool Cut(ref Box set1, ref Box set2) { - float wholeR = Volume(set1, this.vmr); - float wholeG = Volume(set1, this.vmg); - float wholeB = Volume(set1, this.vmb); - float wholeA = Volume(set1, this.vma); - float wholeW = Volume(set1, this.vwt); + float wholeR = Volume(ref set1, this.vmr); + float wholeG = Volume(ref set1, this.vmg); + float wholeB = Volume(ref set1, this.vmb); + float wholeA = Volume(ref set1, this.vma); + float wholeW = Volume(ref set1, this.vwt); - float maxr = this.Maximize(set1, 0, set1.R0 + 1, set1.R1, out int cutr, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxg = this.Maximize(set1, 1, set1.G0 + 1, set1.G1, out int cutg, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxb = this.Maximize(set1, 2, set1.B0 + 1, set1.B1, out int cutb, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxa = this.Maximize(set1, 3, set1.A0 + 1, set1.A1, out int cuta, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxr = this.Maximize(ref set1, 0, set1.R0 + 1, set1.R1, out int cutr, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxg = this.Maximize(ref set1, 1, set1.G0 + 1, set1.G1, out int cutg, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxb = this.Maximize(ref set1, 2, set1.B0 + 1, set1.B1, out int cutb, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxa = this.Maximize(ref set1, 3, set1.A0 + 1, set1.A1, out int cuta, wholeR, wholeG, wholeB, wholeA, wholeW); int dir; @@ -743,7 +744,7 @@ namespace SixLabors.ImageSharp.Quantizers /// /// The cube. /// A label. - private void Mark(Box cube, byte label) + private void Mark(ref Box cube, byte label) { for (int r = cube.R0 + 1; r <= cube.R1; r++) { @@ -768,23 +769,19 @@ namespace SixLabors.ImageSharp.Quantizers this.colorCube = new Box[this.colors]; float[] vv = new float[this.colors]; - for (int i = 0; i < this.colors; i++) - { - this.colorCube[i] = new Box(); - } - - this.colorCube[0].R0 = this.colorCube[0].G0 = this.colorCube[0].B0 = this.colorCube[0].A0 = 0; - this.colorCube[0].R1 = this.colorCube[0].G1 = this.colorCube[0].B1 = IndexCount - 1; - this.colorCube[0].A1 = IndexAlphaCount - 1; + ref var cube = ref this.colorCube[0]; + cube.R0 = cube.G0 = cube.B0 = cube.A0 = 0; + cube.R1 = cube.G1 = cube.B1 = IndexCount - 1; + cube.A1 = IndexAlphaCount - 1; int next = 0; for (int i = 1; i < this.colors; i++) { - if (this.Cut(this.colorCube[next], this.colorCube[i])) + if (this.Cut(ref this.colorCube[next], ref this.colorCube[i])) { - vv[next] = this.colorCube[next].Volume > 1 ? this.Variance(this.colorCube[next]) : 0F; - vv[i] = this.colorCube[i].Volume > 1 ? this.Variance(this.colorCube[i]) : 0F; + vv[next] = this.colorCube[next].Volume > 1 ? this.Variance(ref this.colorCube[next]) : 0F; + vv[i] = this.colorCube[i].Volume > 1 ? this.Variance(ref this.colorCube[i]) : 0F; } else { From 5f734e18b8991488a933012365e79cfc90190e27 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 14:59:14 +1100 Subject: [PATCH 07/27] Use Vector4.Dot where we can --- .../Quantizers/WuQuantizer{TPixel}.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index 11d9bd5dbd..ba1a3c1659 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -579,8 +579,8 @@ namespace SixLabors.ImageSharp.Quantizers - this.m2[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A1)] + this.m2[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A0)]; - // TODO: Vector.Dot - return xx - (((dr * dr) + (dg * dg) + (db * db) + (da * da)) / Volume(ref cube, this.vwt)); + var vector = new Vector4(dr, dg, db, da); + return xx - (Vector4.Dot(vector, vector) / Volume(ref cube, this.vwt)); } /// @@ -625,12 +625,9 @@ namespace SixLabors.ImageSharp.Quantizers continue; } - float temp = ((halfR * halfR) + (halfG * halfG) + (halfB * halfB) + (halfA * halfA)) / halfW; + var vector = new Vector4(halfR, halfG, halfB, halfA); + float temp = Vector4.Dot(vector, vector) / halfW; - halfR = wholeR - halfR; - halfG = wholeG - halfG; - halfB = wholeB - halfB; - halfA = wholeA - halfA; halfW = wholeW - halfW; if (MathF.Abs(halfW) < Constants.Epsilon) @@ -638,7 +635,14 @@ namespace SixLabors.ImageSharp.Quantizers continue; } - temp += ((halfR * halfR) + (halfG * halfG) + (halfB * halfB) + (halfA * halfA)) / halfW; + halfR = wholeR - halfR; + halfG = wholeG - halfG; + halfB = wholeB - halfB; + halfA = wholeA - halfA; + + vector = new Vector4(halfR, halfG, halfB, halfA); + + temp += Vector4.Dot(vector, vector) / halfW; if (temp > max) { From 4876a0087fb8e3b672b5a5f0b3089cf21ea0062c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 15:31:58 +1100 Subject: [PATCH 08/27] Use same ordering as original code Makes debugging a lot easier --- .../Quantizers/WuQuantizer{TPixel}.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index ba1a3c1659..7409021689 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -334,7 +334,7 @@ namespace SixLabors.ImageSharp.Quantizers switch (direction) { // Red - case 0: + case 3: return -moment[GetPaletteIndex(cube.R0, cube.G1, cube.B1, cube.A1)] + moment[GetPaletteIndex(cube.R0, cube.G1, cube.B1, cube.A0)] + moment[GetPaletteIndex(cube.R0, cube.G1, cube.B0, cube.A1)] @@ -345,7 +345,7 @@ namespace SixLabors.ImageSharp.Quantizers + moment[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A0)]; // Green - case 1: + case 2: return -moment[GetPaletteIndex(cube.R1, cube.G0, cube.B1, cube.A1)] + moment[GetPaletteIndex(cube.R1, cube.G0, cube.B1, cube.A0)] + moment[GetPaletteIndex(cube.R1, cube.G0, cube.B0, cube.A1)] @@ -356,7 +356,7 @@ namespace SixLabors.ImageSharp.Quantizers + moment[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A0)]; // Blue - case 2: + case 1: return -moment[GetPaletteIndex(cube.R1, cube.G1, cube.B0, cube.A1)] + moment[GetPaletteIndex(cube.R1, cube.G1, cube.B0, cube.A0)] + moment[GetPaletteIndex(cube.R1, cube.G0, cube.B0, cube.A1)] @@ -367,7 +367,7 @@ namespace SixLabors.ImageSharp.Quantizers + moment[GetPaletteIndex(cube.R0, cube.G0, cube.B0, cube.A0)]; // Alpha - case 3: + case 0: return -moment[GetPaletteIndex(cube.R1, cube.G1, cube.B1, cube.A0)] + moment[GetPaletteIndex(cube.R1, cube.G1, cube.B0, cube.A0)] + moment[GetPaletteIndex(cube.R1, cube.G0, cube.B1, cube.A0)] @@ -395,7 +395,7 @@ namespace SixLabors.ImageSharp.Quantizers switch (direction) { // Red - case 0: + case 3: return moment[GetPaletteIndex(position, cube.G1, cube.B1, cube.A1)] - moment[GetPaletteIndex(position, cube.G1, cube.B1, cube.A0)] - moment[GetPaletteIndex(position, cube.G1, cube.B0, cube.A1)] @@ -406,7 +406,7 @@ namespace SixLabors.ImageSharp.Quantizers - moment[GetPaletteIndex(position, cube.G0, cube.B0, cube.A0)]; // Green - case 1: + case 2: return moment[GetPaletteIndex(cube.R1, position, cube.B1, cube.A1)] - moment[GetPaletteIndex(cube.R1, position, cube.B1, cube.A0)] - moment[GetPaletteIndex(cube.R1, position, cube.B0, cube.A1)] @@ -417,7 +417,7 @@ namespace SixLabors.ImageSharp.Quantizers - moment[GetPaletteIndex(cube.R0, position, cube.B0, cube.A0)]; // Blue - case 2: + case 1: return moment[GetPaletteIndex(cube.R1, cube.G1, position, cube.A1)] - moment[GetPaletteIndex(cube.R1, cube.G1, position, cube.A0)] - moment[GetPaletteIndex(cube.R1, cube.G0, position, cube.A1)] @@ -428,7 +428,7 @@ namespace SixLabors.ImageSharp.Quantizers - moment[GetPaletteIndex(cube.R0, cube.G0, position, cube.A0)]; // Alpha - case 3: + case 0: return moment[GetPaletteIndex(cube.R1, cube.G1, cube.B1, position)] - moment[GetPaletteIndex(cube.R1, cube.G1, cube.B0, position)] - moment[GetPaletteIndex(cube.R1, cube.G0, cube.B1, position)] @@ -668,16 +668,16 @@ namespace SixLabors.ImageSharp.Quantizers float wholeA = Volume(ref set1, this.vma); float wholeW = Volume(ref set1, this.vwt); - float maxr = this.Maximize(ref set1, 0, set1.R0 + 1, set1.R1, out int cutr, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxg = this.Maximize(ref set1, 1, set1.G0 + 1, set1.G1, out int cutg, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxb = this.Maximize(ref set1, 2, set1.B0 + 1, set1.B1, out int cutb, wholeR, wholeG, wholeB, wholeA, wholeW); - float maxa = this.Maximize(ref set1, 3, set1.A0 + 1, set1.A1, out int cuta, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxr = this.Maximize(ref set1, 3, set1.R0 + 1, set1.R1, out int cutr, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxg = this.Maximize(ref set1, 2, set1.G0 + 1, set1.G1, out int cutg, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxb = this.Maximize(ref set1, 1, set1.B0 + 1, set1.B1, out int cutb, wholeR, wholeG, wholeB, wholeA, wholeW); + float maxa = this.Maximize(ref set1, 0, set1.A0 + 1, set1.A1, out int cuta, wholeR, wholeG, wholeB, wholeA, wholeW); int dir; if ((maxr >= maxg) && (maxr >= maxb) && (maxr >= maxa)) { - dir = 0; + dir = 3; if (cutr < 0) { @@ -686,15 +686,15 @@ namespace SixLabors.ImageSharp.Quantizers } else if ((maxg >= maxr) && (maxg >= maxb) && (maxg >= maxa)) { - dir = 1; + dir = 2; } else if ((maxb >= maxr) && (maxb >= maxg) && (maxb >= maxa)) { - dir = 2; + dir = 1; } else { - dir = 3; + dir = 0; } set2.R1 = set1.R1; @@ -705,7 +705,7 @@ namespace SixLabors.ImageSharp.Quantizers switch (dir) { // Red - case 0: + case 3: set2.R0 = set1.R1 = cutr; set2.G0 = set1.G0; set2.B0 = set1.B0; @@ -713,7 +713,7 @@ namespace SixLabors.ImageSharp.Quantizers break; // Green - case 1: + case 2: set2.G0 = set1.G1 = cutg; set2.R0 = set1.R0; set2.B0 = set1.B0; @@ -721,7 +721,7 @@ namespace SixLabors.ImageSharp.Quantizers break; // Blue - case 2: + case 1: set2.B0 = set1.B1 = cutb; set2.R0 = set1.R0; set2.G0 = set1.G0; @@ -729,7 +729,7 @@ namespace SixLabors.ImageSharp.Quantizers break; // Alpha - case 3: + case 0: set2.A0 = set1.A1 = cuta; set2.R0 = set1.R0; set2.G0 = set1.G0; From bf7c0163bb3079cb386cfab11387fadd59231d3c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2017 15:50:37 +1100 Subject: [PATCH 09/27] A little more ref --- src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs index 7409021689..cb9eb9b0e3 100644 --- a/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/WuQuantizer{TPixel}.cs @@ -782,10 +782,12 @@ namespace SixLabors.ImageSharp.Quantizers for (int i = 1; i < this.colors; i++) { - if (this.Cut(ref this.colorCube[next], ref this.colorCube[i])) + ref var nextCube = ref this.colorCube[next]; + ref var currentCube = ref this.colorCube[i]; + if (this.Cut(ref nextCube, ref currentCube)) { - vv[next] = this.colorCube[next].Volume > 1 ? this.Variance(ref this.colorCube[next]) : 0F; - vv[i] = this.colorCube[i].Volume > 1 ? this.Variance(ref this.colorCube[i]) : 0F; + vv[next] = nextCube.Volume > 1 ? this.Variance(ref nextCube) : 0F; + vv[i] = currentCube.Volume > 1 ? this.Variance(ref currentCube) : 0F; } else { @@ -805,7 +807,7 @@ namespace SixLabors.ImageSharp.Quantizers } } - if (temp <= 0.0) + if (temp <= 0F) { this.colors = i + 1; break; From d24decbae7b20976585c089e214720188ceb2e6f Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Fri, 8 Dec 2017 16:53:04 +0100 Subject: [PATCH 10/27] Removed invalid check reported in issue #394. --- src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index e7ffaa9d1a..61b18af551 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -688,7 +688,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort } int th = this.Temp[0] & 0x0f; - if (th > OrigHuffmanTree.MaxTh || (!this.IsProgressive && (th > 1))) + if (th > OrigHuffmanTree.MaxTh) { throw new ImageFormatException("Bad Th value"); } From bcd3371da019d61e9774d7ed0f19238bf6750a55 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 9 Dec 2017 14:29:11 +0100 Subject: [PATCH 11/27] add test for #394 + #395 --- tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/External | 2 +- .../Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg | 3 +++ 4 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index db42be2847..d529bbaeb6 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -42,6 +42,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Baseline.Jpeg444, TestImages.Jpeg.Baseline.Bad.BadEOF, TestImages.Jpeg.Baseline.Bad.ExifUndefType, + TestImages.Jpeg.Issues.MultiHuffmanBaseline394, }; public static string[] ProgressiveTestJpegs = diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f342542395..1b5f0dbadb 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -123,6 +123,7 @@ namespace SixLabors.ImageSharp.Tests public const string MissingFF00ProgressiveGirl159 = "Jpg/issues/Issue159-MissingFF00-Progressive-Girl.jpg"; public const string BadCoeffsProgressive178 = "Jpg/issues/Issue178-BadCoeffsProgressive-Lemon.jpg"; public const string BadZigZagProgressive385 = "Jpg/issues/Issue385-BadZigZag-Progressive.jpg"; + public const string MultiHuffmanBaseline394 = "Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/External b/tests/Images/External index ab7c90362a..dc5479d00b 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit ab7c90362a4881c790c6f264f400dcf4b4dea0ce +Subproject commit dc5479d00b2312f691e6249b9f7765e2316d4a30 diff --git a/tests/Images/Input/Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg b/tests/Images/Input/Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg new file mode 100644 index 0000000000..6e4dc0d0f3 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue394-MultiHuffmanBaseline-Speakers.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8e90cf157d8e2599598c1bea1b8e2bf262651b004cc6261d87bbb49bd6131034 +size 257401 From 554974f55faf95fa42752bd253d48bd3a6b20b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Leh=C3=B3czky?= Date: Sun, 10 Dec 2017 21:25:23 +0100 Subject: [PATCH 12/27] Typo in ResamplingWeightedProcessor Removing duplicated "to". Contribution of the year I guess :D. --- .../Transforms/ResamplingWeightedProcessor.Weights.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs b/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs index 1169d2eadc..61902c3a2f 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs @@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract partial class ResamplingWeightedProcessor { /// - /// Points to a collection of of weights allocated in . + /// Points to a collection of weights allocated in . /// internal struct WeightsWindow { @@ -196,4 +196,4 @@ namespace SixLabors.ImageSharp.Processing.Processors } } } -} \ No newline at end of file +} From 828c7ba6afb844a0bd82c17f45bb4bf9650f550d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Leh=C3=B3czky?= Date: Sun, 10 Dec 2017 21:34:30 +0100 Subject: [PATCH 13/27] Fixing more typos in various comments --- src/ImageSharp/Memory/Buffer2DExtensions.cs | 2 +- .../Transforms/ResamplingWeightedProcessor.Weights.cs | 2 +- .../Processing/Processors/Transforms/ResizeProcessor.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Memory/Buffer2DExtensions.cs b/src/ImageSharp/Memory/Buffer2DExtensions.cs index 401003eedd..ac5ab09dbd 100644 --- a/src/ImageSharp/Memory/Buffer2DExtensions.cs +++ b/src/ImageSharp/Memory/Buffer2DExtensions.cs @@ -70,7 +70,7 @@ namespace SixLabors.ImageSharp.Memory /// /// The element type /// The - /// The rectangel subarea + /// The rectangle subarea /// The public static BufferArea GetArea(this IBuffer2D buffer, Rectangle rectangle) where T : struct => new BufferArea(buffer, rectangle); diff --git a/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs b/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs index 61902c3a2f..22a7c90b75 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ResamplingWeightedProcessor.Weights.cs @@ -153,7 +153,7 @@ namespace SixLabors.ImageSharp.Processing.Processors } /// - /// Holds the values in an optimized contigous memory region. + /// Holds the values in an optimized contiguous memory region. /// internal class WeightsBuffer : IDisposable { diff --git a/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs index a4fdb1a1b4..17b42c5040 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs @@ -58,9 +58,9 @@ namespace SixLabors.ImageSharp.Processing.Processors // We will always be creating the clone even for mutate because thats the way this base processor works // ------------ // For resize we know we are going to populate every pixel with fresh data and we want a different target size so - // let's manually clone an empty set of images at the correct target and then have the base class processs them in turn. + // let's manually clone an empty set of images at the correct target and then have the base class process them in turn. IEnumerable> frames = source.Frames.Select(x => new ImageFrame(this.Width, this.Height, x.MetaData.Clone())); // this will create places holders - var image = new Image(config, source.MetaData.Clone(), frames); // base the place holder images in to prevet a extra frame being added + var image = new Image(config, source.MetaData.Clone(), frames); // base the place holder images in to prevent a extra frame being added return image; } From 68b402971f2bf9f016f1d7e48884681db0274f54 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 11 Dec 2017 21:52:59 +1100 Subject: [PATCH 14/27] Fix #390 --- src/ImageSharp/Image/PixelArea{TPixel}.cs | 4 ++-- .../Formats/Gif/GifDecoderTests.cs | 19 +++++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 4 +++- .../TestUtilities/TestImageExtensions.cs | 1 - tests/Images/Input/Gif/kumin.gif | 3 +++ tests/Images/Input/Png/icon.png | 3 +++ 6 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 tests/Images/Input/Gif/kumin.gif create mode 100644 tests/Images/Input/Png/icon.png diff --git a/src/ImageSharp/Image/PixelArea{TPixel}.cs b/src/ImageSharp/Image/PixelArea{TPixel}.cs index e9924f8235..1c7256455e 100644 --- a/src/ImageSharp/Image/PixelArea{TPixel}.cs +++ b/src/ImageSharp/Image/PixelArea{TPixel}.cs @@ -30,7 +30,7 @@ namespace SixLabors.ImageSharp /// /// The underlying buffer containing the raw pixel data. /// - private Buffer byteBuffer; + private readonly Buffer byteBuffer; /// /// Initializes a new instance of the class. @@ -116,7 +116,7 @@ namespace SixLabors.ImageSharp this.RowStride = (width * GetComponentCount(componentOrder)) + padding; this.Length = this.RowStride * height; - this.byteBuffer = new Buffer(this.Length); + this.byteBuffer = Buffer.CreateClean(this.Length); } /// diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index cd78add758..a4a27bd839 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -10,12 +10,15 @@ using Xunit; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests { + using SixLabors.ImageSharp.Advanced; + public class GifDecoderTests { private const PixelTypes PixelTypes = Tests.PixelTypes.Rgba32 | Tests.PixelTypes.RgbaVector | Tests.PixelTypes.Argb32; public static readonly string[] TestFiles = { TestImages.Gif.Giphy, TestImages.Gif.Rings, TestImages.Gif.Trans }; + [Theory] [WithFileCollection(nameof(TestFiles), PixelTypes)] public void DecodeAndReSave(TestImageProvider imageProvider) @@ -113,5 +116,21 @@ namespace SixLabors.ImageSharp.Tests Assert.True(image.Frames.Count > 1); } } + + [Fact] + public void CanDecodeIntermingledImages() + { + using (var kumin1 = Image.Load(TestFile.Create(TestImages.Gif.Kumin).Bytes)) + using (var icon = Image.Load(TestFile.Create(TestImages.Png.Icon).Bytes)) + using (var kumin2 = Image.Load(TestFile.Create(TestImages.Gif.Kumin).Bytes)) + { + for (int i = 0; i < kumin1.Frames.Count; i++) + { + ImageFrame first = kumin1.Frames[i]; + ImageFrame second = kumin2.Frames[i]; + first.ComparePixelBufferTo(second.GetPixelSpan()); + } + } + } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f342542395..ac4fe09e68 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -31,6 +31,7 @@ namespace SixLabors.ImageSharp.Tests public const string BikeGrayscale = "Png/BikeGrayscale.png"; public const string Rgb48BppInterlaced = "Png/rgb-48bpp-interlaced.png"; public const string SnakeGame = "Png/SnakeGame.png"; + public const string Icon = "Png/icon.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; @@ -153,8 +154,9 @@ namespace SixLabors.ImageSharp.Tests public const string Giphy = "Gif/giphy.gif"; public const string Cheers = "Gif/cheers.gif"; public const string Trans = "Gif/trans.gif"; + public const string Kumin = "Gif/kumin.gif"; - public static readonly string[] All = { Rings, Giphy, Cheers, Trans }; + public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin }; } } } diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index 505cdc1729..2b3cb1bcc3 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -29,7 +29,6 @@ namespace SixLabors.ImageSharp.Tests /// The image provider /// Details to be concatenated to the test output file, describing the parameters of the test. /// The extension - /// A boolean indicating whether we should save a smaller in size. /// A boolean indicating whether to append the pixel type to the output file name. public static Image DebugSave( this Image image, diff --git a/tests/Images/Input/Gif/kumin.gif b/tests/Images/Input/Gif/kumin.gif new file mode 100644 index 0000000000..31efda7d8c --- /dev/null +++ b/tests/Images/Input/Gif/kumin.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:345556b9a0e064412acc3b2ee87a9226113eb65c0a1791c2f855ac3fa1e6b7ad +size 868269 diff --git a/tests/Images/Input/Png/icon.png b/tests/Images/Input/Png/icon.png new file mode 100644 index 0000000000..bc355712b5 --- /dev/null +++ b/tests/Images/Input/Png/icon.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:18f1eb7c5019153f4a0b2de90e7e0f0521193f003fabd6ac31c2f58c2562ae42 +size 4040 From e884bc34903df40f270a739280ff9eae9194fc40 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Dec 2017 10:23:00 +1100 Subject: [PATCH 15/27] Fix #405 --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 9 +++++++-- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 13 +++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 6 ++++++ .../Gif/issues/issue405_badappextlength252-2.gif | 3 +++ .../Gif/issues/issue405_badappextlength252.gif | 3 +++ 5 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif create mode 100644 tests/Images/Input/Gif/issues/issue405_badappextlength252.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 80ee3d130e..de22f190a5 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -155,10 +155,15 @@ namespace SixLabors.ImageSharp.Formats.Gif this.ReadComments(); break; case GifConstants.ApplicationExtensionLabel: - this.Skip(12); // No need to read. + + // The application extension length should be 11 but we've got test images that incorrectly + // set this to 252. + int appLength = stream.ReadByte(); + this.Skip(appLength); // No need to read. break; case GifConstants.PlainTextLabel: - this.Skip(13); // Not supported by any known decoder. + int plainLength = stream.ReadByte(); + this.Skip(plainLength); // Not supported by any known decoder. break; } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index a4a27bd839..eb3e184c74 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -18,6 +18,7 @@ namespace SixLabors.ImageSharp.Tests public static readonly string[] TestFiles = { TestImages.Gif.Giphy, TestImages.Gif.Rings, TestImages.Gif.Trans }; + public static readonly string[] BadAppExtFiles = { TestImages.Gif.Issues.BadAppExtLength, TestImages.Gif.Issues.BadAppExtLength_2 }; [Theory] [WithFileCollection(nameof(TestFiles), PixelTypes)] @@ -30,6 +31,7 @@ namespace SixLabors.ImageSharp.Tests imageProvider.Utility.SaveTestOutputFile(image, "gif"); } } + [Theory] [WithFileCollection(nameof(TestFiles), PixelTypes)] public void DecodeResizeAndSave(TestImageProvider imageProvider) @@ -132,5 +134,16 @@ namespace SixLabors.ImageSharp.Tests } } } + + [Theory] + [WithFileCollection(nameof(BadAppExtFiles), PixelTypes)] + public void DecodeBadApplicationExtensionLength(TestImageProvider imageProvider) + where TPixel : struct, IPixel + { + using (Image image = imageProvider.GetImage()) + { + imageProvider.Utility.SaveTestOutputFile(image, "bmp"); + } + } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 7e98e50ade..300e169ee8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -157,6 +157,12 @@ namespace SixLabors.ImageSharp.Tests public const string Trans = "Gif/trans.gif"; public const string Kumin = "Gif/kumin.gif"; + public class Issues + { + public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; + public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; + } + public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin }; } } diff --git a/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif b/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif new file mode 100644 index 0000000000..98738c5118 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:151d394c7c57a1696fedc32116514b667ad70c4a2c7d34db7c489c17d5755eed +size 24724 diff --git a/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif b/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif new file mode 100644 index 0000000000..6fbc3713f8 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0549f4500693cdf7eac8351d8cd64d1093da9e9eaaed923e8c6f00476e350f43 +size 41721 From 124b349500e790567c1a2fe8c7329b4d06c9e95a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Dec 2017 11:01:34 +1100 Subject: [PATCH 16/27] Fix #403 --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 13 ++++++++++++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Gif/issues/issue403_baddescriptorwidth.gif | 3 +++ 4 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index de22f190a5..ae20be7d5d 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -443,7 +443,8 @@ namespace SixLabors.ImageSharp.Formats.Gif var rgba = new Rgba32(0, 0, 0, 255); - for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width; x++) + // #403 The left + width value can be larger than the image width + for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width && x < rowSpan.Length; x++) { int index = indices[i]; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index eb3e184c74..76a31c1864 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -136,7 +136,7 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [WithFileCollection(nameof(BadAppExtFiles), PixelTypes)] + [WithFileCollection(nameof(BadAppExtFiles), PixelTypes.Rgba32)] public void DecodeBadApplicationExtensionLength(TestImageProvider imageProvider) where TPixel : struct, IPixel { @@ -145,5 +145,16 @@ namespace SixLabors.ImageSharp.Tests imageProvider.Utility.SaveTestOutputFile(image, "bmp"); } } + + [Theory] + [WithFile(TestImages.Gif.Issues.BadDescriptorWidth, PixelTypes.Rgba32)] + public void DecodeBadDescriptorDimensionsLength(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + provider.Utility.SaveTestOutputFile(image, "bmp"); + } + } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 300e169ee8..365aea04ae 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -161,6 +161,7 @@ namespace SixLabors.ImageSharp.Tests { public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; + public const string BadDescriptorWidth = "Gif/issues/issue403_baddescriptorwidth.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin }; diff --git a/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif b/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif new file mode 100644 index 0000000000..0dce4b0eec --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c108091ffddd87178378656e37a7e975aa69be56376b9105adbbc14fe8d9a010 +size 707660 From 4907fa844b9ee6e603904c419ea7809cff4f4d2d Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 09:41:07 +0000 Subject: [PATCH 17/27] update version number generation system to remove gitversion --- appveyor.yml | 6 +-- build.cmd | 24 +--------- build.ps1 | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ gitversion.yml | 31 ------------- 4 files changed, 121 insertions(+), 59 deletions(-) create mode 100644 build.ps1 delete mode 100644 gitversion.yml diff --git a/appveyor.yml b/appveyor.yml index 5c548a71c8..580b9a1dfe 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -4,14 +4,10 @@ image: Visual Studio 2017 # prevent the double build when a branch has an active PR skip_branch_with_pr: true -install: - - choco install gitversion.portable -pre -y - before_build: - git submodule -q update --init - cmd: dotnet --version - - ps: c:\ProgramData\chocolatey\lib\gitversion.portable\tools\gitversion.exe /l console /output buildserver - + build_script: - cmd: build.cmd diff --git a/build.cmd b/build.cmd index 1ba1b3742e..6372b41253 100644 --- a/build.cmd +++ b/build.cmd @@ -1,29 +1,7 @@ @echo Off -SET versionCommand= -if not "%GitVersion_NuGetVersion%" == "" ( - SET versionCommand=/p:packageversion=%GitVersion_NuGetVersion% - @echo building with version set to '%GitVersion_NuGetVersion%' -) +PowerShell -NoProfile -ExecutionPolicy Bypass -Command "& '.\build.ps1'" -dotnet restore %versionCommand% - -ECHO Building projects -dotnet build -c Release %versionCommand% - -if not "%errorlevel%"=="0" goto failure - -if not "%CI%" == "True" ( - ECHO NOT on CI server running tests - dotnet test ./tests/ImageSharp.Tests/ImageSharp.Tests.csproj --no-build -c Release -) -if not "%errorlevel%"=="0" goto failure - -ECHO Packaging projects -dotnet pack ./src/ImageSharp/ -c Release --output ../../artifacts --no-build %versionCommand% -if not "%errorlevel%"=="0" goto failure - -dotnet pack ./src/ImageSharp.Drawing/ -c Release --output ../../artifacts --no-build %versionCommand% if not "%errorlevel%"=="0" goto failure :success diff --git a/build.ps1 b/build.ps1 new file mode 100644 index 0000000000..14610c1cc5 --- /dev/null +++ b/build.ps1 @@ -0,0 +1,119 @@ + +# lets calulat the correct version here +$fallbackVersion = "1.0.0"; +$version = '' + +if($env:APPVEYOR -eq "true"){ +$tagRegex = '^v?(\d+\.\d+\.\d+)(-([a-zA-Z]+)\.?(\d*))?$' + +# we are running on the build server +$isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex + + if($isVersionTag){ + + Write-Debug "Building commit tagged with a compatable version number" + + $version = $matches[1] + $postTag = $matches[3] + $count = $matches[4] + Write-Debug "version number: ${version} post tag: ${postTag} count: ${count}" + if("$postTag" -ne ""){ + $version = "${version}-${postTag}" + } + if("$count" -ne ""){ + $padded = $count.Trim().Trim('0').PadLeft(4,"0"); + Write-Debug "count '$count', padded '${padded}'" + + $version = "${version}${padded}" + } + }else { + + Write-Debug "Untagged" + $lastTag = (git tag --list --sort=-taggerdate) | Out-String + $list = $lastTag.Split("`n") + foreach ($tag in $list) { + + Write-Debug "testing ${tag}" + $tag = $tag.Trim(); + if($tag -match $tagRegex){ + Write-Debug "matched ${tag}" + $version = $matches[1]; + break; + } + } + + if("$version" -eq ""){ + $version = $fallbackVersion + Write-Debug "Failed to discover base version Fallback to '${version}'" + }else{ + + Write-Debug "Discovered base version from tags '${version}'" + } + + $buildNumber = $env:APPVEYOR_BUILD_NUMBER + + $buildNumber = "$buildNumber".Trim().Trim('0').PadLeft(4,"0"); + if("$env:APPVEYOR_PULL_REQUEST_NUMBER" -ne ""){ + + Write-Debug "building a PR" + # this is a PR + $version = "${version}-PullRequest${env:APPVEYOR_PULL_REQUEST_NUMBER}_${buildNumber}"; + }else{ + + Write-Debug "building a branch commit" + # this is a general branch commit + # if master use 'ci' prefix + # if other branch use 'branch' + $branch = $env:APPVEYOR_REPO_BRANCH + + if("$branch" -eq ""){ + $branch = ((git rev-parse --abbrev-ref HEAD) | Out-String).Trim() + + if("$branch" -eq ""){ + $branch = "unknown" + } + } + + $branch = $branch.Replace("/","-").ToLower() + + if($branch.ToLower() -eq "master"){ + $branch = "ci" + } + + $version = "${version}-${branch}${buildNumber}"; + } + } +} + +if("$env:APPVEYOR_API_URL" -ne ""){ + Invoke-RestMethod -Method "PUT" ` + -Uri "${env:APPVEYOR_API_URL}api/build" ` + -Body "{version:'${version}'}" ` + -ContentType = "application/json" +} + + +Write-Host "Building version '${version}'" + +$versionCmd ="/p:packageversion=${$version}" + +dotnet restore ${versionCmd} + +Write-Host "Building projects" +dotnet build -c Release ${versionCmd} + +if ($LASTEXITCODE ){ Exit $LASTEXITCODE } + +if ( $env:CI -ne "True") { + #ECHO NOT on CI server running tests + dotnet test ./tests/ImageSharp.Tests/ImageSharp.Tests.csproj --no-build -c Release +} +if ($LASTEXITCODE ){ Exit $LASTEXITCODE } + +# ECHO Packaging projects +dotnet pack ./src/ImageSharp/ -c Release --output ../../artifacts --no-build ${versionCmd} +if ($LASTEXITCODE ){ Exit $LASTEXITCODE } + +dotnet pack ./src/ImageSharp.Drawing/ -c Release --output ../../artifacts --no-build ${versionCmd} + +if ($LASTEXITCODE ){ Exit $LASTEXITCODE } diff --git a/gitversion.yml b/gitversion.yml deleted file mode 100644 index 9fae0d8c2f..0000000000 --- a/gitversion.yml +++ /dev/null @@ -1,31 +0,0 @@ -# to create a new package you create a new release/tag -# in github appveyor will build it without the -cixxx tag -# it will then be deployable cleanly to nuget.org - -branches: - master: - tag: ci - mode: ContinuousDeployment - increment: Minor - prevent-increment-of-merged-branch-version: false - track-merge-target: true - pull-request: - regex: (pull|pull\-requests|pr)[/-] - mode: ContinuousDelivery - tag: PullRequest - increment: Inherit - prevent-increment-of-merged-branch-version: false - tag-number-pattern: '[/-](?\d+)[-/]' - track-merge-target: false - tracks-release-branches: false - is-release-branch: false - otherbranches: - regex: '.*' - mode: ContinuousDeployment - tag: ci - increment: Patch - prevent-increment-of-merged-branch-version: false - track-merge-target: true - is-release-branch: false -ignore: - sha: [] \ No newline at end of file From 8fd7d1f158d19a9d5bd6336bd7483d42ca5de67f Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 09:42:58 +0000 Subject: [PATCH 18/27] remove erroneous '=' --- build.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.ps1 b/build.ps1 index 14610c1cc5..3cdb8f3d47 100644 --- a/build.ps1 +++ b/build.ps1 @@ -89,7 +89,7 @@ if("$env:APPVEYOR_API_URL" -ne ""){ Invoke-RestMethod -Method "PUT" ` -Uri "${env:APPVEYOR_API_URL}api/build" ` -Body "{version:'${version}'}" ` - -ContentType = "application/json" + -ContentType "application/json" } From 847e50755ed653fd144310037f927376c380e67d Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 09:45:52 +0000 Subject: [PATCH 19/27] update padding rules and master build tag --- build.ps1 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/build.ps1 b/build.ps1 index 3cdb8f3d47..618e839f75 100644 --- a/build.ps1 +++ b/build.ps1 @@ -21,6 +21,7 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex $version = "${version}-${postTag}" } if("$count" -ne ""){ + # for consistancy with previous releases we pad the counter to only 4 places $padded = $count.Trim().Trim('0').PadLeft(4,"0"); Write-Debug "count '$count', padded '${padded}'" @@ -51,8 +52,8 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex } $buildNumber = $env:APPVEYOR_BUILD_NUMBER - - $buildNumber = "$buildNumber".Trim().Trim('0').PadLeft(4,"0"); + # build number replacement is padded to 6 places + $buildNumber = "$buildNumber".Trim().Trim('0').PadLeft(6,"0"); if("$env:APPVEYOR_PULL_REQUEST_NUMBER" -ne ""){ Write-Debug "building a PR" @@ -77,7 +78,7 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex $branch = $branch.Replace("/","-").ToLower() if($branch.ToLower() -eq "master"){ - $branch = "ci" + $branch = "dev" } $version = "${version}-${branch}${buildNumber}"; From 90c66b6fb1259dd8a727ec4967691527d1fb4988 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 10:12:24 +0000 Subject: [PATCH 20/27] build version in artefact path --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 580b9a1dfe..8321209905 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -15,8 +15,8 @@ test_script: - tests\CodeCoverage\CodeCoverage.cmd after_test: - - cmd: appveyor PushArtifact "artifacts\SixLabors.ImageSharp.%GitVersion_NuGetVersion%.nupkg" - - cmd: appveyor PushArtifact "artifacts\SixLabors.ImageSharp.Drawing.%GitVersion_NuGetVersion%.nupkg" + - cmd: appveyor PushArtifact "artifacts\SixLabors.ImageSharp.%APPVEYOR_BUILD_VERSION%.nupkg" + - cmd: appveyor PushArtifact "artifacts\SixLabors.ImageSharp.Drawing.%APPVEYOR_BUILD_VERSION%.nupkg" deploy: # MyGet Deployment for builds & releases From 157cb96bd305a14681970c78409af2dfe2526ab6 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 10:47:18 +0000 Subject: [PATCH 21/27] fix version number build argument --- build.ps1 | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/build.ps1 b/build.ps1 index 618e839f75..cf92f18542 100644 --- a/build.ps1 +++ b/build.ps1 @@ -3,7 +3,6 @@ $fallbackVersion = "1.0.0"; $version = '' -if($env:APPVEYOR -eq "true"){ $tagRegex = '^v?(\d+\.\d+\.\d+)(-([a-zA-Z]+)\.?(\d*))?$' # we are running on the build server @@ -84,7 +83,6 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex $version = "${version}-${branch}${buildNumber}"; } } -} if("$env:APPVEYOR_API_URL" -ne ""){ Invoke-RestMethod -Method "PUT" ` @@ -96,12 +94,10 @@ if("$env:APPVEYOR_API_URL" -ne ""){ Write-Host "Building version '${version}'" -$versionCmd ="/p:packageversion=${$version}" - -dotnet restore ${versionCmd} +dotnet restore /p:packageversion=$version Write-Host "Building projects" -dotnet build -c Release ${versionCmd} +dotnet build -c Release /p:packageversion=$version if ($LASTEXITCODE ){ Exit $LASTEXITCODE } @@ -112,9 +108,9 @@ if ( $env:CI -ne "True") { if ($LASTEXITCODE ){ Exit $LASTEXITCODE } # ECHO Packaging projects -dotnet pack ./src/ImageSharp/ -c Release --output ../../artifacts --no-build ${versionCmd} +dotnet pack ./src/ImageSharp/ -c Release --output ../../artifacts --no-build /p:packageversion=$version if ($LASTEXITCODE ){ Exit $LASTEXITCODE } -dotnet pack ./src/ImageSharp.Drawing/ -c Release --output ../../artifacts --no-build ${versionCmd} +dotnet pack ./src/ImageSharp.Drawing/ -c Release --output ../../artifacts --no-build /p:packageversion=$version if ($LASTEXITCODE ){ Exit $LASTEXITCODE } From 6ab5f7288ff566522f5ab46b14cbee29a650bcbf Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 11:47:58 +0000 Subject: [PATCH 22/27] cleanup --- build.ps1 | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/build.ps1 b/build.ps1 index cf92f18542..c1e882f9c5 100644 --- a/build.ps1 +++ b/build.ps1 @@ -51,19 +51,17 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex } $buildNumber = $env:APPVEYOR_BUILD_NUMBER + # build number replacement is padded to 6 places $buildNumber = "$buildNumber".Trim().Trim('0').PadLeft(6,"0"); if("$env:APPVEYOR_PULL_REQUEST_NUMBER" -ne ""){ - Write-Debug "building a PR" # this is a PR $version = "${version}-PullRequest${env:APPVEYOR_PULL_REQUEST_NUMBER}_${buildNumber}"; }else{ - Write-Debug "building a branch commit" + # this is a general branch commit - # if master use 'ci' prefix - # if other branch use 'branch' $branch = $env:APPVEYOR_REPO_BRANCH if("$branch" -eq ""){ @@ -85,15 +83,14 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex } if("$env:APPVEYOR_API_URL" -ne ""){ + # update appveyor build number for this build Invoke-RestMethod -Method "PUT" ` -Uri "${env:APPVEYOR_API_URL}api/build" ` -Body "{version:'${version}'}" ` -ContentType "application/json" } - Write-Host "Building version '${version}'" - dotnet restore /p:packageversion=$version Write-Host "Building projects" @@ -102,15 +99,13 @@ dotnet build -c Release /p:packageversion=$version if ($LASTEXITCODE ){ Exit $LASTEXITCODE } if ( $env:CI -ne "True") { - #ECHO NOT on CI server running tests dotnet test ./tests/ImageSharp.Tests/ImageSharp.Tests.csproj --no-build -c Release } if ($LASTEXITCODE ){ Exit $LASTEXITCODE } -# ECHO Packaging projects +Write-Host "Packaging projects" dotnet pack ./src/ImageSharp/ -c Release --output ../../artifacts --no-build /p:packageversion=$version if ($LASTEXITCODE ){ Exit $LASTEXITCODE } dotnet pack ./src/ImageSharp.Drawing/ -c Release --output ../../artifacts --no-build /p:packageversion=$version - if ($LASTEXITCODE ){ Exit $LASTEXITCODE } From ac27a4783a527fb219ed592556a682426a552406 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Wed, 20 Dec 2017 12:51:55 +0000 Subject: [PATCH 23/27] fix pr build number --- build.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build.ps1 b/build.ps1 index c1e882f9c5..2f4d1c949f 100644 --- a/build.ps1 +++ b/build.ps1 @@ -56,8 +56,10 @@ $isVersionTag = $env:APPVEYOR_REPO_TAG_NAME -match $tagRegex $buildNumber = "$buildNumber".Trim().Trim('0').PadLeft(6,"0"); if("$env:APPVEYOR_PULL_REQUEST_NUMBER" -ne ""){ Write-Debug "building a PR" + + $prNumber = "$env:APPVEYOR_PULL_REQUEST_NUMBER".Trim().Trim('0').PadLeft(5,"0"); # this is a PR - $version = "${version}-PullRequest${env:APPVEYOR_PULL_REQUEST_NUMBER}_${buildNumber}"; + $version = "${version}-PullRequest${prNumber}${buildNumber}"; }else{ Write-Debug "building a branch commit" From 3bc7816da7560501d8f26d966458f88e83bb3d72 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 24 Dec 2017 14:29:48 +1100 Subject: [PATCH 24/27] Reuse buffer and fix error messaging --- .../Formats/Png/Zlib/ZlibInflateStream.cs | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs index 36d1d62e71..fd9c4ac63e 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs @@ -2,10 +2,8 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; using System.IO; using System.IO.Compression; -using System.Text; namespace SixLabors.ImageSharp.Formats.Png.Zlib { @@ -14,6 +12,13 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// internal sealed class ZlibInflateStream : Stream { + /// + /// Used to read the Adler-32 and Crc-32 checksums + /// We don't actually use this for anything so it doesn't + /// have to be threadsafe. + /// + private static readonly byte[] ChecksumBuffer = new byte[4]; + /// /// The inner raw memory stream /// @@ -38,9 +43,9 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib private bool isDisposed; /// - /// The read crc data. + /// Whether the crc value has been read. /// - private byte[] crcread; + private bool crcRead; /// /// The current data remaining to be read @@ -149,14 +154,12 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.compressedStream.Dispose(); this.compressedStream = null; - if (this.crcread == null) + if (!this.crcRead) { // Consume the trailing 4 bytes - this.crcread = new byte[4]; - for (int i = 0; i < 4; i++) - { - this.crcread[i] = (byte)this.innerStream.ReadByte(); - } + this.innerStream.Read(ChecksumBuffer, 0, 4); + this.currentDataRemaining -= 4; + this.crcRead = true; } } } @@ -171,11 +174,6 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib private void InitializeInflateStream() { - // The DICT dictionary identifier identifying the used dictionary. - - // The preset dictionary. - bool fdict; - // Read the zlib header : http://tools.ietf.org/html/rfc1950 // CMF(Compression Method and flags) // This byte is divided into a 4 - bit compression method and a @@ -195,30 +193,35 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib return; } - if ((cmf & 0x0f) != 8) + if ((cmf & 0x0F) == 8) { - throw new Exception($"Bad compression method for ZLIB header: cmf={cmf}"); - } + // CINFO is the base-2 logarithm of the LZ77 window size, minus eight. + int cinfo = (cmf & 0xF0) >> 4; - // CINFO is the base-2 logarithm of the LZ77 window size, minus eight. - // int cinfo = ((cmf & (0xf0)) >> 8); - fdict = (flag & 32) != 0; + if (cinfo > 7) + { + // Values of CINFO above 7 are not allowed in RFC1950. + // CINFO is not defined in this specification for CM not equal to 8. + throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}"); + } + } + else + { + throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}"); + } + // The preset dictionary. + bool fdict = (flag & 32) != 0; if (fdict) { - // The DICT dictionary identifier identifying the used dictionary. - byte[] dictId = new byte[4]; - - for (int i = 0; i < 4; i++) - { - // We consume but don't use this. - dictId[i] = (byte)this.innerStream.ReadByte(); - this.currentDataRemaining--; - } + // We don't need this for inflate so simply skip by the next four bytes. + // https://tools.ietf.org/html/rfc1950#page-6 + this.innerStream.Read(ChecksumBuffer, 0, 4); + this.currentDataRemaining -= 4; } // Initialize the deflate Stream. this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true); } } -} +} \ No newline at end of file From 1ab5511439ba2b5b5e77be6a2e835bcc3b72ed25 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Sun, 24 Dec 2017 19:17:49 +0000 Subject: [PATCH 25/27] Offset pixel grid when rendering with anti-aliasing turned off Fixes #412 --- .../Processors/FillRegionProcessor.cs | 13 ++++- .../Drawing/SolidPolygonTests.cs | 8 +-- tests/ImageSharp.Tests/Issues/Issue412.cs | 58 +++++++++++++++++++ 3 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 tests/ImageSharp.Tests/Issues/Issue412.cs diff --git a/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs b/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs index d867008d7e..d307e67d59 100644 --- a/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs +++ b/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs @@ -82,8 +82,15 @@ namespace SixLabors.ImageSharp.Drawing.Processors int maxIntersections = region.MaxIntersections; float subpixelCount = 4; + + // we need to offset the pixel grid to account for when we outline a path. + // basically if the line is [1,2] => [3,2] then when outlining at 1 we end up with a region of [0.5,1.5],[1.5, 1.5],[3.5,2.5],[2.5,2.5] + // and this can cause missed fills when not using antialiasing.so we offset the pixel grid by 0.5 in the x & y direction thus causing the# + // region to alline with the pixel grid. + float offset = 0.5f; if (this.Options.Antialias) { + offset = 0f; // we are antialising skip offsetting as real antalising should take care of offset. subpixelCount = this.Options.AntialiasSubpixelDepth; if (subpixelCount < 4) { @@ -117,7 +124,7 @@ namespace SixLabors.ImageSharp.Drawing.Processors float subpixelFractionPoint = subpixelFraction / subpixelCount; for (float subPixel = (float)y; subPixel < y + 1; subPixel += subpixelFraction) { - int pointsFound = region.Scan(subPixel, buffer, 0); + int pointsFound = region.Scan(subPixel + offset, buffer, 0); if (pointsFound == 0) { // nothing on this line skip @@ -131,8 +138,8 @@ namespace SixLabors.ImageSharp.Drawing.Processors // points will be paired up float scanStart = buffer[point] - minX; float scanEnd = buffer[point + 1] - minX; - int startX = (int)MathF.Floor(scanStart); - int endX = (int)MathF.Floor(scanEnd); + int startX = (int)MathF.Floor(scanStart + offset); + int endX = (int)MathF.Floor(scanEnd + offset); if (startX >= 0 && startX < scanline.Length) { diff --git a/tests/ImageSharp.Tests/Drawing/SolidPolygonTests.cs b/tests/ImageSharp.Tests/Drawing/SolidPolygonTests.cs index c210b66ed7..be7c8adb08 100644 --- a/tests/ImageSharp.Tests/Drawing/SolidPolygonTests.cs +++ b/tests/ImageSharp.Tests/Drawing/SolidPolygonTests.cs @@ -81,13 +81,13 @@ namespace SixLabors.ImageSharp.Tests.Drawing using (PixelAccessor sourcePixels = image.Lock()) { - Assert.Equal(Rgba32.HotPink, sourcePixels[11, 11]); + Assert.True(Rgba32.HotPink == sourcePixels[11, 11], "[11, 11] wrong"); - Assert.Equal(Rgba32.HotPink, sourcePixels[199, 150]); + Assert.True(Rgba32.HotPink == sourcePixels[199, 149], "[199, 149] wrong"); - Assert.Equal(Rgba32.HotPink, sourcePixels[50, 50]); + Assert.True(Rgba32.HotPink == sourcePixels[50, 50], "[50, 50] wrong"); - Assert.Equal(Rgba32.Blue, sourcePixels[2, 2]); + Assert.True(Rgba32.Blue == sourcePixels[2, 2], "[2, 2] wrong"); } } } diff --git a/tests/ImageSharp.Tests/Issues/Issue412.cs b/tests/ImageSharp.Tests/Issues/Issue412.cs new file mode 100644 index 0000000000..b77112ba68 --- /dev/null +++ b/tests/ImageSharp.Tests/Issues/Issue412.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; +using System.Text; +using SixLabors.Primitives; +using SixLabors.ImageSharp.Advanced; +using Xunit; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Tests.Issues +{ + public class Issue412 + { + [Theory] + [WithBlankImages(40, 30, PixelTypes.Rgba32)] + public void AllPixelsExpectedToBeRedWhenAntialisedDisabled(TestImageProvider provider) where TPixel : struct, IPixel + { + using (var image = provider.GetImage()) + { + image.Mutate( + context => + { + for (var i = 0; i < 40; ++i) + { + context.DrawLines( + NamedColors.Black, + 1, + new[] + { + new PointF(i, 0.1066f), + new PointF(i, 10.1066f) + }, + new GraphicsOptions(true)); + + context.DrawLines( + NamedColors.Red, + 1, + new[] + { + new PointF(i, 15.1066f), + new PointF(i, 25.1066f) + }, + new GraphicsOptions(false)); + } + }); + + image.DebugSave(provider); + for (var y = 15; y < 25; y++) + { + for (var x = 0; x < 40; x++) + { + + Assert.True(NamedColors.Red.Equals(image[x, y]), $"expected {NamedColors.Red} but found {image[x, y]} at [{x}, {y}]"); + } + } + } + } + } +} From 48c2a913368bb4641c2b2ff192bb4d95c166ec69 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Sun, 24 Dec 2017 19:33:59 +0000 Subject: [PATCH 26/27] Ensure extact half full renders a pixel It can end up missing some pixels if they happen to fill exactly half the pixel --- src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs b/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs index d307e67d59..b6ef4be218 100644 --- a/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs +++ b/src/ImageSharp.Drawing/Processors/FillRegionProcessor.cs @@ -176,7 +176,7 @@ namespace SixLabors.ImageSharp.Drawing.Processors { for (int x = 0; x < scanlineWidth; x++) { - if (scanline[x] > 0.5) + if (scanline[x] >= 0.5) { scanline[x] = 1; } From adc784b94abfcafe6e3a97fae9753a9231f5ead4 Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Tue, 26 Dec 2017 10:24:23 +0100 Subject: [PATCH 27/27] Fixed invalid value for description. --- src/ImageSharp/MetaData/Profiles/Exif/ExifTag.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifTag.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifTag.cs index 5d2ef1436f..625f95b63d 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifTag.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifTag.cs @@ -975,7 +975,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif [ExifTagDescription((ushort)71, "Fired, Red-eye reduction, Return detected")] [ExifTagDescription((ushort)73, "On, Red-eye reduction")] [ExifTagDescription((ushort)77, "On, Red-eye reduction, Return not detected")] - [ExifTagDescription((ushort)69, "On, Red-eye reduction, Return detected")] + [ExifTagDescription((ushort)79, "On, Red-eye reduction, Return detected")] [ExifTagDescription((ushort)80, "Off, Red-eye reduction")] [ExifTagDescription((ushort)88, "Auto, Did not fire, Red-eye reduction")] [ExifTagDescription((ushort)89, "Auto, Fired, Red-eye reduction")]