Merge branch 'main' into js/gif-fixes

Author: JimBobSquarePants

.github/workflows/build-and-test.yml

@@ -9,6 +9,7 @@ on:
   pull_request:
     branches:
       - main
+      - release/*
     types: [ labeled, opened, synchronize, reopened ]
 jobs:
   Build:

src/ImageSharp/Advanced/ParallelRowIterator.cs

@@ -50,7 +50,7 @@ public static void IterateRows<T>(
         int width = rectangle.Width;
         int height = rectangle.Height;
 
-        int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask);
+        int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
         int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
 
         // Avoid TPL overhead in this trivial case:
@@ -115,7 +115,7 @@ public static void IterateRows<T, TBuffer>(
         int width = rectangle.Width;
         int height = rectangle.Height;
 
-        int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask);
+        int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
         int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
         MemoryAllocator allocator = parallelSettings.MemoryAllocator;
         int bufferLength = Unsafe.AsRef(operation).GetRequiredBufferLength(rectangle);
@@ -180,7 +180,7 @@ public static void IterateRowIntervals<T>(
         int width = rectangle.Width;
         int height = rectangle.Height;
 
-        int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask);
+        int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
         int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
 
         // Avoid TPL overhead in this trivial case:
@@ -242,7 +242,7 @@ public static void IterateRowIntervals<T, TBuffer>(
         int width = rectangle.Width;
         int height = rectangle.Height;
 
-        int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask);
+        int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
         int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
         MemoryAllocator allocator = parallelSettings.MemoryAllocator;
         int bufferLength = Unsafe.AsRef(operation).GetRequiredBufferLength(rectangle);
@@ -270,7 +270,7 @@ public static void IterateRowIntervals<T, TBuffer>(
     }
 
     [MethodImpl(InliningOptions.ShortMethod)]
-    private static int DivideCeil(int dividend, int divisor) => 1 + ((dividend - 1) / divisor);
+    private static int DivideCeil(long dividend, int divisor) => (int)Math.Min(1 + ((dividend - 1) / divisor), int.MaxValue);
 
     private static void ValidateRectangle(Rectangle rectangle)
     {

src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs

@@ -212,7 +212,12 @@ public bool FindNextMarker()
     private int ReadStream()
     {
         int value = this.badData ? 0 : this.stream.ReadByte();
-        if (value == -1)
+
+        // We've encountered the end of the file stream which means there's no EOI marker or the marker has been read
+        // during decoding of the SOS marker.
+        // When reading individual bits 'badData' simply means we have hit a marker, When data is '0' and the stream is exhausted
+        // we know we have hit the EOI and completed decoding the scan buffer.
+        if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length))
         {
             // We've encountered the end of the file stream which means there's no EOI marker
             // in the image or the SOS marker has the wrong dimensions set.

src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs

@@ -4,6 +4,7 @@
 using System.Buffers;
 using System.Numerics;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
@@ -34,17 +35,25 @@ public OilPaintingProcessor(Configuration configuration, OilPaintingProcessor de
     /// <inheritdoc/>
     protected override void OnFrameApply(ImageFrame<TPixel> source)
     {
+        int levels = Math.Clamp(this.definition.Levels, 1, 255);
         int brushSize = Math.Clamp(this.definition.BrushSize, 1, Math.Min(source.Width, source.Height));
 
         using Buffer2D<TPixel> targetPixels = this.Configuration.MemoryAllocator.Allocate2D<TPixel>(source.Size());
 
         source.CopyTo(targetPixels);
 
-        RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, this.definition.Levels);
-        ParallelRowIterator.IterateRowIntervals(
+        RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, levels);
+        try
+        {
+            ParallelRowIterator.IterateRowIntervals(
             this.Configuration,
             this.SourceRectangle,
             in operation);
+        }
+        catch (Exception ex)
+        {
+            throw new ImageProcessingException("The OilPaintProcessor failed. The most likely reason is that a pixel component was outside of its' allowed range.", ex);
+        }
 
         Buffer2D<TPixel>.SwapOrCopyContent(source.PixelBuffer, targetPixels);
     }
@@ -105,18 +114,18 @@ public void Invoke(in RowInterval rows)
             Span<Vector4> targetRowVector4Span = targetRowBuffer.Memory.Span;
             Span<Vector4> targetRowAreaVector4Span = targetRowVector4Span.Slice(this.bounds.X, this.bounds.Width);
 
-            ref float binsRef = ref bins.GetReference();
-            ref int intensityBinRef = ref Unsafe.As<float, int>(ref binsRef);
-            ref float redBinRef = ref Unsafe.Add(ref binsRef, (uint)this.levels);
-            ref float blueBinRef = ref Unsafe.Add(ref redBinRef, (uint)this.levels);
-            ref float greenBinRef = ref Unsafe.Add(ref blueBinRef, (uint)this.levels);
+            Span<float> binsSpan = bins.GetSpan();
+            Span<int> intensityBinsSpan = MemoryMarshal.Cast<float, int>(binsSpan);
+            Span<float> redBinSpan = binsSpan[this.levels..];
+            Span<float> blueBinSpan = redBinSpan[this.levels..];
+            Span<float> greenBinSpan = blueBinSpan[this.levels..];
 
             for (int y = rows.Min; y < rows.Max; y++)
             {
                 Span<TPixel> sourceRowPixelSpan = this.source.DangerousGetRowSpan(y);
                 Span<TPixel> sourceRowAreaPixelSpan = sourceRowPixelSpan.Slice(this.bounds.X, this.bounds.Width);
 
-                PixelOperations<TPixel>.Instance.ToVector4(this.configuration, sourceRowAreaPixelSpan, sourceRowAreaVector4Span);
+                PixelOperations<TPixel>.Instance.ToVector4(this.configuration, sourceRowAreaPixelSpan, sourceRowAreaVector4Span, PixelConversionModifiers.Scale);
 
                 for (int x = this.bounds.X; x < this.bounds.Right; x++)
                 {
@@ -140,29 +149,29 @@ public void Invoke(in RowInterval rows)
                             int offsetX = x + fxr;
                             offsetX = Numerics.Clamp(offsetX, 0, maxX);
 
-                            Vector4 vector = sourceOffsetRow[offsetX].ToVector4();
+                            Vector4 vector = sourceOffsetRow[offsetX].ToScaledVector4();
 
                             float sourceRed = vector.X;
                             float sourceBlue = vector.Z;
                             float sourceGreen = vector.Y;
 
                             int currentIntensity = (int)MathF.Round((sourceBlue + sourceGreen + sourceRed) / 3F * (this.levels - 1));
 
-                            Unsafe.Add(ref intensityBinRef, (uint)currentIntensity)++;
-                            Unsafe.Add(ref redBinRef, (uint)currentIntensity) += sourceRed;
-                            Unsafe.Add(ref blueBinRef, (uint)currentIntensity) += sourceBlue;
-                            Unsafe.Add(ref greenBinRef, (uint)currentIntensity) += sourceGreen;
+                            intensityBinsSpan[currentIntensity]++;
+                            redBinSpan[currentIntensity] += sourceRed;
+                            blueBinSpan[currentIntensity] += sourceBlue;
+                            greenBinSpan[currentIntensity] += sourceGreen;
 
-                            if (Unsafe.Add(ref intensityBinRef, (uint)currentIntensity) > maxIntensity)
+                            if (intensityBinsSpan[currentIntensity] > maxIntensity)
                             {
-                                maxIntensity = Unsafe.Add(ref intensityBinRef, (uint)currentIntensity);
+                                maxIntensity = intensityBinsSpan[currentIntensity];
                                 maxIndex = currentIntensity;
                             }
                         }
 
-                        float red = MathF.Abs(Unsafe.Add(ref redBinRef, (uint)maxIndex) / maxIntensity);
-                        float blue = MathF.Abs(Unsafe.Add(ref blueBinRef, (uint)maxIndex) / maxIntensity);
-                        float green = MathF.Abs(Unsafe.Add(ref greenBinRef, (uint)maxIndex) / maxIntensity);
+                        float red = redBinSpan[maxIndex] / maxIntensity;
+                        float blue = blueBinSpan[maxIndex] / maxIntensity;
+                        float green = greenBinSpan[maxIndex] / maxIntensity;
                         float alpha = sourceRowVector4Span[x].W;
 
                         targetRowVector4Span[x] = new Vector4(red, green, blue, alpha);
@@ -171,7 +180,7 @@ public void Invoke(in RowInterval rows)
 
                 Span<TPixel> targetRowAreaPixelSpan = this.targetPixels.DangerousGetRowSpan(y).Slice(this.bounds.X, this.bounds.Width);
 
-                PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, targetRowAreaVector4Span, targetRowAreaPixelSpan);
+                PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, targetRowAreaVector4Span, targetRowAreaPixelSpan, PixelConversionModifiers.Scale);
             }
         }
     }

tests/ImageSharp.Benchmarks/Processing/OilPaint.cs

@@ -0,0 +1,19 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using BenchmarkDotNet.Attributes;
+using SixLabors.ImageSharp.PixelFormats;
+using SixLabors.ImageSharp.Processing;
+
+namespace SixLabors.ImageSharp.Benchmarks.Processing;
+
+[Config(typeof(Config.MultiFramework))]
+public class OilPaint
+{
+    [Benchmark]
+    public void DoOilPaint()
+    {
+        using Image<RgbaVector> image = new Image<RgbaVector>(1920, 1200, new(127, 191, 255));
+        image.Mutate(ctx => ctx.OilPaint());
+    }
+}

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs

@@ -325,4 +325,21 @@ public void Issue2478_DecodeWorks<TPixel>(TestImageProvider<TPixel> provider)
         image.DebugSave(provider);
         image.CompareToOriginal(provider);
     }
+
+    [Theory]
+    [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)]
+    public void DecodeHang<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        if (TestEnvironment.IsWindows &&
+            TestEnvironment.RunsOnCI)
+        {
+            // Windows CI runs consistently fail with OOM.
+            return;
+        }
+
+        using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
+        Assert.Equal(65503, image.Width);
+        Assert.Equal(65503, image.Height);
+    }
 }

tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs

@@ -2,6 +2,8 @@
 // Licensed under the Six Labors Split License.
 
 using System.Numerics;
+using System.Runtime.CompilerServices;
+using Castle.Core.Configuration;
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
@@ -406,6 +408,43 @@ void RowAction(RowInterval rows, Span<Rgba32> memory)
         Assert.Contains(width <= 0 ? "Width" : "Height", ex.Message);
     }
 
+    [Fact]
+    public void CanIterateWithoutIntOverflow()
+    {
+        ParallelExecutionSettings parallelSettings = ParallelExecutionSettings.FromConfiguration(Configuration.Default);
+        const int max = 100_000;
+
+        Rectangle rect = new(0, 0, max, max);
+        int intervalMaxY = 0;
+        void RowAction(RowInterval rows, Span<Rgba32> memory) => intervalMaxY = Math.Max(rows.Max, intervalMaxY);
+
+        TestRowOperation operation = new();
+        TestRowIntervalOperation<Rgba32> intervalOperation = new(RowAction);
+
+        ParallelRowIterator.IterateRows(Configuration.Default, rect, in operation);
+        Assert.Equal(max - 1, operation.MaxY.Value);
+
+        ParallelRowIterator.IterateRowIntervals<TestRowIntervalOperation<Rgba32>, Rgba32>(rect, in parallelSettings, in intervalOperation);
+        Assert.Equal(max, intervalMaxY);
+    }
+
+    private readonly struct TestRowOperation : IRowOperation
+    {
+        public TestRowOperation()
+        {
+        }
+
+        public StrongBox<int> MaxY { get; } = new StrongBox<int>();
+
+        public void Invoke(int y)
+        {
+            lock (this.MaxY)
+            {
+                this.MaxY.Value = Math.Max(y, this.MaxY.Value);
+            }
+        }
+    }
+
     private readonly struct TestRowIntervalOperation : IRowIntervalOperation
     {
         private readonly Action<RowInterval> action;

tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs

@@ -27,26 +27,29 @@ public class OilPaintTest
     [WithFileCollection(nameof(InputImages), nameof(OilPaintValues), PixelTypes.Rgba32)]
     public void FullImage<TPixel>(TestImageProvider<TPixel> provider, int levels, int brushSize)
         where TPixel : unmanaged, IPixel<TPixel>
-    {
-        provider.RunValidatingProcessorTest(
+        => provider.RunValidatingProcessorTest(
             x =>
             {
                 x.OilPaint(levels, brushSize);
                 return $"{levels}-{brushSize}";
             },
             ImageComparer.TolerantPercentage(0.01F),
             appendPixelTypeToFileName: false);
-    }
 
     [Theory]
     [WithFileCollection(nameof(InputImages), nameof(OilPaintValues), PixelTypes.Rgba32)]
     [WithTestPatternImages(nameof(OilPaintValues), 100, 100, PixelTypes.Rgba32)]
     public void InBox<TPixel>(TestImageProvider<TPixel> provider, int levels, int brushSize)
         where TPixel : unmanaged, IPixel<TPixel>
-    {
-        provider.RunRectangleConstrainedValidatingProcessorTest(
+        => provider.RunRectangleConstrainedValidatingProcessorTest(
             (x, rect) => x.OilPaint(levels, brushSize, rect),
             $"{levels}-{brushSize}",
             ImageComparer.TolerantPercentage(0.01F));
+
+    [Fact]
+    public void Issue2518_PixelComponentOutsideOfRange_ThrowsImageProcessingException()
+    {
+        using Image<RgbaVector> image = new(10, 10, new RgbaVector(1, 1, 100));
+        Assert.Throws<ImageProcessingException>(() => image.Mutate(ctx => ctx.OilPaint()));
     }
 }

tests/ImageSharp.Tests/TestImages.cs

@@ -291,6 +291,7 @@ public static class Issues
             public const string Issue2334_NotEnoughBytesA = "Jpg/issues/issue-2334-a.jpg";
             public const string Issue2334_NotEnoughBytesB = "Jpg/issues/issue-2334-b.jpg";
             public const string Issue2478_JFXX = "Jpg/issues/issue-2478-jfxx.jpg";
+            public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg";
 
             public static class Fuzz
             {