Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DrawImage offsetting issues and improve API parameter names. #2474

Merged
merged 4 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
162 changes: 81 additions & 81 deletions src/ImageSharp/Processing/Extensions/Drawing/DrawImageExtensions.cs

Large diffs are not rendered by default.

31 changes: 20 additions & 11 deletions src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,22 @@ public class DrawImageProcessor : IImageProcessor
/// Initializes a new instance of the <see cref="DrawImageProcessor"/> class.
/// </summary>
/// <param name="image">The image to blend.</param>
/// <param name="location">The location to draw the blended image.</param>
/// <param name="backgroundLocation">The location to draw the foreground image on the background.</param>
/// <param name="foregoundRectangle">The rectangular portion of the foreground image to draw.</param>
/// <param name="colorBlendingMode">The blending mode to use when drawing the image.</param>
/// <param name="alphaCompositionMode">The Alpha blending mode to use when drawing the image.</param>
/// <param name="opacity">The opacity of the image to blend.</param>
public DrawImageProcessor(
Image image,
Point location,
Point backgroundLocation,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add an overload with out the ForegroundRectangle for back compatability, as this will be an binary breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always forget these are public.

Rectangle foregoundRectangle,
PixelColorBlendingMode colorBlendingMode,
PixelAlphaCompositionMode alphaCompositionMode,
float opacity)
{
this.Image = image;
this.Location = location;
this.BackgroundLocation = backgroundLocation;
this.ForegroundRectangle = foregoundRectangle;
this.ColorBlendingMode = colorBlendingMode;
this.AlphaCompositionMode = alphaCompositionMode;
this.Opacity = opacity;
Expand All @@ -39,9 +42,14 @@ public class DrawImageProcessor : IImageProcessor
public Image Image { get; }

/// <summary>
/// Gets the location to draw the blended image.
/// Gets the location to draw the foreground image on the background.
/// </summary>
public Point Location { get; }
public Point BackgroundLocation { get; }

/// <summary>
/// Gets the rectangular portion of the foreground image to draw.
/// </summary>
public Rectangle ForegroundRectangle { get; }

/// <summary>
/// Gets the blending mode to use when drawing the image.
Expand All @@ -62,7 +70,7 @@ public class DrawImageProcessor : IImageProcessor
public IImageProcessor<TPixelBg> CreatePixelSpecificProcessor<TPixelBg>(Configuration configuration, Image<TPixelBg> source, Rectangle sourceRectangle)
where TPixelBg : unmanaged, IPixel<TPixelBg>
{
ProcessorFactoryVisitor<TPixelBg> visitor = new(configuration, this, source, sourceRectangle);
ProcessorFactoryVisitor<TPixelBg> visitor = new(configuration, this, source);
this.Image.AcceptVisitor(visitor);
return visitor.Result!;
}
Expand All @@ -73,14 +81,15 @@ private class ProcessorFactoryVisitor<TPixelBg> : IImageVisitor
private readonly Configuration configuration;
private readonly DrawImageProcessor definition;
private readonly Image<TPixelBg> source;
private readonly Rectangle sourceRectangle;

public ProcessorFactoryVisitor(Configuration configuration, DrawImageProcessor definition, Image<TPixelBg> source, Rectangle sourceRectangle)
public ProcessorFactoryVisitor(
Configuration configuration,
DrawImageProcessor definition,
Image<TPixelBg> source)
{
this.configuration = configuration;
this.definition = definition;
this.source = source;
this.sourceRectangle = sourceRectangle;
}

public IImageProcessor<TPixelBg>? Result { get; private set; }
Expand All @@ -91,8 +100,8 @@ public void Visit<TPixelFg>(Image<TPixelFg> image)
this.configuration,
image,
this.source,
this.sourceRectangle,
this.definition.Location,
this.definition.BackgroundLocation,
this.definition.ForegroundRectangle,
this.definition.ColorBlendingMode,
this.definition.AlphaCompositionMode,
this.definition.Opacity);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,36 +21,42 @@ internal class DrawImageProcessor<TPixelBg, TPixelFg> : ImageProcessor<TPixelBg>
/// Initializes a new instance of the <see cref="DrawImageProcessor{TPixelBg, TPixelFg}"/> class.
/// </summary>
/// <param name="configuration">The configuration which allows altering default behaviour or extending the library.</param>
/// <param name="image">The foreground <see cref="Image{TPixelFg}"/> to blend with the currently processing image.</param>
/// <param name="source">The source <see cref="Image{TPixelBg}"/> for the current processor instance.</param>
/// <param name="sourceRectangle">The source area to process for the current processor instance.</param>
/// <param name="location">The location to draw the blended image.</param>
/// <param name="foregroundImage">The foreground <see cref="Image{TPixelFg}"/> to blend with the currently processing image.</param>
/// <param name="backgroundImage">The source <see cref="Image{TPixelBg}"/> for the current processor instance.</param>
/// <param name="backgroundLocation">The location to draw the blended image.</param>
/// <param name="foregroundRectangle">The source area to process for the current processor instance.</param>
/// <param name="colorBlendingMode">The blending mode to use when drawing the image.</param>
/// <param name="alphaCompositionMode">The Alpha blending mode to use when drawing the image.</param>
/// <param name="alphaCompositionMode">The alpha blending mode to use when drawing the image.</param>
/// <param name="opacity">The opacity of the image to blend. Must be between 0 and 1.</param>
public DrawImageProcessor(
Configuration configuration,
Image<TPixelFg> image,
Image<TPixelBg> source,
Rectangle sourceRectangle,
Point location,
Image<TPixelFg> foregroundImage,
Image<TPixelBg> backgroundImage,
Point backgroundLocation,
Rectangle foregroundRectangle,
PixelColorBlendingMode colorBlendingMode,
PixelAlphaCompositionMode alphaCompositionMode,
float opacity)
: base(configuration, source, sourceRectangle)
: base(configuration, backgroundImage, backgroundImage.Bounds)
{
Guard.MustBeBetweenOrEqualTo(opacity, 0, 1, nameof(opacity));

this.Image = image;
this.ForegroundImage = foregroundImage;
this.ForegroundRectangle = foregroundRectangle;
this.Opacity = opacity;
this.Blender = PixelOperations<TPixelBg>.Instance.GetPixelBlender(colorBlendingMode, alphaCompositionMode);
this.Location = location;
this.BackgroundLocation = backgroundLocation;
}

