Add FillRule::clockwise to the runtime

Diffs= 11d4e46498 Add FillRule::clockwise to the runtime (#8786) Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>
rive-app · Dec 20, 2024 · 4356a5c · 4356a5c
1 parent 2490a1b
commit 4356a5c
Show file tree

Hide file tree

Showing 29 changed files with 414 additions and 153 deletions.
diff --git a/.rive_head b/.rive_head
@@ -1 +1 @@
-49e1109e3f8b3b04d4cf152bf2a826e6bea8aae3
+11d4e46498b239fd56c15edcfe2c9d3e752e12cc
diff --git a/cg_renderer/src/cg_factory.cpp b/cg_renderer/src/cg_factory.cpp
@@ -23,8 +23,8 @@ static CGAffineTransform convert(const Mat2D& m)
 
 static CGPathDrawingMode convert(FillRule rule)
 {
-    return (rule == FillRule::nonZero) ? CGPathDrawingMode::kCGPathFill
-                                       : CGPathDrawingMode::kCGPathEOFill;
+    return (rule == FillRule::evenOdd) ? CGPathDrawingMode::kCGPathEOFill
+                                       : CGPathDrawingMode::kCGPathFill;
 }
 
 static CGLineJoin convert(StrokeJoin j)

diff --git a/include/rive/math/path_types.hpp b/include/rive/math/path_types.hpp
@@ -14,6 +14,7 @@ enum class FillRule
 {
     nonZero,
     evenOdd,
+    clockwise,
 };
 
 enum class PathDirection

diff --git a/renderer/include/rive/renderer/draw.hpp b/renderer/include/rive/renderer/draw.hpp
@@ -224,7 +224,6 @@ class RiveRenderPathDraw : public Draw
                        gpu::InterlockMode);
 
     const Gradient* gradient() const { return m_gradientRef; }
-    FillRule fillRule() const { return m_fillRule; }
     gpu::PaintType paintType() const { return m_paintType; }
     float strokeRadius() const { return m_strokeRadius; }
     gpu::ContourDirections contourDirections() const
@@ -310,8 +309,6 @@ class RiveRenderPathDraw : public Draw
 
     const RiveRenderPath* const m_pathRef;
     const Gradient* m_gradientRef;
-    const FillRule m_fillRule; // Bc RiveRenderPath fillRule can mutate during
-                               // the artboard draw process.
     const gpu::PaintType m_paintType;
     float m_strokeRadius = 0;
     gpu::ContourDirections m_contourDirections;

diff --git a/renderer/include/rive/renderer/gpu.hpp b/renderer/include/rive/renderer/gpu.hpp
@@ -773,10 +773,12 @@ enum class DrawContents
     none = 0,
     opaquePaint = 1 << 0,
     stroke = 1 << 1,
-    evenOddFill = 1 << 2,
-    activeClip = 1 << 3,
-    clipUpdate = 1 << 4,
-    advancedBlend = 1 << 5,
+    clockwiseFill = 1 << 2,
+    nonZeroFill = 1 << 3,
+    evenOddFill = 1 << 4,
+    activeClip = 1 << 5,
+    clipUpdate = 1 << 6,
+    advancedBlend = 1 << 7,
 };
 RIVE_MAKE_ENUM_BITSET(DrawContents)
 
@@ -917,7 +919,8 @@ struct FlushDescriptor
     uint32_t complexGradRowsTop = 0;
     uint32_t complexGradRowsHeight = 0;
     uint32_t tessDataHeight = 0;
-    bool clockwiseFill = false; // Override path fill rules with "clockwise".
+    // Override path fill rules with "clockwise".
+    bool clockwiseFillOverride = false;
     bool hasTriangleVertices = false;
     bool wireframe = false;
     bool isFinalFlushOfFrame = false;
@@ -1108,7 +1111,7 @@ struct PaintData
     constexpr static StorageBufferStructure kBufferStructure =
         StorageBufferStructure::uint32x2;
 
