Merge pull request #1646 from SixLabors/bp/tiff4bitaligned

Make sure encoding 4bit paletted tiff rows are byte aligned

src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs

@@ -202,7 +202,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff
 
             if (this.PlanarConfiguration == TiffPlanarConfiguration.Chunky)
             {
-                DebugGuard.IsTrue(plane == -1, "Excepted Chunky planar.");
+                DebugGuard.IsTrue(plane == -1, "Expected Chunky planar.");
                 bitsPerPixel = this.BitsPerPixel;
             }
             else

src/ImageSharp/Formats/Tiff/Writers/TiffPaletteWriter{TPixel}.cs

@@ -55,23 +55,38 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers
         /// <inheritdoc />
         protected override void EncodeStrip(int y, int height, TiffBaseCompressor compressor)
         {
-            Span<byte> pixels = GetStripPixels(((IPixelSource)this.quantizedImage).PixelBuffer, y, height);
+            Span<byte> indexedPixels = GetStripPixels(((IPixelSource)this.quantizedImage).PixelBuffer, y, height);
             if (this.BitsPerPixel == 4)
             {
-                using IMemoryOwner<byte> rows4bitBuffer = this.MemoryAllocator.Allocate<byte>(pixels.Length / 2);
+                int width = this.Image.Width;
+                int halfWidth = width >> 1;
+                int excess = (width & 1) * height; // (width % 2) * height
+                int rows4BitBufferLength = (halfWidth * height) + excess;
+                using IMemoryOwner<byte> rows4bitBuffer = this.MemoryAllocator.Allocate<byte>(rows4BitBufferLength);
                 Span<byte> rows4bit = rows4bitBuffer.GetSpan();
-                int idx = 0;
+                int idxPixels = 0;
-                for (int i = 0; i < rows4bit.Length; i++)
+                int idx4bitRows = 0;
+                for (int row = 0; row < height; row++)
                 {
-                    rows4bit[i] = (byte)((pixels[idx] << 4) | (pixels[idx + 1] & 0xF));
+                    for (int x = 0; x < halfWidth; x++)
-                    idx += 2;
+                    {
+                        rows4bit[idx4bitRows] = (byte)((indexedPixels[idxPixels] << 4) | (indexedPixels[idxPixels + 1] & 0xF));
+                        idxPixels += 2;
+                        idx4bitRows++;
+                    }
+
+                    // Make sure rows are byte-aligned.
+                    if (width % 2 != 0)
+                    {
+                        rows4bit[idx4bitRows++] = (byte)(indexedPixels[idxPixels++] << 4);
+                    }
                 }
 
-                compressor.CompressStrip(rows4bit, height);
+                compressor.CompressStrip(rows4bit.Slice(0, idx4bitRows), height);
             }
             else
             {
-                compressor.CompressStrip(pixels, height);
+                compressor.CompressStrip(indexedPixels, height);
             }
         }
 
@@ -91,7 +106,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Writers
             PixelOperations<TPixel>.Instance.ToRgb48(this.Configuration, quantizedColors, quantizedColorRgb48);
 
             // It can happen that the quantized colors are less than the expected maximum per channel.
-            var diffToMaxColors = this.maxColors - quantizedColors.Length;
+            int diffToMaxColors = this.maxColors - quantizedColors.Length;
 
             // In a TIFF ColorMap, all the Red values come first, followed by the Green values,
             // then the Blue values. Convert the quantized palette to this format.

tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs

@@ -4,7 +4,7 @@
 // ReSharper disable InconsistentNaming
 using System;
 using System.IO;
-
+using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Formats.Tiff;
 using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.PixelFormats;
@@ -37,6 +37,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff
         [InlineData(RgbUncompressed, 24, 256, 256, 300, 300, PixelResolutionUnit.PixelsPerInch)]
         [InlineData(SmallRgbDeflate, 24, 32, 32, 96, 96, PixelResolutionUnit.PixelsPerInch)]
         [InlineData(Calliphora_GrayscaleUncompressed, 8, 804, 1198, 96, 96, PixelResolutionUnit.PixelsPerInch)]