/// <summary>
/// Gets the image to blend
/// </summary>
public Image<TPixelFg> Image { get; }
public Image<TPixelFg> ForegroundImage { get; }

/// <summary>
/// Gets the rectangular portion of the foreground image to draw.
/// </summary>
public Rectangle ForegroundRectangle { get; }

/// <summary>
/// Gets the opacity of the image to blend
Expand All @@ -65,43 +71,57 @@ internal class DrawImageProcessor<TPixelBg, TPixelFg> : ImageProcessor<TPixelBg>
/// <summary>
/// Gets the location to draw the blended image
/// </summary>
public Point Location { get; }
public Point BackgroundLocation { get; }

/// <inheritdoc/>
protected override void OnFrameApply(ImageFrame<TPixelBg> source)
{
Rectangle sourceRectangle = this.SourceRectangle;
Configuration configuration = this.Configuration;

Image<TPixelFg> targetImage = this.Image;
PixelBlender<TPixelBg> blender = this.Blender;
int locationY = this.Location.Y;
// Align the bounds so that both the source and targets are the same width and height for blending.
// We ensure that negative locations are subtracted from both bounds so that foreground images can partially overlap.
Rectangle foregroundRectangle = this.ForegroundRectangle;

// Align start/end positions.
Rectangle bounds = targetImage.Bounds;
// Sanitize the location so that we don't try and sample outside the image.
int left = this.BackgroundLocation.X;
int top = this.BackgroundLocation.Y;

int minX = Math.Max(this.Location.X, sourceRectangle.X);
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);
int maxY = Math.Min(this.Location.Y + bounds.Height, sourceRectangle.Bottom);

int width = maxX - minX;
if (this.BackgroundLocation.X < 0)
{
foregroundRectangle.Width += this.BackgroundLocation.X;
left = 0;
}

Rectangle workingRect = Rectangle.FromLTRB(minX, minY, maxX, maxY);
if (this.BackgroundLocation.Y < 0)
{
foregroundRectangle.Height += this.BackgroundLocation.Y;
top = 0;
}

// Not a valid operation because rectangle does not overlap with this image.
if (workingRect.Width <= 0 || workingRect.Height <= 0)
int width = foregroundRectangle.Width;
int height = foregroundRectangle.Height;
if (width <= 0 || height <= 0)
{
throw new ImageProcessingException(
"Cannot draw image because the source image does not overlap the target image.");
// Nothing to do, return.
return;
}

DrawImageProcessor<TPixelBg, TPixelFg>.RowOperation operation = new(source.PixelBuffer, targetImage.Frames.RootFrame.PixelBuffer, blender, configuration, minX, width, locationY, targetX, this.Opacity);
// Sanitize the dimensions so that we don't try and sample outside the image.
foregroundRectangle = Rectangle.Intersect(foregroundRectangle, this.ForegroundImage.Bounds);
Rectangle backgroundRectangle = Rectangle.Intersect(new(left, top, width, height), this.SourceRectangle);
Configuration configuration = this.Configuration;

DrawImageProcessor<TPixelBg, TPixelFg>.RowOperation operation =
new(
configuration,
source.PixelBuffer,
this.ForegroundImage.Frames.RootFrame.PixelBuffer,
backgroundRectangle,
foregroundRectangle,
this.Blender,
this.Opacity);

ParallelRowIterator.IterateRows(
configuration,
workingRect,
new(0, 0, foregroundRectangle.Width, foregroundRectangle.Height),
in operation);
}

