Merge pull request #420 from SixLabors/js/drawing-cache

Make DrawingTextCache thread-safe

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.1.0" />
-    <PackageReference Include="SixLabors.ImageSharp" Version="4.1.0" />
+    <PackageReference Include="SixLabors.Fonts" Version="3.1.2" />
+    <PackageReference Include="SixLabors.ImageSharp" Version="4.1.2" />
     <PackageReference Include="SixLabors.PolygonClipper" Version="1.0.1" />
   </ItemGroup>
 

src/ImageSharp.Drawing/Processing/DrawingCanvas{TPixel}.cs

@@ -72,26 +72,6 @@ public sealed class DrawingCanvas<TPixel> : DrawingCanvas
     /// </summary>
     private readonly DrawingTextCache textCache;
 
-    /// <summary>
-    /// Reusable operation list handed to each text renderer. Hosted by the text cache because
-    /// canvases are per-frame objects; sharing the cache-owned list keeps its capacity across
-    /// frames instead of regrowing a fresh list of large operation structs per draw.
-    /// </summary>
-    private readonly List<DrawingOperation> textOperations;
-
-    /// <summary>
-    /// Reusable sort buffer for <see cref="DrawTextOperations"/>, hosted by the text cache for
-    /// the same reason as <see cref="textOperations"/>. Only pass and index pairs are sorted;
-    /// the operations themselves stay in place so the per-draw sort moves eight bytes per
-    /// entry instead of the full operation struct.
-    /// </summary>
-    private readonly List<(byte RenderPass, int Sequence)> textOperationSortBuffer;
-
-    /// <summary>
-    /// Reusable stack pairing the begin and end commands for nested text composite layers.
-    /// </summary>
-    private readonly List<DrawingCanvasLayer> textCompositeLayerStack;
-
     /// <summary>
     /// Initializes a new instance of the <see cref="DrawingCanvas{TPixel}"/> class.
     /// </summary>
@@ -303,9 +283,6 @@ private DrawingCanvas(
         this.targetFrame = targetFrame;
         this.batcher = batcher;
         this.textCache = textCache;
-        this.textOperations = textCache.OperationScratch;
-        this.textOperationSortBuffer = textCache.OperationSortScratch;
-        this.textCompositeLayerStack = textCache.CompositeLayerScratch;
         this.ownsBatcher = ownsBatcher;
         this.ownsTextCache = ownsTextCache;
         this.pendingImageResources = pendingImageResources;
@@ -869,11 +846,11 @@ configuredOptions.VisibleBounds is null &&
             };
         }
 
-        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, configuredPath, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, configuredPath, pen, brush, this.textCache);
         TextRenderer renderer = new(glyphRenderer);
         renderer.Render(text, configuredOptions);
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, effectiveOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, effectiveOptions);
     }
 
     /// <inheritdoc />
@@ -900,7 +877,7 @@ public override void DrawText(
             Matrix4x4.CreateTranslation(location.X, location.Y, 0) * effectiveOptions.Transform,
             effectiveOptions.TextContrast);
 
-        using RichTextGlyphRenderer glyphRenderer = new(placedOptions, path: null, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(placedOptions, path: null, pen, brush, this.textCache);
         if (TryGetVisibleTextBounds(state, placedOptions.Transform, out FontRectangle visibleBounds))
         {
             textBlock.RenderTo(glyphRenderer, wrappingLength, visibleBounds);
@@ -910,7 +887,7 @@ public override void DrawText(
             textBlock.RenderTo(glyphRenderer, wrappingLength);
         }
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, placedOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, placedOptions);
     }
 
     /// <inheritdoc />
@@ -929,10 +906,10 @@ public override void DrawText(
         DrawingCanvasState state = this.ResolveState();
         DrawingOptions effectiveOptions = state.Options;
 
-        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path, pen, brush, this.textCache);
         textBlock.RenderTo(glyphRenderer, wrappingLength);
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, effectiveOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, effectiveOptions);
     }
 
     /// <inheritdoc />
@@ -958,10 +935,10 @@ public override void DrawText(
             Matrix4x4.CreateTranslation(location.X, location.Y, 0) * effectiveOptions.Transform,
             effectiveOptions.TextContrast);
 
-        using RichTextGlyphRenderer glyphRenderer = new(placedOptions, path: null, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(placedOptions, path: null, pen, brush, this.textCache);
         lineLayout.RenderTo(glyphRenderer);
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, placedOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, placedOptions);
     }
 
     /// <inheritdoc />