+        [InlineData(Flower4BitPalette, 4, 73, 43, 72, 72, PixelResolutionUnit.PixelsPerInch)]
         public void Identify(string imagePath, int expectedPixelSize, int expectedWidth, int expectedHeight, double expectedHResolution, double expectedVResolution, PixelResolutionUnit expectedResolutionUnit)
         {
             var testFile = TestFile.Create(imagePath);
@@ -91,6 +92,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff
         public void TiffDecoder_CanDecode_WithPalette<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel> => TestTiffDecoder(provider);
 
+        [Theory]
+        [WithFile(Rgb4BitPalette, PixelTypes.Rgba32)]
+        [WithFile(Flower4BitPalette, PixelTypes.Rgba32)]
+        [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)]
+        public void TiffDecoder_CanDecode_4Bit_WithPalette<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            if (TestEnvironment.IsWindows)
+            {
+                TestTiffDecoder(provider, new SystemDrawingReferenceDecoder(), useExactComparer: false, 0.01f);
+            }
+        }
+
         [Theory]
         [WithFile(GrayscaleDeflateMultistrip, PixelTypes.Rgba32)]
         [WithFile(RgbDeflateMultistrip, PixelTypes.Rgba32)]
@@ -155,12 +169,15 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff
             image.CompareToOriginalMultiFrame(provider, ImageComparer.Exact, ReferenceDecoder);
         }
 
-        private static void TestTiffDecoder<TPixel>(TestImageProvider<TPixel> provider)
+        private static void TestTiffDecoder<TPixel>(TestImageProvider<TPixel> provider, IImageDecoder referenceDecoder = null, bool useExactComparer = true, float compareTolerance = 0.001f)
             where TPixel : unmanaged, IPixel<TPixel>
         {
             using Image<TPixel> image = provider.GetImage(TiffDecoder);
             image.DebugSave(provider);
-            image.CompareToOriginal(provider, ImageComparer.Exact, ReferenceDecoder);
+            image.CompareToOriginal(
+                provider,
+                useExactComparer ? ImageComparer.Exact : ImageComparer.Tolerant(compareTolerance),
+                referenceDecoder ?? ReferenceDecoder);
         }
     }
 }

tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs

@@ -296,10 +296,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff
 
         [Theory]
         [WithFile(Rgb4BitPalette, PixelTypes.Rgba32)]
+        [WithFile(Flower4BitPalette, PixelTypes.Rgba32)]
+        [WithFile(Flower4BitPaletteGray, PixelTypes.Rgba32)]
         public void TiffEncoder_EncodeColorPalette_With4Bit_Works<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel> =>
             //// Note: The magick reference decoder does not support 4 bit tiff's, so we use our TIFF decoder instead.
-            TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.001f, imageDecoder: new TiffDecoder());
+            TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit4, TiffPhotometricInterpretation.PaletteColor, useExactComparer: false, compareTolerance: 0.003f, imageDecoder: new TiffDecoder());
 
         [Theory]
         [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)]
@@ -460,7 +462,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff
             TiffCompression compression = TiffCompression.None,
             TiffPredictor predictor = TiffPredictor.None,
             bool useExactComparer = true,
-            float compareTolerance = 0.01f,
+            float compareTolerance = 0.001f,
             IImageDecoder imageDecoder = null)
             where TPixel : unmanaged, IPixel<TPixel>
         {

tests/ImageSharp.Tests/TestImages.cs

@@ -558,6 +558,8 @@ namespace SixLabors.ImageSharp.Tests
             public const string RgbPalette = "Tiff/rgb_palette.tiff";
             public const string Rgb4BitPalette = "Tiff/bike_colorpalette_4bit.tiff";
             public const string RgbPaletteDeflate = "Tiff/rgb_palette_deflate.tiff";
+            public const string Flower4BitPalette = "Tiff/flower-palette-04.tiff";
+            public const string Flower4BitPaletteGray = "Tiff/flower-minisblack-04.tiff";
 
             public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff";
             public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff";

tests/Images/Input/Tiff/flower-minisblack-04.tiff

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:18991fca75a89b3d15c7f93dee0454e3943920b595ba16145ebc1fd8bd45b1f5
+size 1905

tests/Images/Input/Tiff/flower-palette-04.tiff

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:700ec8103b4197c415ba90d983a7d5f471f155fd5b1c952d86ee9becba898a1a
+size 2010