Correctly handle vertical glyph decorations

Author: JimBobSquarePants

src/ImageSharp.Drawing/ImageSharp.Drawing.csproj

@@ -18,7 +18,7 @@
     <None Include="..\..\shared-infrastructure\branding\icons\imagesharp.drawing\sixlabors.imagesharp.drawing.128.png" Pack="true" PackagePath="" />
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="SixLabors.Fonts" Version="1.0.0-beta19.10" />
+    <PackageReference Include="SixLabors.Fonts" Version="1.0.0-beta19.12" />
     <PackageReference Include="SixLabors.ImageSharp" Version="2.1.3" />
   </ItemGroup>
   <Import Project="..\..\shared-infrastructure\src\SharedInfrastructure\SharedInfrastructure.projitems" Label="Shared" />

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

@@ -24,7 +24,6 @@ internal sealed class RichTextGlyphRenderer : BaseGlyphBuilder, IColorGlyphRende
         private const byte RenderOrderOutline = 1;
         private const byte RenderOrderDecoration = 2;
 
-        private readonly RichTextOptions textOptions;
         private readonly DrawingOptions drawingOptions;
         private readonly MemoryAllocator memoryAllocator;
         private readonly Pen defaultPen;
@@ -40,7 +39,7 @@ internal sealed class RichTextGlyphRenderer : BaseGlyphBuilder, IColorGlyphRende
         private TextDecorationDetails? currentUnderline;
         private TextDecorationDetails? currentStrikout;
         private TextDecorationDetails? currentOverline;
-        private bool currentDecorationsRotated;
+        private bool currentDecorationRotated;
 
         // Just enough accuracy to allow for 1/8 px differences which later are accumulated while rendering,
         // but do not grow into full px offsets.