@@ -979,10 +956,10 @@ public override void DrawText(
         DrawingCanvasState state = this.ResolveState();
         DrawingOptions effectiveOptions = state.Options;
 
-        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path, pen, brush, this.textCache);
         lineLayout.RenderTo(glyphRenderer);
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, effectiveOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, effectiveOptions);
     }
 
     /// <inheritdoc />
@@ -999,11 +976,11 @@ public override void DrawText(
         DrawingCanvasState state = this.ResolveState();
         DrawingOptions effectiveOptions = state.Options;
 
-        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path: null, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path: null, pen, brush, this.textCache);
         TextRenderer renderer = new(glyphRenderer);
         renderer.Render(glyphId, options);
 
-        this.DrawTextOperations(glyphRenderer.DrawingOperations, effectiveOptions);
+        this.DrawTextOperations(glyphRenderer.DrawingOperations, glyphRenderer.Scratch, effectiveOptions);
     }
 
     /// <inheritdoc />
@@ -1023,11 +1000,11 @@ public override void DrawText(ReadOnlySpan<ushort> glyphIds, ReadOnlySpan<Vector
         DrawingCanvasState state = this.ResolveState();
         DrawingOptions effectiveOptions = state.Options;
 
-        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path: null, pen, brush, this.textCache, this.textOperations);
+        using RichTextGlyphRenderer glyphRenderer = new(effectiveOptions, path: null, pen, brush, this.textCache);
         TextRenderer renderer = new(glyphRenderer);
         renderer.Render(glyphIds, points, options);
 
-        this.DrawTextOperations(this.BatchGlyphRunOperations(glyphRenderer.DrawingOperations), effectiveOptions);
+        this.DrawTextOperations(this.BatchGlyphRunOperations(glyphRenderer.DrawingOperations), glyphRenderer.Scratch, effectiveOptions);
     }
 
     /// <inheritdoc />
@@ -1801,17 +1778,20 @@ private static IPath GetPositionedGlyphPath(DrawingTextCache.RunPathCacheEntry e
     /// Converts rendered text operations to composition commands and submits them to the batcher.
     /// </summary>
     /// <param name="operations">Text drawing operations produced by glyph layout/rendering.</param>
+    /// <param name="scratch">The working buffers leased by this draw's renderer.</param>
     /// <param name="drawingOptions">Drawing options applied to each operation.</param>
-    private void DrawTextOperations(List<DrawingOperation> operations, DrawingOptions drawingOptions)
+    private void DrawTextOperations(
+        List<DrawingOperation> operations,
+        DrawingTextCache.DrawingScratch scratch,
+        DrawingOptions drawingOptions)
     {
         // Enforce render-pass ordering while preserving original emission order inside each
         // pass. This preserves overlapping color-font layer compositing semantics (for
         // example emoji mouth/teeth layers) and keeps the composite group markers paired
         // with the fill operations they contain.
-        // The cache-owned buffer keeps its capacity across draws; draw calls never overlap on
-        // one canvas.
-        List<(byte RenderPass, int Sequence)> entries = this.textOperationSortBuffer;
-        entries.Clear();
+        // The renderer leases these buffers until command submission completes. Other canvases
+        // sharing the cache rent different buffers, while sequential draws retain capacity.
+        List<(byte RenderPass, int Sequence)> entries = scratch.SortBuffer;
 
         // Queued glyph commands never carry the canvas transform: glyph geometry arrives with
         // it already applied, and the sub-pixel remainder rides the command itself. One shared
@@ -1832,8 +1812,7 @@ private void DrawTextOperations(List<DrawingOperation> operations, DrawingOption
             return cmp != 0 ? cmp : a.Sequence.CompareTo(b.Sequence);
         });
 
-        List<DrawingCanvasLayer> compositeLayers = this.textCompositeLayerStack;
-        compositeLayers.Clear();
+        List<DrawingCanvasLayer> compositeLayers = scratch.CompositeLayers;
         DrawingCanvasState state = this.ResolveState();
 
         for (int i = 0; i < entries.Count; i++)
@@ -1879,11 +1858,6 @@ private void DrawTextOperations(List<DrawingOperation> operations, DrawingOption
                 this.batcher.AddStrokePath(((StrokePathCompositionSceneCommand)command).Command);
             }
         }
-
-        // The buffers outlive the canvas (the text cache hosts them), so drop the layer
-        // references now rather than rooting the final draw's state until the next draw.
-        entries.Clear();
-        compositeLayers.Clear();
     }
 
     /// <summary>