-    void set(FillRule,
+    void set(DrawContents singleDrawContents,
              PaintType,
              SimplePaintValue,
              GradTextureLayout,

diff --git a/renderer/include/rive/renderer/render_context.hpp b/renderer/include/rive/renderer/render_context.hpp
@@ -100,7 +100,7 @@ class RenderContext : public RiveRenderFactory
         bool strokesDisabled = false;
         // Override all paths' fill rules (winding or even/odd) to emulate
         // clockwiseAtomic mode.
-        bool clockwiseFill = false;
+        bool clockwiseFillOverride = false;
     };
 
     // Called at the beginning of a frame and establishes where and how it will

diff --git a/renderer/path_fiddle/path_fiddle.cpp b/renderer/path_fiddle/path_fiddle.cpp
@@ -755,7 +755,7 @@ void riveMainLoop()
         .wireframe = s_wireframe,
         .fillsDisabled = s_disableFill,
         .strokesDisabled = s_disableStroke,
-        .clockwiseFill = s_clockwiseFill,
+        .clockwiseFillOverride = s_clockwiseFill,
     });
 
     int instances = 1;

diff --git a/renderer/src/d3d/render_context_d3d_impl.cpp b/renderer/src/d3d/render_context_d3d_impl.cpp
@@ -1113,7 +1113,7 @@ void RenderContextD3DImpl::setPipelineLayoutAndShaders(
         }
         if (pixelShaderMiscFlags & gpu::ShaderMiscFlags::clockwiseFill)
         {
-            s << "#define " << GLSL_CLOCKWISE_FILL << '\n';
+            s << "#define " << GLSL_CLOCKWISE_FILL << " 1\n";
         }
         switch (drawType)
         {
@@ -1704,7 +1704,8 @@ void RenderContextD3DImpl::flush(const FlushDescriptor& desc)
             pixelShaderMiscFlags |=
                 gpu::ShaderMiscFlags::fixedFunctionColorOutput;
         }
-        if (desc.clockwiseFill)
+        if (desc.interlockMode == gpu::InterlockMode::rasterOrdering &&
+            (batch.drawContents & gpu::DrawContents::clockwiseFill))
         {
             pixelShaderMiscFlags |= gpu::ShaderMiscFlags::clockwiseFill;
         }

