Merge pull request #1181 from AvaloniaUI/fixes/1178-rendering-artifacts

Fix rendering artifacts

src/Avalonia.Visuals/Rendering/SceneGraph/BrushDrawOperation.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using Avalonia.Media;
-using Avalonia.Platform;
 using Avalonia.VisualTree;
 
 namespace Avalonia.Rendering.SceneGraph
@@ -12,20 +11,16 @@ namespace Avalonia.Rendering.SceneGraph
     /// <summary>
     /// Base class for draw operations that can use a brush.
     /// </summary>
-    internal abstract class BrushDrawOperation : IDrawOperation
+    internal abstract class BrushDrawOperation : DrawOperation
     {
-        /// <inheritdoc/>
-        public abstract Rect Bounds { get; }
-
-        /// <inheritdoc/>
-        public abstract bool HitTest(Point p);
+        public BrushDrawOperation(Rect bounds, Matrix transform, Pen pen)
+            : base(bounds, transform, pen)
+        {
+        }
 
         /// <summary>
         /// Gets a collection of child scenes that are needed to draw visual brushes.
         /// </summary>
         public abstract IDictionary<IVisual, Scene> ChildScenes { get; }
-
-        /// <inheritdoc/>
-        public abstract void Render(IDrawingContextImpl context);
     }
 }

src/Avalonia.Visuals/Rendering/SceneGraph/DrawOperation.cs

@@ -0,0 +1,26 @@
+using System;
+using Avalonia.Media;
+using Avalonia.Platform;
+
+namespace Avalonia.Rendering.SceneGraph
+{
+    /// <summary>
+    /// Base class for draw operations that have bounds.
+    /// </summary>
+    internal abstract class DrawOperation : IDrawOperation
+    {
+        public DrawOperation(Rect bounds, Matrix transform, Pen pen)
+        {
+            bounds = bounds.Inflate((pen?.Thickness ?? 0) / 2).TransformToAABB(transform);
+            Bounds = new Rect(
+                new Point(Math.Floor(bounds.X), Math.Floor(bounds.Y)),
+                new Point(Math.Ceiling(bounds.Right), Math.Ceiling(bounds.Bottom)));
+        }
+
+        public Rect Bounds { get; }
+
+        public abstract bool HitTest(Point p);
+
+        public abstract void Render(IDrawingContextImpl context);
+    }
+}

src/Avalonia.Visuals/Rendering/SceneGraph/GeometryNode.cs

@@ -28,8 +28,8 @@ namespace Avalonia.Rendering.SceneGraph
             Pen pen,
             IGeometryImpl geometry,
             IDictionary<IVisual, Scene> childScenes = null)
+            : base(geometry.GetRenderBounds(pen?.Thickness ?? 0), transform, null)
         {
-            Bounds = geometry.GetRenderBounds(pen?.Thickness ?? 0).TransformToAABB(transform);
             Transform = transform;
             Brush = brush?.ToImmutable();
             Pen = pen?.ToImmutable();
@@ -37,9 +37,6 @@ namespace Avalonia.Rendering.SceneGraph
             ChildScenes = childScenes;
         }
 
-        /// <inheritdoc/>
-        public override Rect Bounds { get; }
-
         /// <summary>
         /// Gets the transform with which the node will be drawn.
         /// </summary>

src/Avalonia.Visuals/Rendering/SceneGraph/IDrawOperation.cs

@@ -12,7 +12,7 @@ namespace Avalonia.Rendering.SceneGraph
     public interface IDrawOperation
     {
         /// <summary>
-        /// Gets the bounds of the visible content in the node.
+        /// Gets the bounds of the visible content in the node in global coordinates.
         /// </summary>
         Rect Bounds { get; }
 

src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs

@@ -9,7 +9,7 @@ namespace Avalonia.Rendering.SceneGraph
     /// <summary>
     /// A node in the scene graph which represents an image draw.
     /// </summary>
-    internal class ImageNode : IDrawOperation
+    internal class ImageNode : DrawOperation
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="ImageNode"/> class.
@@ -20,8 +20,8 @@ namespace Avalonia.Rendering.SceneGraph
         /// <param name="sourceRect">The source rect.</param>
         /// <param name="destRect">The destination rect.</param>
         public ImageNode(Matrix transform, IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect)
+            : base(destRect, transform, null)
         {
-            Bounds = destRect.TransformToAABB(transform);
             Transform = transform;
             Source = source;
             Opacity = opacity;
@@ -29,9 +29,6 @@ namespace Avalonia.Rendering.SceneGraph
             DestRect = destRect;
         }
 
-        /// <inheritdoc/>
-        public Rect Bounds { get; }
-
         /// <summary>
         /// Gets the transform with which the node will be drawn.
         /// </summary>
@@ -80,7 +77,7 @@ namespace Avalonia.Rendering.SceneGraph
         }
 
         /// <inheritdoc/>
-        public void Render(IDrawingContextImpl context)
+        public override void Render(IDrawingContextImpl context)
         {
             // TODO: Probably need to introduce some kind of locking mechanism in the case of
             // WriteableBitmap.
@@ -89,6 +86,6 @@ namespace Avalonia.Rendering.SceneGraph
         }
 
         /// <inheritdoc/>
-        public bool HitTest(Point p) => Bounds.Contains(p);
+        public override bool HitTest(Point p) => Bounds.Contains(p);
     }
 }

src/Avalonia.Visuals/Rendering/SceneGraph/LineNode.cs

