Merge pull request #413 from SixLabors/js/font-palette

Fix rich text cloning and palette cache key

Author: JimBobSquarePants

src/ImageSharp.Drawing/ImageSharp.Drawing.csproj

@@ -44,8 +44,8 @@
     <InternalsVisibleTo Include="SixLabors.ImageSharp.Drawing.WebGPU" Key="$(SixLaborsPublicKey)" />
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="SixLabors.Fonts" Version="3.0.1-alpha.0.17" />
-    <PackageReference Include="SixLabors.ImageSharp" Version="4.0.1-alpha.0.23" />
+    <PackageReference Include="SixLabors.Fonts" Version="3.0.1-alpha.0.18" />
+    <PackageReference Include="SixLabors.ImageSharp" Version="4.0.1-alpha.0.24" />
     <PackageReference Include="SixLabors.PolygonClipper" Version="1.0.1-alpha.0.6" />
   </ItemGroup>
 

src/ImageSharp.Drawing/Processing/RichTextGlyphRenderer.cs

@@ -1218,6 +1218,14 @@ internal struct GlyphRenderData
         /// </summary>
         public Pen? PenReference { get; init; }
 
+        /// <summary>
+        /// Gets the color palette selection the glyph's colors were resolved with, or
+        /// <see langword="null"/> when the glyph resolves no palette colors. The selection
+        /// changes the cached layer paints, so palette variants of one glyph must occupy
+        /// separate cache entries.
+        /// </summary>
+        public FontPalette? FontPalette { get; init; }
+
         /// <summary>
         /// Determines whether two <see cref="CacheKey"/> instances are equal.
         /// </summary>
@@ -1269,7 +1277,8 @@ public static CacheKey FromParameters(
                 TextAttributes = parameters.TextRun.TextAttributes,
                 TextDecorations = parameters.TextRun.TextDecorations,
                 Size = size,
-                PenReference = penReference
+                PenReference = penReference,
+                FontPalette = parameters.FontPalette
             };
 
         /// <inheritdoc/>
@@ -1291,7 +1300,8 @@ public bool Equals(CacheKey other)
             this.TextAttributes == other.TextAttributes &&
             this.TextDecorations == other.TextDecorations &&
             this.Size.Equals(other.Size) &&
-            ReferenceEquals(this.PenReference, other.PenReference);
+            ReferenceEquals(this.PenReference, other.PenReference) &&
+            Equals(this.FontPalette, other.FontPalette);
 
         /// <inheritdoc/>
         public override int GetHashCode()
@@ -1312,6 +1322,7 @@ public override int GetHashCode()
             hash.Add(this.TextDecorations);
             hash.Add(this.Size);
             hash.Add(this.PenReference is null ? 0 : RuntimeHelpers.GetHashCode(this.PenReference));
+            hash.Add(this.FontPalette);
             return hash.ToHashCode();
         }
     }

src/ImageSharp.Drawing/Processing/RichTextOptions.cs

@@ -2,6 +2,7 @@
 // Licensed under the Six Labors Split License.
 
 using SixLabors.Fonts;
+using SixLabors.Fonts.Tables.AdvancedTypographic;
 
 namespace SixLabors.ImageSharp.Drawing.Processing;
 
@@ -27,12 +28,17 @@ public RichTextOptions(RichTextOptions options)
         : base(options)
     {
         // Copy each run into a fresh instance so later mutation of the source runs
-        // cannot leak into this options instance (and vice versa).
+        // cannot leak into this options instance (and vice versa). Every property of
+        // RichTextRun and its TextRun base must appear here: a missing property
+        // silently resets to its default on the clone DrawText renders from.
         List<RichTextRun> runs = new(options.TextRuns.Count);
         foreach (RichTextRun run in options.TextRuns)
         {
             runs.Add(new RichTextRun()
             {
+                // Brushes, pens, fonts, and palettes copy by reference: each is immutable
+                // once constructed (FontPalette snapshots its overrides in its own
+                // constructor), so a shared reference cannot leak later mutation.
                 Brush = run.Brush,
                 Pen = run.Pen,
                 StrikeoutPen = run.StrikeoutPen,
@@ -41,8 +47,17 @@ public RichTextOptions(RichTextOptions options)
                 Start = run.Start,
                 End = run.End,
                 Font = run.Font,
+                FontWeight = run.FontWeight,
+                Script = run.Script,
+                Culture = run.Culture,
+
+                // The feature tag list is the one caller-owned mutable collection on a
+                // run; the read-only interface is only a view, so isolation needs a copy.
+                FeatureTags = run.FeatureTags is null ? null : new List<Tag>(run.FeatureTags),
                 TextAttributes = run.TextAttributes,
                 TextDecorations = run.TextDecorations,
+                ColorFontSupport = run.ColorFontSupport,
+                FontPalette = run.FontPalette,
                 Placeholder = run.Placeholder
             });
         }

tests/ImageSharp.Drawing.Tests/Processing/DrawingCanvasTests.Text.cs

@@ -107,6 +107,7 @@ public void DrawGlyphById_Inter_OverlappingContours_NoHoles<TPixel>(TestImagePro
                         TextDecorations.None,
                         LayoutMode.HorizontalTopBottom,
                         ColorFontSupport.None,
+                        null,
                         out FontGlyphMetrics metrics))
                 {
                     continue;
@@ -160,6 +161,7 @@ public void DrawGlyphById_Inter_EvenOddCanvasState_NoHoles<TPixel>(TestImageProv
                         TextDecorations.None,
                         LayoutMode.HorizontalTopBottom,
                         ColorFontSupport.None,
+                        null,
                         out FontGlyphMetrics metrics))
                 {
                     continue;
@@ -306,6 +308,7 @@ public void DrawPositionedGlyphs_Inter_MatchesGlyphByIdLoop<TPixel>(TestImagePro
                         TextDecorations.None,
                         LayoutMode.HorizontalTopBottom,
                         ColorFontSupport.None,
+                        null,
                         out FontGlyphMetrics metrics))
                 {
                     continue;
@@ -820,6 +823,7 @@ private static void DrawGlyphs<TPixel>(DrawingCanvas<TPixel> canvas, string text
                     TextDecorations.None,
                     LayoutMode.HorizontalTopBottom,
                     ColorFontSupport.None,
+                    null,
                     out FontGlyphMetrics metrics))
             {
                 continue;