From b04bac0a635f006f9f3d25787950108fadb923c1 Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Wed, 3 May 2023 15:32:51 +0100
Subject: [PATCH] CoreGraphicsContext: Fix issue where clipping a gradient on a
 layer-backed view could cause rendering glitches

---
 .../native/juce_CoreGraphicsContext_mac.h     |  12 +-
 .../native/juce_CoreGraphicsContext_mac.mm    | 153 +++++++-----------
 2 files changed, 69 insertions(+), 96 deletions(-)

diff --git a/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.h b/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.h
index 6230c99c1c..0d82ef6704 100644
--- a/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.h
+++ b/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.h
@@ -58,6 +58,11 @@ namespace detail
         void operator() (CGColorRef ptr) const noexcept { CGColorRelease (ptr); }
     };
 
+    struct PathDelete
+    {
+        void operator() (CGMutablePathRef ptr) const noexcept { CGPathRelease (ptr); }
+    };
+
     //==============================================================================
     using ColorSpacePtr = std::unique_ptr<CGColorSpace, ColorSpaceDelete>;
     using ContextPtr = std::unique_ptr<CGContext, ContextDelete>;
@@ -65,6 +70,7 @@ namespace detail
     using ImagePtr = std::unique_ptr<CGImage, ImageDelete>;
     using GradientPtr = std::unique_ptr<CGGradient, GradientDelete>;
     using ColorPtr = std::unique_ptr<CGColor, ColorDelete>;
+    using PathPtr = std::unique_ptr<std::remove_pointer_t<CGMutablePathRef>, PathDelete>;
 }
 
 //==============================================================================
@@ -120,8 +126,7 @@ private:
     detail::ContextPtr context;
     const CGFloat flipHeight;
     detail::ColorSpacePtr rgbColourSpace, greyColourSpace;
-    mutable Rectangle<int> lastClipRect;
-    mutable bool lastClipRectIsValid = false;
+    mutable std::optional<Rectangle<int>> lastClipRect;
 
     struct SavedState
     {
@@ -142,9 +147,8 @@ private:
     std::unique_ptr<SavedState> state;
     OwnedArray<SavedState> stateStack;
 
+    void setContextClipToPath (const Path&, const AffineTransform&);
     void drawGradient();
-    void createPath (const Path&) const;
-    void createPath (const Path&, const AffineTransform&) const;
     void flip() const;
     void applyTransform (const AffineTransform&) const;
     void drawImage (const Image&, const AffineTransform&, bool fillEntireClipAsTiles);
diff --git a/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.mm b/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.mm
index 8496147b0a..3d75bdad9d 100644
--- a/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.mm
+++ b/modules/juce_graphics/native/juce_CoreGraphicsContext_mac.mm
@@ -221,8 +221,8 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
 {
     CGContextTranslateCTM (context.get(), o.x, -o.y);
 
-    if (lastClipRectIsValid)
-        lastClipRect.translate (-o.x, -o.y);
+    if (lastClipRect.has_value())
+        lastClipRect->translate (-o.x, -o.y);
 }
 
 void CoreGraphicsContext::addTransform (const AffineTransform& transform)
@@ -231,7 +231,7 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
                                     .followedBy (transform)
                                     .translated (0, (float) -flipHeight)
                                     .scaled (1.0f, -1.0f));
-    lastClipRectIsValid = false;
+    lastClipRect.reset();
 
     jassert (getPhysicalPixelScaleFactor() > 0.0f);
 }
@@ -249,14 +249,14 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
     CGContextClipToRect (context.get(), CGRectMake (r.getX(), flipHeight - r.getBottom(),
                                                     r.getWidth(), r.getHeight()));
 
-    if (lastClipRectIsValid)
+    if (lastClipRect.has_value())
     {
         // This is actually incorrect, because the actual clip region may be complex, and
         // clipping its bounds to a rect may not be right... But, removing this shortcut
         // doesn't actually fix anything because CoreGraphics also ignores complex regions
         // when calculating the resultant clip bounds, and makes the same mistake!
-        lastClipRect = lastClipRect.getIntersection (r);
-        return ! lastClipRect.isEmpty();
+        lastClipRect = lastClipRect->getIntersection (r);
+        return ! lastClipRect->isEmpty();
     }
 
     return ! isClipEmpty();
