Skip to content

Commit

Permalink
Now throws a better excpetion DrawImage source does not overlap target (
Browse files Browse the repository at this point in the history
SixLabors#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 SixLabors#875

* Refines DrawImage non-overlap error logic

Adresses PR feedback in SixLabors#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 SixLabors#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
  • Loading branch information
atruskie authored and JimBobSquarePants committed Apr 30, 2019
1 parent 7771c83 commit 499e0bb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions tests/ImageSharp.Tests/Drawing/DrawImageTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 499e0bb

Please sign in to comment.