Merge pull request #417 from SixLabors/js/round-join-arc-guard

Round join tessellation and CPU edge accuracy

Author: JimBobSquarePants

Directory.Build.props

@@ -18,4 +18,13 @@
   <!-- Import the shared global .props file -->
   <Import Project="$(MSBuildThisFileDirectory)shared-infrastructure\msbuild\props\SixLabors.Global.props" />
 
+  <!--
+    Suppress the advisory against Microsoft.Build.Tasks.Git 8.0.0, pulled in by the SourceLink
+    package pinned in shared infrastructure, while we wait for the ability to update it.
+    https://github.com/advisories/GHSA-23fw-v26w-5fgq
+  -->
+  <ItemGroup>
+    <NuGetAuditSuppress Include="https://github.com/advisories/GHSA-23fw-v26w-5fgq" />
+  </ItemGroup>
+
 </Project>

src/ImageSharp.Drawing.WebGPU/Shaders/WgslSource/path_lowering.wgsl

@@ -84,8 +84,8 @@ fn stroke_normalize_positive_angle(angle: f32) -> f32 {
     return a;
 }
 
-// Port of the CPU stroker's GetArcSubdivisionCount (round cap tessellation): returns the number
-// of interior vertices needed to keep the arc's chordal error within the arc detail scale.
+// Port of the CPU stroker's GetArcSubdivisionCount (round join and cap tessellation): returns the
+// number of interior vertices needed to keep the arc's chordal error within the arc detail scale.
 fn stroke_arc_subdivision_count(radius: f32, sweep: f32, arc_detail_scale: f32) -> u32 {
     let safe_radius = max(radius, TANGENT_THRESH);
     let safe_scale = max(arc_detail_scale, 0.01);
@@ -129,8 +129,8 @@ fn stroke_chain_arc(
 }
 
 // Port of PolygonStroker.CalcArc (round joins): sweeps from offset o1 to offset o2 around corner
-// v1 with a fixed angular step derived from the arc detail scale. Both endpoints are appended,
-// so the caller's chain need not already sit on the arc.
+// v1 with the interior vertex count from stroke_arc_subdivision_count, shared with round caps.
+// Both endpoints are appended, so the caller's chain need not already sit on the arc.
 fn stroke_calc_arc(
     path_ix: u32, last: ptr<function, vec2f>,
     v1: vec2f, o1: vec2f, o2: vec2f,
@@ -141,15 +141,13 @@ fn stroke_calc_arc(
     // must rotate o1; absolute angles of the offsets are not computable here.
     let cross_oo = (o1.x * o2.y) - (o1.y * o2.x);
     let sweep = stroke_normalize_positive_angle(atan2(cross_oo, dot(o1, o2)));
-    let da = acos(half_width / (half_width + (0.125 / arc_detail_scale))) * 2.0;
     stroke_chain_point(path_ix, last, v1 + o1, transform);
-    // Bounded for GPU safety; matches the CPU count for all real detail scales.
-    let n = clamp(i32(sweep / da), 0, 1024);
-    let step = sweep / f32(n + 1);
+    let n = stroke_arc_subdivision_count(half_width, sweep, arc_detail_scale);
+    let step = sweep / f32(n + 1u);
     let rot_c = cos(step);
     let rot_s = sin(step);
     var offset = o1;
-    for (var i = 0; i < n; i += 1) {
+    for (var i = 1u; i <= n; i += 1u) {
         offset = vec2((offset.x * rot_c) - (offset.y * rot_s), (offset.x * rot_s) + (offset.y * rot_c));
         stroke_chain_point(path_ix, last, v1 + offset, transform);
     }

src/ImageSharp.Drawing/Processing/Backends/DefaultRasterizer.Linearizer.cs

@@ -700,11 +700,15 @@ private void SplitAcrossBands(int x0, int y0, int x1, int y1, uint tag)
 
             while (currentBand != endBand)
             {
-                // Walk to the band boundary in the direction of travel, interpolating X in
-                // 64-bit so the dx * deltaY product cannot overflow 32-bit fixed point.
+                // Walk to the band boundary in the direction of travel. X at the boundary is
+                // interpolated from the segment's own endpoint and rounded to nearest, so every
+                // boundary sits within half a fixed-point unit of the true edge. Stepping from the
+                // previous boundary instead accumulates the truncation of each division along the
+                // segment. The product is formed in 64-bit so dx * deltaY cannot overflow.
                 int bandBoundaryY = dy > 0 ? bandTopStart + ((currentBand + 1) * bandHeight) : bandTopStart + (currentBand * bandHeight);
-                int deltaY = bandBoundaryY - currentY;
-                int nextX = currentX + (int)(((long)dx * deltaY) / dy);
+                long numerator = (long)dx * (bandBoundaryY - y0);
+                long half = dy / 2;
+                int nextX = x0 + (int)((numerator ^ dy) >= 0 ? (numerator + half) / dy : (numerator - half) / dy);
                 int rowTop = bandTopStart + (currentBand * bandHeight);
 
                 // Each retained segment is stored in the local coordinate space of its owning band.

src/ImageSharp.Drawing/Processing/Backends/DefaultRasterizer.Stroke.cs

@@ -202,6 +202,31 @@ internal static partial class DefaultRasterizer
                 rowBandCount));
     }
 
+    /// <summary>
+    /// Returns the tessellation segment count used for one round join or cap arc.
+    /// Ported to the GPU as <c>stroke_arc_subdivision_count</c> in <c>path_lowering.wgsl</c>;
+    /// changes here must be mirrored there.
+    /// </summary>
+    /// <param name="radius">The arc radius.</param>
+    /// <param name="angle">The arc sweep angle in radians.</param>
+    /// <param name="arcDetailScale">The tessellation detail scale.</param>
+    /// <returns>The number of intermediate tessellation points.</returns>
+    public static int GetArcSubdivisionCount(float radius, double angle, double arcDetailScale)
+    {
+        double safeRadius = Math.Max(radius, StrokeDirectionEpsilon);
+        double safeScale = Math.Max(arcDetailScale, 0.01D);
+
+        // Chordal-error step: theta is the largest angular step whose chord midpoint
+        // deviates from the arc by at most 0.125 / scale pixels, so tessellation density
+        // adapts to both radius and requested detail.
+        double ratio = safeRadius / (safeRadius + (0.125D / safeScale));
+        ratio = Math.Clamp(ratio, -1D, 1D);
+        double theta = Math.Acos(ratio) * 2D;
+        return theta <= 0D
+            ? 0
+            : Math.Max(0, (int)(angle / theta));
+    }
+
     /// <summary>
     /// Expands one stroked centerline geometry once into retained per-band line storage.
     /// </summary>
@@ -1453,31 +1478,6 @@ private static void AppendEdgeInterval(
         }
     }
 
-    /// <summary>
-    /// Returns the tessellation segment count used for one round join or cap arc.
-    /// Ported to the GPU as <c>stroke_arc_subdivision_count</c> in <c>path_lowering.wgsl</c>;
-    /// changes here must be mirrored there.
-    /// </summary>
-    /// <param name="radius">The arc radius.</param>
-    /// <param name="angle">The arc sweep angle in radians.</param>
-    /// <param name="arcDetailScale">The tessellation detail scale.</param>
-    /// <returns>The number of intermediate tessellation points.</returns>
-    private static int GetArcSubdivisionCount(float radius, double angle, double arcDetailScale)
-    {
-        double safeRadius = Math.Max(radius, StrokeDirectionEpsilon);
-        double safeScale = Math.Max(arcDetailScale, 0.01D);
-
-        // AGG's chordal-error step: theta is the largest angular step whose chord midpoint
-        // deviates from the arc by at most 0.125 / scale pixels, so tessellation density
-        // adapts to both radius and requested detail.
-        double ratio = safeRadius / (safeRadius + (0.125D / safeScale));
-        ratio = Math.Clamp(ratio, -1D, 1D);
-        double theta = Math.Acos(ratio) * 2D;
-        return theta <= 0D
-            ? 0
-            : Math.Max(0, (int)(angle / theta));
-    }
-
     /// <summary>
     /// Returns the stroke offset unit normal for a normalized tangent.
     /// </summary>

src/ImageSharp.Drawing/Processing/Backends/DefaultRasterizer.StrokeLinearizer.cs

@@ -898,7 +898,8 @@ private void CalcMiter(
 
         /// <summary>
         /// Direct port of <c>PolygonStroker.CalcArc</c>. Emits intermediate arc vertices
-        /// around a join center between two offset vectors. Ported to the GPU as
+        /// around a join center between two offset vectors. The interior vertex count comes from
+        /// <see cref="GetArcSubdivisionCount"/>, shared with the round cap arcs. Ported to the GPU as
         /// <c>stroke_calc_arc</c> in <c>path_lowering.wgsl</c>; changes here must be mirrored there.
         /// </summary>
         /// <param name="contour">The active contour state.</param>
@@ -922,11 +923,6 @@ private void CalcArc(
             float strokeWidth = this.stroke.HalfWidth;
             double a1 = Math.Atan2(dy1, dx1);
             double a2 = Math.Atan2(dy2, dx2);
-
-            // AGG's chordal-error step: da is the largest angular step whose chord stays within
-            // 0.125 / arc-detail-scale pixels of the true arc.
-            double widthAbs = strokeWidth;
-            double da = Math.Acos(widthAbs / (widthAbs + (0.125D / this.stroke.ArcDetailScale))) * 2D;
             this.AppendContourPoint(ref contour, new Vector2(x + dx1, y + dy1), contained);
 
             // Wrap the end angle forward so the sweep is always positive.
@@ -937,8 +933,9 @@ private void CalcArc(
 
             // Distribute the sweep evenly over n interior points so the last step lands exactly
             // on the end offset.
-            int n = (int)((a2 - a1) / da);
-            da = (a2 - a1) / (n + 1);
+            double sweep = a2 - a1;
+            int n = GetArcSubdivisionCount(strokeWidth, sweep, this.stroke.ArcDetailScale);
+            double da = sweep / (n + 1);
             a1 += da;
             for (int i = 0; i < n; i++)
             {

src/ImageSharp.Drawing/Processing/Backends/DefaultRasterizer.cs

@@ -55,8 +55,8 @@ internal static partial class DefaultRasterizer
     private static readonly int WordBitCount = nint.Size * 8;
 
     /// <summary>
-    /// Right-shift that converts an accumulated doubled cell area (max 2 * 256 * 256) down to the
-    /// 0..256 coverage step domain used by <see cref="Context.AreaToCoverage"/>.
+    /// Left-shift that scales a 24.8 winding cover (256 per fully covered pixel) up to the doubled
+    /// cell area domain accumulated by the cells (<see cref="FullCoverageArea"/> per fully covered pixel).
     /// </summary>
     private const int AreaToCoverageShift = 9;
 
@@ -98,24 +98,24 @@ internal static partial class DefaultRasterizer
     private const int CrossingShift = ProfileIdBits + 1;
 
     /// <summary>
-    /// Number of discrete coverage steps per fully covered pixel (one 24.8 unit of winding).
+    /// The doubled cell area of one fully covered pixel (2 * 256 * 256), one 24.8 unit of winding.
     /// </summary>
-    private const int CoverageStepCount = 256;
+    private const int FullCoverageArea = (FixedOne * FixedOne) << 1;
 
     /// <summary>
-    /// Bitmask implementing modulo 2 * <see cref="CoverageStepCount"/> for even-odd wrapping.
+    /// Bitmask implementing modulo 2 * <see cref="FullCoverageArea"/> for even-odd wrapping.
     /// </summary>
-    private const int EvenOddMask = (CoverageStepCount * 2) - 1;
+    private const int EvenOddMask = (FullCoverageArea * 2) - 1;
 
     /// <summary>
     /// Length of one even-odd winding period; values past the midpoint mirror back down.
     /// </summary>
-    private const int EvenOddPeriod = CoverageStepCount * 2;
+    private const int EvenOddPeriod = FullCoverageArea * 2;
 
     /// <summary>
-    /// Multiplier converting integer coverage steps to normalized [0, 1] coverage.
+    /// Multiplier converting doubled cell area to normalized [0, 1] coverage.
     /// </summary>
-    private const float CoverageScale = 1F / CoverageStepCount;
+    private const float CoverageScale = 1F / FullCoverageArea;
 
     /// <summary>
     /// Gets the preferred scene row height used by the CPU rasterizer.
@@ -1625,20 +1625,21 @@ private void CaptureCrossings(int x0, int y0, int x1, int y1, uint tag)
         /// </summary>
         /// <param name="area">
         /// The accumulated doubled signed area in fixed-point cell units; a fully covered pixel
-        /// corresponds to 2 * 256 * 256, which <see cref="AreaToCoverageShift"/> maps to <see cref="CoverageStepCount"/>.
+        /// corresponds to <see cref="FullCoverageArea"/>.
         /// </param>
         /// <returns>The normalized coverage value in [0, 1].</returns>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private readonly float AreaToCoverage(int area)
         {
-            int signedArea = area >> AreaToCoverageShift;
-            int absoluteArea = signedArea < 0 ? -signedArea : signedArea;
+            // The area keeps its full precision. Quantizing it to 1/256 steps before the blend
+            // moves edge pixels up to one output level away from the exact composite.
+            int absoluteArea = area < 0 ? -area : area;
             float coverage;
 
             if (this.intersectionRule == IntersectionRule.NonZero)
             {
                 // Non-zero winding clamps absolute winding accumulation to [0, 1].
-                if (absoluteArea >= CoverageStepCount)
+                if (absoluteArea >= FullCoverageArea)
                 {
                     coverage = 1F;
                 }
@@ -1649,14 +1650,14 @@ private readonly float AreaToCoverage(int area)
             }
             else
             {
-                // Even-odd wraps every 2*CoverageStepCount and mirrors second half.
+                // Even-odd wraps every 2 * FullCoverageArea and mirrors the second half.
                 int wrapped = absoluteArea & EvenOddMask;
-                if (wrapped > CoverageStepCount)
+                if (wrapped > FullCoverageArea)
                 {
                     wrapped = EvenOddPeriod - wrapped;
                 }
 
-                coverage = wrapped >= CoverageStepCount ? 1F : wrapped * CoverageScale;
+                coverage = wrapped >= FullCoverageArea ? 1F : wrapped * CoverageScale;
             }
 
             if (this.coverageBoost != 0F)
@@ -1992,10 +1993,12 @@ private void RowDownR(int rowIndex, int p0x, int p0y, int p1x, int p1y)
             }
 
             // pp/mod/lift/rem implement an integer DDA that advances y at column boundaries
-            // without accumulating rounding error; the remainder carries the exact fraction.
+            // without accumulating rounding error; the remainder carries the exact fraction. The
+            // half-divisor start term rounds every boundary to the nearest unit instead of
+            // flooring it, so the walked edge sits within half a unit of the true line.
             int dx = p1x - p0x;
             int dy = p1y - p0y;
-            int pp = (FixedOne - fx0) * dy;
+            int pp = ((FixedOne - fx0) * dy) + (dx >> 1);
             int cy = p0y + (pp / dx);
 
             this.Cell(rowIndex, columnIndex0, fx0, p0y, FixedOne, cy);
@@ -2073,7 +2076,7 @@ private void RowUpR(int rowIndex, int p0x, int p0y, int p1x, int p1y)
 
             int dx = p1x - p0x;
             int dy = p0y - p1y;
-            int pp = (FixedOne - fx0) * dy;
+            int pp = ((FixedOne - fx0) * dy) + (dx >> 1);
             int cy = p0y - (pp / dx);
 
             this.Cell(rowIndex, columnIndex0, fx0, p0y, FixedOne, cy);
@@ -2151,7 +2154,7 @@ private void RowDownL(int rowIndex, int p0x, int p0y, int p1x, int p1y)
 
             int dx = p0x - p1x;
             int dy = p1y - p0y;
-            int pp = fx0 * dy;
+            int pp = (fx0 * dy) + (dx >> 1);
             int cy = p0y + (pp / dx);
 
             this.Cell(rowIndex, columnIndex0, fx0, p0y, 0, cy);
@@ -2229,7 +2232,7 @@ private void RowUpL(int rowIndex, int p0x, int p0y, int p1x, int p1y)
 
             int dx = p0x - p1x;
             int dy = p0y - p1y;
-            int pp = fx0 * dy;
+            int pp = (fx0 * dy) + (dx >> 1);
             int cy = p0y - (pp / dx);
 
             this.Cell(rowIndex, columnIndex0, fx0, p0y, 0, cy);
@@ -2301,8 +2304,9 @@ private void LineDownR(int rowIndex0, int rowIndex1, int x0, int y0, int x1, int
             int fy1 = y1 - (rowIndex1 << FixedShift);
 
             // p/delta/mod/rem implement an integer DDA that advances x at row boundaries
-            // without per-row floating-point math.
-            int p = (FixedOne - fy0) * dx;
+            // without per-row floating-point math. The half-divisor start term rounds every
+            // boundary to the nearest unit instead of flooring it.
+            int p = ((FixedOne - fy0) * dx) + (dy >> 1);
             int delta = p / dy;
             int cx = x0 + delta;
 
@@ -2353,7 +2357,7 @@ private void LineUpR(int rowIndex0, int rowIndex1, int x0, int y0, int x1, int y
             int fy1 = y1 - (rowIndex1 << FixedShift);
 
             // Upward version of the same integer DDA stepping as LineDownR.
-            int p = fy0 * dx;
+            int p = (fy0 * dx) + (dy >> 1);
             int delta = p / dy;
             int cx = x0 + delta;
 
@@ -2403,7 +2407,7 @@ private void LineDownL(int rowIndex0, int rowIndex1, int x0, int y0, int x1, int
             int fy1 = y1 - (rowIndex1 << FixedShift);
 
             // Right-to-left variant of the integer DDA.
-            int p = (FixedOne - fy0) * dx;
+            int p = ((FixedOne - fy0) * dx) + (dy >> 1);
             int delta = p / dy;
             int cx = x0 - delta;
 
@@ -2453,7 +2457,7 @@ private void LineUpL(int rowIndex0, int rowIndex1, int x0, int y0, int x1, int y
             int fy1 = y1 - (rowIndex1 << FixedShift);
 
             // Upward + right-to-left variant of the integer DDA.
-            int p = fy0 * dx;
+            int p = (fy0 * dx) + (dy >> 1);
             int delta = p / dy;
             int cx = x0 - delta;
 

src/ImageSharp.Drawing/Processing/StrokeOptions.cs

@@ -6,11 +6,22 @@ namespace SixLabors.ImageSharp.Drawing.Processing;
 /// <inheritdoc cref="PolygonClipper.StrokeOptions" />
 public sealed class StrokeOptions : IEquatable<StrokeOptions?>
 {
+    private double arcDetailScale = 1D;
+
     /// <inheritdoc cref="PolygonClipper.StrokeOptions.MiterLimit" />
     public double MiterLimit { get; set; } = 4D;
 
     /// <inheritdoc cref="PolygonClipper.StrokeOptions.ArcDetailScale" />
-    public double ArcDetailScale { get; set; } = 1D;
+    /// <exception cref="ArgumentOutOfRangeException">The value is not greater than zero.</exception>
+    public double ArcDetailScale
+    {
+        get => this.arcDetailScale;
+        set
+        {
+            Guard.MustBeGreaterThan(value, 0, nameof(this.ArcDetailScale));
+            this.arcDetailScale = value;
+        }
+    }
 
     /// <inheritdoc cref="PolygonClipper.StrokeOptions.LineJoin" />
     public LineJoin LineJoin { get; set; } = LineJoin.Bevel;