@@ -61,7 +60,6 @@ public RichTextGlyphRenderer(
             Brush brush)
             : base(drawingOptions.Transform)
         {
-            this.textOptions = textOptions;
             this.drawingOptions = drawingOptions;
             this.memoryAllocator = memoryAllocator;
             this.defaultPen = pen;
@@ -102,7 +100,7 @@ protected override void BeginText(in FontRectangle bounds)
         protected override void BeginGlyph(in FontRectangle bounds, in GlyphRendererParameters parameters)
         {
             this.currentColor = null;
-            this.currentDecorationsRotated = parameters.LayoutMode.IsVertical() || parameters.LayoutMode.IsVerticalMixed();
+            this.currentDecorationRotated = parameters.LayoutMode.IsVertical() || parameters.LayoutMode.IsVerticalMixed();
             this.currentTextRun = parameters.TextRun;
             if (parameters.TextRun is RichTextRun drawingRun)
             {
@@ -225,19 +223,14 @@ public override void SetDecoration(TextDecorations textDecorations, Vector2 star
             {
                 thickness = pen.StrokeWidth;
             }
-
-            // Center the line at the given position.
-            bool rotated = this.currentDecorationsRotated;
-            Vector2 pad = rotated ? new(thickness * .5F, 0) : new(0, thickness * .5F);
-            Vector2 a = start - pad;
-            Vector2 b = start + pad;
-            Vector2 d = end - pad;
-            thickness = rotated ? b.X - a.X : b.Y - a.Y;
-
-            pen ??= new SolidPen(this.currentBrush ?? this.defaultBrush, thickness);
+            else
+            {
+                pen = new SolidPen(this.currentBrush ?? this.defaultBrush, thickness);
+            }
 
             // Drawing is always centered around the point so we need to offset by half.
             Vector2 offset = Vector2.Zero;
+            bool rotated = this.currentDecorationRotated;
             if (textDecorations == TextDecorations.Overline)
             {
                 // CSS overline is drawn above the position, so we need to move it up.
@@ -249,8 +242,7 @@ public override void SetDecoration(TextDecorations textDecorations, Vector2 star
                 offset = rotated ? new(-(thickness * .5F), 0) : new(0, thickness * .5F);
             }
 
-            // Clamp the line to whole pixels
-            this.AppendDecoration(ref targetDecoration, ClampToPixel(a + offset), ClampToPixel(d + offset), pen, (float)Math.Truncate(thickness));
+            this.AppendDecoration(ref targetDecoration, start + offset, end + offset, pen, thickness, rotated);
         }
 
         protected override void EndGlyph()
@@ -425,24 +417,47 @@ private void FinalizeDecoration(ref TextDecorationDetails? decoration)
             }
         }
 
-        private void AppendDecoration(ref TextDecorationDetails? decoration, Vector2 start, Vector2 end, Pen pen, float thickness)
+        private void AppendDecoration(
+            ref TextDecorationDetails? decoration,
+            Vector2 start,
+            Vector2 end,
+            Pen pen,
+            float thickness,
+            bool rotated)
         {
             if (decoration != null)
             {
                 // TODO: This only works well if we are not trying to follow a path.
                 if (this.path is null)
                 {
                     // Let's try and expand it first.
-                    if (thickness == decoration.Value.Thickness
-                        && decoration.Value.End.Y == start.Y
-                        && (decoration.Value.End.X + 1) >= start.X
+                    if (rotated)
+                    {
+                        if (thickness == decoration.Value.Thickness
+                        && decoration.Value.End.Y + 1 >= start.Y
+                        && decoration.Value.End.X == start.X
                         && decoration.Value.Pen.Equals(pen))
+                        {
+                            // Expand the line
+                            start = decoration.Value.Start;
+
+                            // If this is null finalize does nothing.
+                            decoration = null;
+                        }
+                    }
+                    else
                     {
-                        // Expand the line
-                        start = decoration.Value.Start;
+                        if (thickness == decoration.Value.Thickness
+                        && decoration.Value.End.Y == start.Y
+                        && decoration.Value.End.X + 1 >= start.X
+                        && decoration.Value.Pen.Equals(pen))
+                        {
+                            // Expand the line
+                            start = decoration.Value.Start;
 
-                        // If this is null finalize does nothing.
-                        decoration = null;
+                            // If this is null finalize does nothing.
+                            decoration = null;
+                        }
                     }
                 }
             }

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

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Numerics;
 using System.Text;
@@ -785,6 +786,102 @@ public void PathAndTextDrawingMatch<TPixel>(TestImageProvider<TPixel> provider)
               });
         }
 
+        [Theory]
+        [WithBlankImage(500, 400, PixelTypes.Rgba32)]
+        public void CanFillTextVertical<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            Font font = CreateFont(TestFonts.OpenSans, 36);
+            Font fallback = CreateFont(TestFonts.NotoSansKRRegular, 36);
+
+            const string text = "한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo";
+            RichTextOptions textOptions = new(font)
+            {
+                Origin = new(0, 0),
+                FallbackFontFamilies = new[] { fallback.Family },
+                WrappingLength = 300,
+                LayoutMode = LayoutMode.VerticalLeftRight,
+                TextRuns = new[] { new RichTextRun() { Start = 0, End = text.GetGraphemeCount(), TextDecorations = TextDecorations.Underline | TextDecorations.Strikeout | TextDecorations.Overline } }
+            };
+
+            IPathCollection glyphs = TextBuilder.GenerateGlyphs(text, textOptions);
+
+            DrawingOptions options = new() { ShapeOptions = new() { IntersectionRule = IntersectionRule.Nonzero } };
+
+            provider.RunValidatingProcessorTest(
+                c => c.Fill(Color.White).Fill(options, Color.Black, glyphs),
+                comparer: ImageComparer.TolerantPercentage(0.002f));
+        }
+
+        [Theory]
+        [WithBlankImage(500, 400, PixelTypes.Rgba32)]
+        public void CanFillTextVerticalMixed<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            Font font = CreateFont(TestFonts.OpenSans, 36);
+            Font fallback = CreateFont(TestFonts.NotoSansKRRegular, 36);
+
+            const string text = "한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo";
+            RichTextOptions textOptions = new(font)
+            {
+                FallbackFontFamilies = new[] { fallback.Family },
+                WrappingLength = 400,
+                LayoutMode = LayoutMode.VerticalMixedLeftRight,
+                TextRuns = new[] { new RichTextRun() { Start = 0, End = text.GetGraphemeCount(), TextDecorations = TextDecorations.Underline | TextDecorations.Strikeout | TextDecorations.Overline } }
+            };
+
+            IPathCollection glyphs = TextBuilder.GenerateGlyphs(text, textOptions);
+            DrawingOptions options = new() { ShapeOptions = new() { IntersectionRule = IntersectionRule.Nonzero } };
+
+            provider.RunValidatingProcessorTest(
+                c => c.Fill(Color.White).Fill(options, Color.Black, glyphs),
+                comparer: ImageComparer.TolerantPercentage(0.002f));
+        }
+
+        [Theory]
+        [WithBlankImage(500, 400, PixelTypes.Rgba32)]
+        public void CanDrawTextVertical<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            Font font = CreateFont(TestFonts.OpenSans, 36);
+            Font fallback = CreateFont(TestFonts.NotoSansKRRegular, 36);
+
+            const string text = "한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo";
+            RichTextOptions textOptions = new(font)
+            {
+                FallbackFontFamilies = new[] { fallback.Family },
+                WrappingLength = 400,
+                LayoutMode = LayoutMode.VerticalLeftRight,
+                TextRuns = new[] { new RichTextRun() { Start = 0, End = text.GetGraphemeCount(), TextDecorations = TextDecorations.Underline | TextDecorations.Strikeout | TextDecorations.Overline } }
+            };
+
+            provider.RunValidatingProcessorTest(
+                c => c.Fill(Color.White).DrawText(textOptions, text, Brushes.Solid(Color.Black)),
+                comparer: ImageComparer.TolerantPercentage(0.002f));
+        }
+
+        [Theory]
+        [WithBlankImage(500, 400, PixelTypes.Rgba32)]
+        public void CanDrawTextVerticalMixed<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            Font font = CreateFont(TestFonts.OpenSans, 36);
+            Font fallback = CreateFont(TestFonts.NotoSansKRRegular, 36);
+
+            const string text = "한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo 한국어 hangugeo";
+            RichTextOptions textOptions = new(font)
+            {
+                FallbackFontFamilies = new[] { fallback.Family },
+                WrappingLength = 400,
+                LayoutMode = LayoutMode.VerticalMixedLeftRight,
+                TextRuns = new[] { new RichTextRun() { Start = 0, End = text.GetGraphemeCount(), TextDecorations = TextDecorations.Underline | TextDecorations.Strikeout | TextDecorations.Overline } }
+            };
+
+            provider.RunValidatingProcessorTest(
+                c => c.Fill(Color.White).DrawText(textOptions, text, Brushes.Solid(Color.Black)),
+                comparer: ImageComparer.TolerantPercentage(0.002f));
+        }
+
         private static string Repeat(string str, int times) => string.Concat(Enumerable.Repeat(str, times));
 
         private static string ToTestOutputDisplayText(string text)

tests/ImageSharp.Drawing.Tests/ImageSharp.Drawing.Tests.csproj

@@ -26,6 +26,9 @@
     <None Update="TestFonts\*.woff">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="TestFonts\*.otf">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="xunit.runner.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

tests/ImageSharp.Drawing.Tests/TestFonts.cs

@@ -22,5 +22,7 @@ public static class TestFonts
         public const string MeQuranVolyNewmet = "me_quran_volt_newmet.ttf";
 
         public const string NettoOffc = "NettoOffc.ttf";
+
+        public const string NotoSansKRRegular = "NotoSansKR-Regular.otf";
     }
 }