@@ -267,7 +267,6 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
     if (clipRegion.isEmpty())
     {
         CGContextClipToRect (context.get(), CGRectZero);
-        lastClipRectIsValid = true;
         lastClipRect = Rectangle<int>();
         return false;
     }
@@ -280,7 +279,7 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
         rects[i++] = CGRectMake (r.getX(), flipHeight - r.getBottom(), r.getWidth(), r.getHeight());
 
     CGContextClipToRects (context.get(), rects, numRects);
-    lastClipRectIsValid = false;
+    lastClipRect.reset();
     return true;
 }
 
@@ -296,16 +295,54 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
     clipToRectangleListWithoutTest (remaining);
 }
 
-void CoreGraphicsContext::clipToPath (const Path& path, const AffineTransform& transform)
+static CGAffineTransform makeCGAffineTransform (const AffineTransform& transform)
+{
+    CGAffineTransform t;
+    t.a  = transform.mat00;
+    t.b  = transform.mat10;
+    t.c  = transform.mat01;
+    t.d  = transform.mat11;
+    t.tx = transform.mat02;
+    t.ty = transform.mat12;
+    return t;
+}
+
+static detail::PathPtr createPath (const Path& path, const AffineTransform& t)
 {
-    createPath (path, transform);
+    const auto transform = makeCGAffineTransform (t);
+    detail::PathPtr result { CGPathCreateMutable() };
+
+    for (Path::Iterator i (path); i.next();)
+    {
+        switch (i.elementType)
+        {
+            case Path::Iterator::startNewSubPath:  CGPathMoveToPoint (result.get(), &transform, i.x1, i.y1); break;
+            case Path::Iterator::lineTo:           CGPathAddLineToPoint (result.get(), &transform, i.x1, i.y1); break;
+            case Path::Iterator::quadraticTo:      CGPathAddQuadCurveToPoint (result.get(), &transform, i.x1, i.y1, i.x2, i.y2); break;
+            case Path::Iterator::cubicTo:          CGPathAddCurveToPoint (result.get(), &transform, i.x1, i.y1, i.x2, i.y2, i.x3, i.y3); break;
+            case Path::Iterator::closePath:        CGPathCloseSubpath (result.get()); break;
+            default:                               jassertfalse; break;
+        }
+    }
+
+    return result;
+}
+
+void CoreGraphicsContext::setContextClipToPath (const Path& path, const AffineTransform& transform)
+{
+    CGContextBeginPath (context.get());
+    CGContextAddPath (context.get(), createPath (path, transform).get());
 
     if (path.isUsingNonZeroWinding())
         CGContextClip (context.get());
     else
         CGContextEOClip (context.get());
+}
 
-    lastClipRectIsValid = false;
+void CoreGraphicsContext::clipToPath (const Path& path, const AffineTransform& transform)
+{
+    setContextClipToPath (path, transform);
+    lastClipRect.reset();
 }
 
 void CoreGraphicsContext::clipToImageAlpha (const Image& sourceImage, const AffineTransform& transform)
@@ -329,7 +366,7 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
         applyTransform (t.inverted());
         flip();
 
-        lastClipRectIsValid = false;
+        lastClipRect.reset();
     }
 }
 
@@ -340,18 +377,17 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
 
 Rectangle<int> CoreGraphicsContext::getClipBounds() const
 {
-    if (! lastClipRectIsValid)
+    if (! lastClipRect.has_value())
     {
         auto bounds = CGRectIntegral (CGContextGetClipBoundingBox (context.get()));
 
-        lastClipRectIsValid = true;
-        lastClipRect.setBounds (roundToInt (bounds.origin.x),
-                                roundToInt (flipHeight - (bounds.origin.y + bounds.size.height)),
-                                roundToInt (bounds.size.width),
-                                roundToInt (bounds.size.height));
+        lastClipRect = Rectangle<int> (roundToInt (bounds.origin.x),
+                                       roundToInt (flipHeight - (bounds.origin.y + bounds.size.height)),
+                                       roundToInt (bounds.size.width),
+                                       roundToInt (bounds.size.height));
     }
 
-    return lastClipRect;
+    return *lastClipRect;
 }
 
 bool CoreGraphicsContext::isClipEmpty() const
@@ -376,7 +412,7 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
         CGContextSetTextMatrix (context.get(), state->textMatrix);
 
         stateStack.removeLast (1, false);