Expand All @@ -110,45 +130,39 @@ protected override void OnFrameApply(ImageFrame<TPixelBg> source)
/// </summary>
private readonly struct RowOperation : IRowOperation
{
private readonly Buffer2D<TPixelBg> source;
private readonly Buffer2D<TPixelFg> target;
private readonly Buffer2D<TPixelBg> background;
private readonly Buffer2D<TPixelFg> foreground;
private readonly PixelBlender<TPixelBg> blender;
private readonly Configuration configuration;
private readonly int minX;
private readonly int width;
private readonly int locationY;
private readonly int targetX;
private readonly Rectangle foregroundRectangle;
private readonly Rectangle backgroundRectangle;
private readonly float opacity;

[MethodImpl(InliningOptions.ShortMethod)]
public RowOperation(
Buffer2D<TPixelBg> source,
Buffer2D<TPixelFg> target,
PixelBlender<TPixelBg> blender,
Configuration configuration,
int minX,
int width,
int locationY,
int targetX,
Buffer2D<TPixelBg> background,
Buffer2D<TPixelFg> foreground,
Rectangle backgroundRectangle,
Rectangle foregroundRectangle,
PixelBlender<TPixelBg> blender,
float opacity)
{
this.source = source;
this.target = target;
this.blender = blender;
this.configuration = configuration;
this.minX = minX;
this.width = width;
this.locationY = locationY;
this.targetX = targetX;
this.background = background;
this.foreground = foreground;
this.backgroundRectangle = backgroundRectangle;
this.foregroundRectangle = foregroundRectangle;
this.blender = blender;
this.opacity = opacity;
}

/// <inheritdoc/>
[MethodImpl(InliningOptions.ShortMethod)]
public void Invoke(int y)
{
Span<TPixelBg> background = this.source.DangerousGetRowSpan(y).Slice(this.minX, this.width);
Span<TPixelFg> foreground = this.target.DangerousGetRowSpan(y - this.locationY).Slice(this.targetX, this.width);
Span<TPixelBg> background = this.background.DangerousGetRowSpan(y + this.backgroundRectangle.Top).Slice(this.backgroundRectangle.Left, this.backgroundRectangle.Width);
Span<TPixelFg> foreground = this.foreground.DangerousGetRowSpan(y + this.foregroundRectangle.Top).Slice(this.foregroundRectangle.Left, this.foregroundRectangle.Width);
this.blender.Blend<TPixelFg>(this.configuration, background, background, foreground, this.opacity);
}
}
Expand Down
67 changes: 57 additions & 10 deletions tests/ImageSharp.Tests/Drawing/DrawImageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,18 +190,65 @@ public void DrawTransformed<TPixel>(TestImageProvider<TPixel> provider)
}

[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)
[WithFile(TestImages.Png.Issue2447, PixelTypes.Rgba32)]
public void Issue2447_A<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<Rgba32> background = provider.GetImage();
using Image<Rgba32> overlay = new(Configuration.Default, 10, 10, Color.Black);
ImageProcessingException ex = Assert.Throws<ImageProcessingException>(Test);
using Image<TPixel> foreground = provider.GetImage();
using Image<Rgba32> background = new(100, 100, new Rgba32(0, 255, 255));

Assert.Contains("does not overlap", ex.ToString());
background.Mutate(c => c.DrawImage(foreground, new Point(64, 10), new Rectangle(32, 32, 32, 32), 1F));

void Test() => background.Mutate(context => context.DrawImage(overlay, new Point(x, y), new GraphicsOptions()));
background.DebugSave(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);

background.CompareToReferenceOutput(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);
}

[Theory]
[WithFile(TestImages.Png.Issue2447, PixelTypes.Rgba32)]
public void Issue2447_B<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> foreground = provider.GetImage();
using Image<Rgba32> background = new(100, 100, new Rgba32(0, 255, 255));

background.Mutate(c => c.DrawImage(foreground, new Point(10, 10), new Rectangle(320, 128, 32, 32), 1F));

background.DebugSave(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);

background.CompareToReferenceOutput(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);
}

[Theory]
[WithFile(TestImages.Png.Issue2447, PixelTypes.Rgba32)]
public void Issue2447_C<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> foreground = provider.GetImage();
using Image<Rgba32> background = new(100, 100, new Rgba32(0, 255, 255));

background.Mutate(c => c.DrawImage(foreground, new Point(10, 10), new Rectangle(32, 32, 32, 32), 1F));

background.DebugSave(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);

background.CompareToReferenceOutput(
provider,
appendPixelTypeToFileName: false,
appendSourceFileOrDescription: false);
}
}
3 changes: 3 additions & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ public static class Png
// Issue 2259: https://github.com/SixLabors/ImageSharp/issues/2259
public const string Issue2259 = "Png/issues/Issue_2259.png";

// Issue 2447: https://github.com/SixLabors/ImageSharp/issues/2447
public const string Issue2447 = "Png/issues/Issue_2447.png";

public static class Bad
{
public const string MissingDataChunk = "Png/xdtn0g01.png";
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions tests/Images/Input/Png/issues/issue_2447.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.