Now throws a better excpetion DrawImage source does not overlap target (#877)

* No longer throws when DrawImage source does not overlap target

Previously, when DrawImage was used to overlay an image, in cases where the source image did not overlap the target image, a very confusing error was reported: "System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
Parameter name: MaxDegreeOfParallelism"

Now, when this case happens, the DrawImage method will simply not affect the target image, which is the same way FillRegionProcessor handles such cases.

ParallelHelper.IterRows also now does more validation of the input rectangle so that any further cases of this kind of problem throw a more relvant exception. Note I switched from DebugGuard to Guard because IterRows is a public API and thus should always validate its inputs.

Fixes #875

* Refines DrawImage non-overlap error logic

Adresses PR feedback in #877.

Changes DrawImage shortcut to be less than or equal to. Also changes maxX calculation to use `Right` over `Width`. This is a semantic change that reflects intention better. No actual change because Top,Left for that rectanngle should always be 0,0.

And adds more informative argument names to ParallelHelper.IterRows error message.

* Non-overlapping DrawImage now throws

Adressing PR feedback from #877

DrawImage now throws when the source image does not overlap, with a useful error message.

Also improved the error messages for IterRows (and added validation to the other IterRows method)

* DrawImage overlap test changed to support RELEASE

The tests on the CI server are run in RELEASE which wrap the expected exception with an ImageProcessingException.

* Adress feedback for DrawImage exception

DrawImage throws an ImageProcessor exception which makes it easier to catch.

And reverted IterRows to use Guard helpers

Author: atruskie

src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs

@@ -71,7 +71,7 @@ protected override void OnFrameApply(ImageFrame<TPixelDst> source, Rectangle sou
             Rectangle bounds = targetImage.Bounds();
 
             int minX = Math.Max(this.Location.X, sourceRectangle.X);
-            int maxX = Math.Min(this.Location.X + bounds.Width, sourceRectangle.Width);
+            int maxX = Math.Min(this.Location.X + bounds.Width, sourceRectangle.Right);
             int targetX = minX - this.Location.X;
 
             int minY = Math.Max(this.Location.Y, sourceRectangle.Y);
@@ -81,6 +81,12 @@ protected override void OnFrameApply(ImageFrame<TPixelDst> source, Rectangle sou
 
             var workingRect = Rectangle.FromLTRB(minX, minY, maxX, maxY);
 
+            // not a valid operation because rectangle does not overlap with this image.
+            if (workingRect.Width <= 0 || workingRect.Height <= 0)
+            {
+                throw new ImageProcessingException("Cannot draw image because the source image does not overlap the target image.");
+            }
+
             ParallelHelper.IterateRows(
                 workingRect,
                 configuration,

src/ImageSharp/Common/ParallelUtils/ParallelHelper.cs

@@ -45,7 +45,7 @@ public static void IterateRows(
             in ParallelExecutionSettings parallelSettings,
             Action<RowInterval> body)
         {
-            DebugGuard.MustBeGreaterThan(rectangle.Width, 0, nameof(rectangle));
+            ValidateRectangle(rectangle);
 
             int maxSteps = DivideCeil(rectangle.Width * rectangle.Height, parallelSettings.MinimumPixelsProcessedPerTask);
 
@@ -87,6 +87,8 @@ public static void IterateRowsWithTempBuffer<T>(
             Action<RowInterval, Memory<T>> body)
             where T : unmanaged
         {
+            ValidateRectangle(rectangle);
+
             int maxSteps = DivideCeil(rectangle.Width * rectangle.Height, parallelSettings.MinimumPixelsProcessedPerTask);
 
             int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
@@ -142,5 +144,18 @@ public static void IterateRowsWithTempBuffer<T>(
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static int DivideCeil(int dividend, int divisor) => 1 + ((dividend - 1) / divisor);
+
+        private static void ValidateRectangle(Rectangle rectangle)
+        {
+            Guard.MustBeGreaterThan(
+                rectangle.Width,
+                0,
+                $"{nameof(rectangle)}.{nameof(rectangle.Width)}");
+
+            Guard.MustBeGreaterThan(
+                rectangle.Height,
+                0,
+                $"{nameof(rectangle)}.{nameof(rectangle.Height)}");
+        }
     }
 }

tests/ImageSharp.Tests/Drawing/DrawImageTest.cs

@@ -131,6 +131,48 @@ public void ImageShouldHandlePositiveLocation(TestImageProvider<Rgba32> provider
                 background.DebugSave(provider, testOutputDetails: "Positive");
             }
         }
+        [Theory]
+        [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32)]
+        public void ImageShouldHandlePositiveLocationTruncatedOverlay(TestImageProvider<Rgba32> provider)
+        {
+            using (Image<Rgba32> background = provider.GetImage())
+            using (var overlay = new Image<Rgba32>(50, 50))
+            {
+                overlay.Mutate(x => x.Fill(Rgba32.Black));
+
+                const int xy = 75;
+                Rgba32 backgroundPixel = background[xy - 1, xy - 1];
+                Rgba32 overlayPixel = overlay[0, 0];
+
+                background.Mutate(x => x.DrawImage(overlay, new Point(xy, xy), PixelColorBlendingMode.Normal, 1F));
+
+                Assert.Equal(Rgba32.White, backgroundPixel);
+                Assert.Equal(overlayPixel, background[xy, xy]);
+
+                background.DebugSave(provider, testOutputDetails: "PositiveTruncated");
+            }
+        }
+
+        [Theory]
+        [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32, -30, -30)]
+        [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32, 130, -30)]
+        [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32, 130, 130)]
+        [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32, -30, 130)]
+        public void NonOverlappingImageThrows(TestImageProvider<Rgba32> provider, int x, int y)
+        {
+            using (Image<Rgba32> background = provider.GetImage())
+            using (var overlay = new Image<Rgba32>(Configuration.Default, 10, 10, Rgba32.Black))
+            {
+                ImageProcessingException ex = Assert.Throws<ImageProcessingException>(Test);
+
+                Assert.Contains("does not overlap", ex.ToString());
+
+                void Test()
+                {
+                    background.Mutate(context => context.DrawImage(overlay, new Point(x, y), GraphicsOptions.Default));
+                }
+            }
+        }
 
         private static void VerifyImage<TPixel>(
             TestImageProvider<TPixel> provider,

tests/ImageSharp.Tests/Helpers/ParallelHelperTests.cs

@@ -9,6 +9,7 @@
 
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.ParallelUtils;
+using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.Memory;
 using SixLabors.Primitives;
 
@@ -334,5 +335,39 @@ void FillRow(int y, Buffer2D<Point> buffer)
                 TestImageExtensions.CompareBuffers(expected.Span, actual.Span);
             }
         }
+
+        [Theory]
+        [InlineData(0, 10)]
+        [InlineData(10, 0)]
+        [InlineData(-10, 10)]
+        [InlineData(10, -10)]
+        public void IterateRowsRequiresValidRectangle(int width, int height)
+        {
+            var parallelSettings = new ParallelExecutionSettings();
+
+            var rect = new Rectangle(0, 0, width, height);
+
+            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
+                () => ParallelHelper.IterateRows(rect, parallelSettings, (rows) => { }));
+
+            Assert.Contains(width <= 0 ? "Width" : "Height", ex.Message);
+        }
+
+        [Theory]
+        [InlineData(0, 10)]
+        [InlineData(10, 0)]
+        [InlineData(-10, 10)]
+        [InlineData(10, -10)]
+        public void IterateRowsWithTempBufferRequiresValidRectangle(int width, int height)
+        {
+            var parallelSettings = new ParallelExecutionSettings();
+
+            var rect = new Rectangle(0, 0, width, height);
+
+            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
+                () => ParallelHelper.IterateRowsWithTempBuffer<Rgba32>(rect, parallelSettings, (rows, memory) => { }));
+
+            Assert.Contains(width <= 0 ? "Width" : "Height", ex.Message);
+        }
     }
 }