-        lastClipRectIsValid = false;
+        lastClipRect.reset();
     }
     else
     {
@@ -498,22 +534,13 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)
     if (state->fillType.isColour())
     {
         flip();
-        applyTransform (transform);
-        createPath (path);
-
-        if (path.isUsingNonZeroWinding())
-            CGContextFillPath (context.get());
-        else
-            CGContextEOFillPath (context.get());
+        setContextClipToPath (path, transform);
+        flip();
+        fillAll();
     }
     else
     {
-        createPath (path, transform);
-
-        if (path.isUsingNonZeroWinding())
-            CGContextClip (context.get());
-        else
-            CGContextEOClip (context.get());
+        setContextClipToPath (path, transform);
 
         if (state->fillType.isGradient())
             drawGradient();
@@ -773,57 +800,6 @@ static CGGradientRef createGradient (const ColourGradient& g, CGColorSpaceRef co
                                      kCGGradientDrawsBeforeStartLocation | kCGGradientDrawsAfterEndLocation);
 }
 
-void CoreGraphicsContext::createPath (const Path& path) const
-{
-    CGContextBeginPath (context.get());
-
-    for (Path::Iterator i (path); i.next();)
-    {
-        switch (i.elementType)
-        {
-            case Path::Iterator::startNewSubPath:  CGContextMoveToPoint (context.get(), i.x1, i.y1); break;
-            case Path::Iterator::lineTo:           CGContextAddLineToPoint (context.get(), i.x1, i.y1); break;
-            case Path::Iterator::quadraticTo:      CGContextAddQuadCurveToPoint (context.get(), i.x1, i.y1, i.x2, i.y2); break;
-            case Path::Iterator::cubicTo:          CGContextAddCurveToPoint (context.get(), i.x1, i.y1, i.x2, i.y2, i.x3, i.y3); break;
-            case Path::Iterator::closePath:        CGContextClosePath (context.get()); break;
-            default:                               jassertfalse; break;
-        }
-    }
-}
-
-void CoreGraphicsContext::createPath (const Path& path, const AffineTransform& transform) const
-{
-    CGContextBeginPath (context.get());
-
-    for (Path::Iterator i (path); i.next();)
-    {
-        switch (i.elementType)
-        {
-        case Path::Iterator::startNewSubPath:
-            transform.transformPoint (i.x1, i.y1);
-            CGContextMoveToPoint (context.get(), i.x1, flipHeight - i.y1);
-            break;
-        case Path::Iterator::lineTo:
-            transform.transformPoint (i.x1, i.y1);
-            CGContextAddLineToPoint (context.get(), i.x1, flipHeight - i.y1);
-            break;
-        case Path::Iterator::quadraticTo:
-            transform.transformPoints (i.x1, i.y1, i.x2, i.y2);
-            CGContextAddQuadCurveToPoint (context.get(), i.x1, flipHeight - i.y1, i.x2, flipHeight - i.y2);
-            break;
-        case Path::Iterator::cubicTo:
-            transform.transformPoints (i.x1, i.y1, i.x2, i.y2, i.x3, i.y3);
-            CGContextAddCurveToPoint (context.get(), i.x1, flipHeight - i.y1, i.x2, flipHeight - i.y2, i.x3, flipHeight - i.y3);
-            break;
-        case Path::Iterator::closePath:
-            CGContextClosePath (context.get()); break;
-        default:
-            jassertfalse;
-            break;
-        }
-    }
-}
-
 void CoreGraphicsContext::flip() const
 {
     CGContextConcatCTM (context.get(), CGAffineTransformMake (1, 0, 0, -1, 0, flipHeight));
@@ -831,14 +807,7 @@ static CGGradientRef createGradient (const ColourGradient& g, CGColorSpaceRef co
 
 void CoreGraphicsContext::applyTransform (const AffineTransform& transform) const
 {
-    CGAffineTransform t;
-    t.a  = transform.mat00;
-    t.b  = transform.mat10;
-    t.c  = transform.mat01;
-    t.d  = transform.mat11;
-    t.tx = transform.mat02;
-    t.ty = transform.mat12;
-    CGContextConcatCTM (context.get(), t);
+    CGContextConcatCTM (context.get(), makeCGAffineTransform (transform));
 }
 
 //==============================================================================
-- 
2.39.2 (Apple Git-143)