diff --git a/renderer/src/draw.cpp b/renderer/src/draw.cpp
@@ -456,7 +456,7 @@ RiveRenderPathDraw::RiveRenderPathDraw(
     AABB bounds,
     const Mat2D& matrix,
     rcp<const RiveRenderPath> path,
-    FillRule fillRule,
+    FillRule initialFillRule,
     const RiveRenderPaint* paint,
     Type type,
     const RenderContext::FrameDescriptor& frameDesc,
@@ -468,12 +468,6 @@ RiveRenderPathDraw::RiveRenderPathDraw(
          type),
     m_pathRef(path.release()),
     m_gradientRef(safe_ref(paint->getGradient())),
-    m_fillRule(paint->getIsStroked() || frameDesc.clockwiseFill
-                   // Fill rule is irrelevant for stroking and clockwiseFill,
-                   // but override it to nonZero so the triangulator doesn't do
-                   // evenOdd optimizations in "clockwiseFill" mode.
-                   ? FillRule::nonZero
-                   : fillRule),
     m_paintType(paint->getType())
 {
     assert(m_pathRef != nullptr);
@@ -495,7 +489,16 @@ RiveRenderPathDraw::RiveRenderPathDraw(
                                              // RiveRenderer::drawPath().
         assert(m_strokeRadius > 0);
     }
-    else if (m_fillRule == FillRule::evenOdd)
+    else if (initialFillRule == FillRule::clockwise ||
+             frameDesc.clockwiseFillOverride)
+    {
+        m_drawContents |= gpu::DrawContents::clockwiseFill;
+    }
+    else if (initialFillRule == FillRule::nonZero)
+    {
+        m_drawContents |= gpu::DrawContents::nonZeroFill;
+    }
+    else if (initialFillRule == FillRule::evenOdd)
     {
         m_drawContents |= gpu::DrawContents::evenOddFill;
     }
@@ -513,10 +516,32 @@ RiveRenderPathDraw::RiveRenderPathDraw(
         // Stroke triangles are always forward.
         m_contourDirections = gpu::ContourDirections::forward;
     }
+    else if (initialFillRule == FillRule::clockwise)
+    {
+        // Clockwise paths need to be reversed when the matrix is left-handed,
+        // so that the intended forward triangles remain clockwise.
+        float det = matrix.xx() * matrix.yy() - matrix.yx() * matrix.xy();
+        if (det < 0)
+        {
+            m_contourDirections =
+                interlockMode == gpu::InterlockMode::msaa
+                    ? gpu::ContourDirections::reverse
+                    : gpu::ContourDirections::forwardThenReverse;
+            m_contourFlags |= NEGATE_PATH_FILL_COVERAGE_FLAG; // ignored by msaa
+        }
+        else
+        {
+            m_contourDirections =
+                interlockMode == gpu::InterlockMode::msaa
+                    ? gpu::ContourDirections::forward
+                    : gpu::ContourDirections::reverseThenForward;
+        }
+    }
     else if (interlockMode != gpu::InterlockMode::msaa)
     {
         // atomic and rasterOrdering fills need reverse AND forward triangles.
-        if (frameDesc.clockwiseFill && !m_pathRef->isClockwiseDominant(matrix))
+        if (frameDesc.clockwiseFillOverride &&
+            !m_pathRef->isClockwiseDominant(matrix))
         {
             // For clockwiseFill, this is also our opportunity to logically
             // reverse the winding of the path, if it is predominantly
@@ -529,20 +554,25 @@ RiveRenderPathDraw::RiveRenderPathDraw(
             m_contourDirections = gpu::ContourDirections::reverseThenForward;
         }
     }
-    else if (m_fillRule != FillRule::evenOdd)
-    {
-        // Emit "nonZero" msaa fills in a direction such that the dominant
-        // triangle winding area is always clockwise. This maximizes pixel
-        // throughput since we will draw counterclockwise triangles twice and
-        // clockwise only once.
-        m_contourDirections = m_pathRef->isClockwiseDominant(matrix)
-                                  ? gpu::ContourDirections::forward
-                                  : gpu::ContourDirections::reverse;
-    }
     else
     {
-        // "evenOdd" msaa fills just get drawn twice, so any direction is fine.
-        m_contourDirections = gpu::ContourDirections::forward;
+        if (initialFillRule == FillRule::nonZero ||
+            frameDesc.clockwiseFillOverride)
+        {
+            // Emit "nonZero" msaa fills in a direction such that the dominant
+            // triangle winding area is always clockwise. This maximizes pixel
+            // throughput since we will draw counterclockwise triangles twice
+            // and clockwise only once.
+            m_contourDirections = m_pathRef->isClockwiseDominant(matrix)
+                                      ? gpu::ContourDirections::forward
+                                      : gpu::ContourDirections::reverse;
+        }
+        else
+        {
+            // "evenOdd" msaa fills just get drawn twice, so any direction is
+            // fine.
+            m_contourDirections = gpu::ContourDirections::forward;
+        }
     }
 
     m_simplePaintValue = paint->getSimpleValue();
@@ -2100,7 +2130,10 @@ void RiveRenderPathDraw::iterateInteriorTriangulation(
             triangulatorAxis == TriangulatorAxis::horizontal
                 ? GrTriangulator::Comparator::Direction::kHorizontal
                 : GrTriangulator::Comparator::Direction::kVertical,
-            m_fillRule,
+            // clockwise and nonZero paths both get triangulated as nonZero,
+            // because clockwise fill still needs the backwards triangles for
+            // borrowed coverage.
+            isEvenOddFill() ? FillRule::evenOdd : FillRule::nonZero,
             allocator);
         float matrixDeterminant =
             m_matrix[0] * m_matrix[3] - m_matrix[2] * m_matrix[1];

diff --git a/renderer/src/gl/render_context_gl_impl.cpp b/renderer/src/gl/render_context_gl_impl.cpp
@@ -1185,7 +1185,8 @@ void RenderContextGLImpl::flush(const FlushDescriptor& desc)
             m_plsImpl != nullptr
                 ? m_plsImpl->shaderMiscFlags(desc, batch.drawType)
                 : gpu::ShaderMiscFlags::none;
-        if (desc.clockwiseFill)
+        if (desc.interlockMode == gpu::InterlockMode::rasterOrdering &&
+            (batch.drawContents & gpu::DrawContents::clockwiseFill))
         {
             fragmentShaderMiscFlags |= gpu::ShaderMiscFlags::clockwiseFill;
         }
@@ -1303,7 +1304,8 @@ void RenderContextGLImpl::flush(const FlushDescriptor& desc)
             m_plsImpl != nullptr
                 ? m_plsImpl->shaderMiscFlags(desc, batch.drawType)
                 : gpu::ShaderMiscFlags::none;
-        if (desc.clockwiseFill)
+        if (desc.interlockMode == gpu::InterlockMode::rasterOrdering &&
+            (batch.drawContents & gpu::DrawContents::clockwiseFill))
         {
             fragmentShaderMiscFlags |= gpu::ShaderMiscFlags::clockwiseFill;
         }
@@ -1499,8 +1501,17 @@ void RenderContextGLImpl::flush(const FlushDescriptor& desc)
                     drawHelper.draw();
 
                     // Clean up backward triangles in the stencil buffer, (also
-                    // filling negative winding numbers).
+                    // filling negative winding numbers for nonZero fill).
                     m_state->setCullFace(GL_FRONT);
+                    if (batch.drawContents & gpu::DrawContents::clockwiseFill)
+                    {
+                        // For clockwise fill, disable the color mask when
+                        // cleaning up backward triangles. This mode only fills
+                        // in forward triangles.
+                        m_state->setWriteMasks(false,
+                                               false,
+                                               isClipUpdate ? 0xff : 0x7f);
+                    }
                     drawHelper.draw();
                     break;
                 }

diff --git a/renderer/src/gpu.cpp b/renderer/src/gpu.cpp
@@ -435,7 +435,7 @@ void PathData::set(const Mat2D& m,
     m_coverageBufferRange.offsetY = coverageBufferRange.offsetY;
 }
 
-void PaintData::set(FillRule fillRule,
+void PaintData::set(DrawContents singleDrawContents,
                     PaintType paintType,
                     SimplePaintValue simplePaintValue,
                     GradTextureLayout gradTextureLayout,
@@ -483,9 +483,13 @@ void PaintData::set(FillRule fillRule,
             break;
         }
     }
-    if (fillRule == FillRule::evenOdd)
+    if (singleDrawContents & gpu::DrawContents::nonZeroFill)
     {
-        localParams |= PAINT_FLAG_EVEN_ODD;
+        localParams |= PAINT_FLAG_NON_ZERO_FILL;
+    }
+    else if (singleDrawContents & gpu::DrawContents::evenOddFill)
+    {
+        localParams |= PAINT_FLAG_EVEN_ODD_FILL;
     }
     if (hasClipRect)
     {

diff --git a/renderer/src/metal/background_shader_compiler.mm b/renderer/src/metal/background_shader_compiler.mm
@@ -118,7 +118,7 @@
         }
         if (shaderMiscFlags & gpu::ShaderMiscFlags::clockwiseFill)
         {
-            defines[@GLSL_CLOCKWISE_FILL] = @"";
+            defines[@GLSL_CLOCKWISE_FILL] = @"1";
         }
 
         auto source =
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		49e1109e3f8b3b04d4cf152bf2a826e6bea8aae3
		11d4e46498b239fd56c15edcfe2c9d3e752e12cc