@@ -28,8 +28,8 @@ namespace Avalonia.Rendering.SceneGraph
             Point p1,
             Point p2,
             IDictionary<IVisual, Scene> childScenes = null)
+            : base(new Rect(p1, p2), transform, pen)
         {
-            Bounds = new Rect(p1, p2).TransformToAABB(transform).Inflate(pen?.Thickness ?? 0);
             Transform = transform;
             Pen = pen?.ToImmutable();
             P1 = p1;
@@ -37,9 +37,6 @@ namespace Avalonia.Rendering.SceneGraph
             ChildScenes = childScenes;
         }
 
-        /// <inheritdoc/>
-        public override Rect Bounds { get; }
-
         /// <summary>
         /// Gets the transform with which the node will be drawn.
         /// </summary>

src/Avalonia.Visuals/Rendering/SceneGraph/OpacityMaskNode.cs

@@ -19,6 +19,7 @@ namespace Avalonia.Rendering.SceneGraph
         /// <param name="bounds">The bounds of the mask.</param>
         /// <param name="childScenes">Child scenes for drawing visual brushes.</param>
         public OpacityMaskNode(IBrush mask, Rect bounds, IDictionary<IVisual, Scene> childScenes = null)
+            : base(Rect.Empty, Matrix.Identity, null)
         {
             Mask = mask?.ToImmutable();
             MaskBounds = bounds;
@@ -30,12 +31,10 @@ namespace Avalonia.Rendering.SceneGraph
         /// opacity mask pop.
         /// </summary>
         public OpacityMaskNode()
+            : base(Rect.Empty, Matrix.Identity, null)
         {
         }
 
-        /// <inheritdoc/>
-        public override Rect Bounds => Rect.Empty;
-
         /// <summary>
         /// Gets the mask to be pushed or null if the operation represents a pop.
         /// </summary>

src/Avalonia.Visuals/Rendering/SceneGraph/RectangleNode.cs

@@ -30,8 +30,8 @@ namespace Avalonia.Rendering.SceneGraph
             Rect rect,
             float cornerRadius,
             IDictionary<IVisual, Scene> childScenes = null)
+            : base(rect, transform, pen)
         {
-            Bounds = rect.TransformToAABB(transform).Inflate(pen?.Thickness ?? 0);
             Transform = transform;
             Brush = brush?.ToImmutable();
             Pen = pen?.ToImmutable();
@@ -40,9 +40,6 @@ namespace Avalonia.Rendering.SceneGraph
             ChildScenes = childScenes;
         }
 
-        /// <inheritdoc/>
-        public override Rect Bounds { get; }
-
         /// <summary>
         /// Gets the transform with which the node will be drawn.
         /// </summary>

src/Avalonia.Visuals/Rendering/SceneGraph/TextNode.cs

@@ -28,8 +28,8 @@ namespace Avalonia.Rendering.SceneGraph
             Point origin,
             IFormattedTextImpl text,
             IDictionary<IVisual, Scene> childScenes = null)
+            : base(new Rect(origin, text.Size), transform, null)
         {
-            Bounds = new Rect(origin, text.Size).TransformToAABB(transform);
             Transform = transform;
             Foreground = foreground?.ToImmutable();
             Origin = origin;
@@ -37,9 +37,6 @@ namespace Avalonia.Rendering.SceneGraph
             ChildScenes = childScenes;
         }
 
-        /// <inheritdoc/>
-        public override Rect Bounds { get; }
-
         /// <summary>
         /// Gets the transform with which the node will be drawn.
         /// </summary>

tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DrawOperationTests.cs

@@ -0,0 +1,55 @@
+using System;
+using Avalonia.Media;
+using Avalonia.Platform;
+using Avalonia.Rendering.SceneGraph;
+using Xunit;
+
+namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph
+{
+    public class DrawOperationTests
+    {
+        [Fact]
+        public void Empty_Bounds_Remain_Empty()
+        {
+            var target = new TestDrawOperation(Rect.Empty, Matrix.Identity, null);
+
+            Assert.Equal(Rect.Empty, target.Bounds);
+        }
+
+        [Theory]
+        [InlineData(10, 10, 10, 10, 1, 1, 1, 9, 9, 12, 12)]
+        [InlineData(10, 10, 10, 10, 1, 1, 2, 9, 9, 12, 12)]
+        [InlineData(10, 10, 10, 10, 1.5, 1.5, 1, 14, 14, 17, 17)]
+        public void Rectangle_Bounds_Are_Snapped_To_Pixels(
+            double x,
+            double y,
+            double width,
+            double height,
+            double scaleX,
+            double scaleY,
+            double? penThickness,
+            double expectedX,
+            double expectedY,
+            double expectedWidth,
+            double expectedHeight)
+        {
+            var target = new TestDrawOperation(
+                new Rect(x, y, width, height),
+                Matrix.CreateScale(scaleX, scaleY),
+                penThickness.HasValue ? new Pen(Brushes.Black, penThickness.Value) : null);
+            Assert.Equal(new Rect(expectedX, expectedY, expectedWidth, expectedHeight), target.Bounds);
+        }
+
+        private class TestDrawOperation : DrawOperation
+        {
+            public TestDrawOperation(Rect bounds, Matrix transform, Pen pen)
+                :base(bounds, transform, pen)
+            {
+            }
+
+            public override bool HitTest(Point p) => false;
+
+            public override void Render(IDrawingContextImpl context) { }
+        }
+    }
+}