Merge pull request #755 from SixLabors/sw/wandering-text

Prevent text wandering from baseline

src/ImageSharp.Drawing/Processing/Processors/Text/DrawTextProcessor.cs

@@ -97,7 +97,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
 
             this.textRenderer = new CachingGlyphRenderer(source.GetMemoryAllocator(), this.Text.Length, this.Pen, this.Brush != null);
             this.textRenderer.Options = (GraphicsOptions)this.Options;
-            TextRenderer.RenderTextTo(this.textRenderer, this.Text, style);
+            var renderer = new TextRenderer(this.textRenderer);
+            renderer.RenderText(this.Text, style);
         }
 
         protected override void AfterImageApply(Image<TPixel> source, Rectangle sourceRectangle)
@@ -164,18 +165,26 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
 
         private class CachingGlyphRenderer : IGlyphRenderer, IDisposable
         {
-            private PathBuilder builder;
+            // just enough accuracy to allow for 1/8 pixel differences which
+            // later are accumulated while rendering, but do not grow into full pixel offsets
+            // The value 8 is benchmarked to:
+            // - Provide a good accuracy (smaller than 0.2% image difference compared to the non-caching variant)
+            // - Cache hit ratio above 60%
+            private const float AccuracyMultiple = 8;
+
+            private readonly PathBuilder builder;
 
             private Point currentRenderPosition = default;
-            private GlyphRendererParameters currentGlyphRenderParams = default;
-            private int offset = 0;
+            private (GlyphRendererParameters glyph, PointF subPixelOffset) currentGlyphRenderParams = default;
+            private readonly int offset = 0;
             private PointF currentPoint = default(PointF);
 
-            private readonly Dictionary<GlyphRendererParameters, GlyphRenderData> glyphData = new Dictionary<GlyphRendererParameters, GlyphRenderData>();
+            private readonly Dictionary<(GlyphRendererParameters glyph, PointF subPixelOffset), GlyphRenderData>
+                glyphData = new Dictionary<(GlyphRendererParameters glyph, PointF subPixelOffset), GlyphRenderData>();
 
-            private bool renderOutline = false;
-            private bool renderFill = false;
-            private bool raterizationRequired = false;
+            private readonly bool renderOutline = false;
+            private readonly bool renderFill = false;
+            private bool rasterizationRequired = false;
 
             public CachingGlyphRenderer(MemoryAllocator memoryAllocator, int size, IPen pen, bool renderFill)
             {
@@ -213,17 +222,22 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
                 this.builder.StartFigure();
             }
 
-            public bool BeginGlyph(RectangleF bounds, GlyphRendererParameters paramters)
+            public bool BeginGlyph(RectangleF bounds, GlyphRendererParameters parameters)
             {
                 this.currentRenderPosition = Point.Truncate(bounds.Location);
+                PointF subPixelOffset = bounds.Location - this.currentRenderPosition;
+
+                subPixelOffset.X = MathF.Round(subPixelOffset.X * AccuracyMultiple) / AccuracyMultiple;
+                subPixelOffset.Y = MathF.Round(subPixelOffset.Y * AccuracyMultiple) / AccuracyMultiple;
 
                 // we have offset our rendering origion a little bit down to prevent edge cropping, move the draw origin up to compensate
                 this.currentRenderPosition = new Point(this.currentRenderPosition.X - this.offset, this.currentRenderPosition.Y - this.offset);
-                this.currentGlyphRenderParams = paramters;
-                if (this.glyphData.ContainsKey(paramters))
+                this.currentGlyphRenderParams = (parameters, subPixelOffset);
+
+                if (this.glyphData.ContainsKey(this.currentGlyphRenderParams))
                 {
                     // we have already drawn the glyph vectors skip trying again
-                    this.raterizationRequired = false;
+                    this.rasterizationRequired = false;
                     return false;
                 }
 
@@ -233,7 +247,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
                 // ensure all glyphs render around [zero, zero]  so offset negative root positions so when we draw the glyph we can offet it back
                 this.builder.SetOrigin(new PointF(-(int)bounds.X + this.offset, -(int)bounds.Y + this.offset));
 
-                this.raterizationRequired = true;
+                this.rasterizationRequired = true;
                 return true;
             }
 
@@ -252,7 +266,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
 
             public void Dispose()
             {
-                foreach (KeyValuePair<GlyphRendererParameters, GlyphRenderData> kv in this.glyphData)
+                foreach (KeyValuePair<(GlyphRendererParameters glyph, PointF subPixelOffset), GlyphRenderData> kv in this.glyphData)
                 {
                     kv.Value.Dispose();
                 }
@@ -270,7 +284,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text
                 GlyphRenderData renderData = default;
 
                 // has the glyoh been rendedered already????
-                if (this.raterizationRequired)
+                if (this.rasterizationRequired)
                 {
                     IPath path = this.builder.Build();
 

tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs

@@ -11,6 +11,8 @@ using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
 using SixLabors.Primitives;
 
 using Xunit;
+using Xunit.Abstractions;
+
 // ReSharper disable InconsistentNaming
 
 namespace SixLabors.ImageSharp.Tests.Drawing.Text
@@ -22,8 +24,15 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text
 
         private const string TestText = "Sphinx of black quartz, judge my vow\n0123456789";
 
-        public static ImageComparer TextDrawingComparer = ImageComparer.TolerantPercentage(0.01f);
-        public static ImageComparer OutlinedTextDrawingComparer = ImageComparer.TolerantPercentage(0.5f, 3);
+        public static ImageComparer TextDrawingComparer = ImageComparer.TolerantPercentage(1e-5f);
+        public static ImageComparer OutlinedTextDrawingComparer = ImageComparer.TolerantPercentage(5e-4f);
+
+        public DrawTextOnImageTests(ITestOutputHelper output)
+        {
+            this.Output = output;
+        }
+
+        private ITestOutputHelper Output { get; }
 
         [Theory]
         [WithSolidFilledImages(276, 336, "White", PixelTypes.Rgba32)]
@@ -122,8 +131,12 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text
 
             TPixel color = NamedColors<TPixel>.Black;
 
+            // Based on the reported 0.0270% difference with AccuracyMultiple = 8
+            // We should avoid quality regressions leading to higher difference!
+            var comparer = ImageComparer.TolerantPercentage(0.03f);
+
             provider.VerifyOperation(
-                TextDrawingComparer,
+                comparer,
                 img =>
                     {
                         img.Mutate(c => c.DrawText(textOptions, sb.ToString(), font, color, new PointF(10, 5)));
@@ -186,6 +199,31 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text
                 appendSourceFileOrDescription: true);
         }
 
+        [Theory]
+        [WithSolidFilledImages(1000, 1500, "White", PixelTypes.Rgba32, "OpenSans-Regular.ttf")]
+        public void TextPositioningIsRobust<TPixel>(TestImageProvider<TPixel> provider, string fontName)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            Font font = CreateFont(fontName, 30);
+
+            string text = Repeat("Beware the Jabberwock, my son!  The jaws that bite, the claws that catch!  Beware the Jubjub bird, and shun The frumious Bandersnatch!\n",
+                20);
+            var textOptions = new TextGraphicsOptions(true) { WrapTextWidth = 1000 };
+
+            string details = fontName.Replace(" ", "");
+
+            // Based on the reported 0.1755% difference with AccuracyMultiple = 8
+            // We should avoid quality regressions leading to higher difference!
+            var comparer = ImageComparer.TolerantPercentage(0.2f);
+
+            provider.RunValidatingProcessorTest(
+                x => x.DrawText(textOptions, text, font, NamedColors<TPixel>.Black, new PointF(10, 50)),
+                details,
+                comparer,
+                appendPixelTypeToFileName: false,
+                appendSourceFileOrDescription: false);
+        }
+
         private static string Repeat(string str, int times) => string.Concat(Enumerable.Repeat(str, times));
 
         private static string ToTestOutputDisplayText(string text)
@@ -202,5 +240,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text
             Font font = fontCollection.Install(fontPath).CreateFont(size);
             return font;
         }
+
+        
     }
 }

tests/Images/External

@@ -1 +1 @@
-Subproject commit c6980db777e49d5e526b56cb986001d1a191acdf
+Subproject commit f41ae0327a3ab21ab2388c32160bda67debcc082