Title: [284888] trunk
Revision
284888
Author
timothy_hor...@apple.com
Date
2021-10-26 12:42:43 -0700 (Tue, 26 Oct 2021)

Log Message

DisplayList::Recorder's clipBounds() becomes empty if a flip is applied to the CTM
https://bugs.webkit.org/show_bug.cgi?id=232134

Reviewed by Darin Adler.

Source/WebCore:

New test: BifurcatedGraphicsContextTests.TransformedClip

* platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::clip):
(WebCore::DisplayList::Recorder::clipPath):
(WebCore::DisplayList::Recorder::clipBounds const):
Instead of updating clipBounds any time the CTM changes, store
clipBounds in base coordinates and map through the CTM when retrieved.

This matches CG's behavior and makes the clipBounds much sturdier.
For example, previously, applying a `scale(1, -1)` to the context
would immediately result in the clipBounds' height becoming negative,
making the bounds empty and confusing anything that reads from it.

(WebCore::DisplayList::Recorder::ContextState::translate):
(WebCore::DisplayList::Recorder::ContextState::rotate):
(WebCore::DisplayList::Recorder::ContextState::scale):
(WebCore::DisplayList::Recorder::ContextState::setCTM):
(WebCore::DisplayList::Recorder::ContextState::concatCTM):
Stop updating the clipBounds when the CTM changes, this is no longer necessary.

* platform/graphics/displaylists/DisplayListRecorderImpl.cpp:
(WebCore::DisplayList::RecorderImpl::extentFromLocalBounds):
Since the clipBounds is now in base space, map the display list
item bounds to base space /before/ intersecting it with clipBounds.

* platform/graphics/displaylists/DisplayListRecorder.h:
Drive-by add a default parameter so getCTM can be called on the subclass the same way it can on GraphicsContext.

* platform/graphics/transforms/AffineTransform.h: Fix a typo.

Tools:

* TestWebKitAPI/Tests/WebCore/cg/BifurcatedGraphicsContextTestsCG.cpp:
(TestWebKitAPI::TEST):
Add a test ensuring that our clipBounds and CG's match through a series
of simple transforms.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (284887 => 284888)


--- trunk/Source/WebCore/ChangeLog	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Source/WebCore/ChangeLog	2021-10-26 19:42:43 UTC (rev 284888)
@@ -1,3 +1,41 @@
+2021-10-26  Tim Horton  <timothy_hor...@apple.com>
+
+        DisplayList::Recorder's clipBounds() becomes empty if a flip is applied to the CTM
+        https://bugs.webkit.org/show_bug.cgi?id=232134
+
+        Reviewed by Darin Adler.
+
+        New test: BifurcatedGraphicsContextTests.TransformedClip
+
+        * platform/graphics/displaylists/DisplayListRecorder.cpp:
+        (WebCore::DisplayList::Recorder::clip):
+        (WebCore::DisplayList::Recorder::clipPath):
+        (WebCore::DisplayList::Recorder::clipBounds const):
+        Instead of updating clipBounds any time the CTM changes, store
+        clipBounds in base coordinates and map through the CTM when retrieved.
+
+        This matches CG's behavior and makes the clipBounds much sturdier.
+        For example, previously, applying a `scale(1, -1)` to the context
+        would immediately result in the clipBounds' height becoming negative,
+        making the bounds empty and confusing anything that reads from it.
+
+        (WebCore::DisplayList::Recorder::ContextState::translate):
+        (WebCore::DisplayList::Recorder::ContextState::rotate):
+        (WebCore::DisplayList::Recorder::ContextState::scale):
+        (WebCore::DisplayList::Recorder::ContextState::setCTM):
+        (WebCore::DisplayList::Recorder::ContextState::concatCTM):
+        Stop updating the clipBounds when the CTM changes, this is no longer necessary.
+
+        * platform/graphics/displaylists/DisplayListRecorderImpl.cpp:
+        (WebCore::DisplayList::RecorderImpl::extentFromLocalBounds):
+        Since the clipBounds is now in base space, map the display list
+        item bounds to base space /before/ intersecting it with clipBounds.
+
+        * platform/graphics/displaylists/DisplayListRecorder.h:
+        Drive-by add a default parameter so getCTM can be called on the subclass the same way it can on GraphicsContext.
+
+        * platform/graphics/transforms/AffineTransform.h: Fix a typo.
+
 2021-10-26  Chris Dumez  <cdu...@apple.com>
 
         <link> elements should be able to fire more than one load / error event

Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp (284887 => 284888)


--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp	2021-10-26 19:42:43 UTC (rev 284888)
@@ -424,7 +424,7 @@
 
 void Recorder::clip(const FloatRect& rect)
 {
-    currentState().clipBounds.intersect(rect);
+    currentState().clipBounds.intersect(currentState().ctm.mapRect(rect));
     recordClip(rect);
 }
 
@@ -440,12 +440,17 @@
 
 void Recorder::clipPath(const Path& path, WindRule windRule)
 {
-    currentState().clipBounds.intersect(path.fastBoundingRect());
+    currentState().clipBounds.intersect(currentState().ctm.mapRect(path.fastBoundingRect()));
     recordClipPath(path, windRule);
 }
 
 IntRect Recorder::clipBounds() const
 {
+    if (auto inverse = currentState().ctm.inverse())
+        return enclosingIntRect(inverse->mapRect(currentState().clipBounds));
+
+    // If the CTM is not invertible, return the original rect.
+    // This matches CGRectApplyInverseAffineTransform behavior.
     return enclosingIntRect(currentState().clipBounds);
 }
 
@@ -521,7 +526,6 @@
 void Recorder::ContextState::translate(float x, float y)
 {
     ctm.translate(x, y);
-    clipBounds.move(-x, -y);
 }
 
 void Recorder::ContextState::rotate(float angleInRadians)
@@ -531,35 +535,21 @@
     
     AffineTransform rotation;
     rotation.rotate(angleInDegrees);
-
-    if (std::optional<AffineTransform> inverse = rotation.inverse())
-        clipBounds = inverse.value().mapRect(clipBounds);
 }
 
 void Recorder::ContextState::scale(const FloatSize& size)
 {
     ctm.scale(size);
-    clipBounds.scale(1 / size.width(), 1 / size.height());
 }
 
 void Recorder::ContextState::setCTM(const AffineTransform& matrix)
 {
-    std::optional<AffineTransform> inverseTransformForClipBounds;
-    if (auto originalCTMInverse = ctm.inverse())
-        inverseTransformForClipBounds = originalCTMInverse->multiply(matrix).inverse();
-
     ctm = matrix;
-
-    if (inverseTransformForClipBounds)
-        clipBounds = inverseTransformForClipBounds->mapRect(clipBounds);
 }
 
 void Recorder::ContextState::concatCTM(const AffineTransform& matrix)
 {
     ctm *= matrix;
-
-    if (std::optional<AffineTransform> inverse = matrix.inverse())
-        clipBounds = inverse.value().mapRect(clipBounds);
 }
 
 } // namespace DisplayList

Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h (284887 => 284888)


--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h	2021-10-26 19:42:43 UTC (rev 284888)
@@ -255,7 +255,7 @@
     WEBCORE_EXPORT void scale(const FloatSize&) final;
     WEBCORE_EXPORT void concatCTM(const AffineTransform&) final;
     WEBCORE_EXPORT void setCTM(const AffineTransform&) final;
-    WEBCORE_EXPORT AffineTransform getCTM(GraphicsContext::IncludeDeviceScale) const final;
+    WEBCORE_EXPORT AffineTransform getCTM(GraphicsContext::IncludeDeviceScale = PossiblyIncludeDeviceScale) const final;
 
     WEBCORE_EXPORT void beginTransparencyLayer(float opacity) final;
     WEBCORE_EXPORT void endTransparencyLayer() final;

Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.cpp (284887 => 284888)


--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.cpp	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.cpp	2021-10-26 19:42:43 UTC (rev 284888)
@@ -458,8 +458,7 @@
         bounds.unite(shadowExtent);
     }
 
-    FloatRect clippedExtent = intersection(state.clipBounds, bounds);
-    return state.ctm.mapRect(clippedExtent);
+    return intersection(state.clipBounds, state.ctm.mapRect(bounds));
 }
 
 } // namespace DisplayList

Modified: trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h (284887 => 284888)


--- trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h	2021-10-26 19:42:43 UTC (rev 284888)
@@ -132,7 +132,7 @@
     WEBCORE_EXPORT double xScale() const;
     WEBCORE_EXPORT double yScale() const;
 
-    bool isInvertible() const; // If you call this this, you're probably doing it wrong.
+    bool isInvertible() const; // If you call this, you're probably doing it wrong.
     WEBCORE_EXPORT std::optional<AffineTransform> inverse() const;
 
     WEBCORE_EXPORT void blend(const AffineTransform& from, double progress, CompositeOperation = CompositeOperation::Replace);

Modified: trunk/Tools/ChangeLog (284887 => 284888)


--- trunk/Tools/ChangeLog	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Tools/ChangeLog	2021-10-26 19:42:43 UTC (rev 284888)
@@ -1,3 +1,15 @@
+2021-10-26  Tim Horton  <timothy_hor...@apple.com>
+
+        DisplayList::Recorder's clipBounds() becomes empty if a flip is applied to the CTM
+        https://bugs.webkit.org/show_bug.cgi?id=232134
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WebCore/cg/BifurcatedGraphicsContextTestsCG.cpp:
+        (TestWebKitAPI::TEST):
+        Add a test ensuring that our clipBounds and CG's match through a series
+        of simple transforms.
+
 2021-10-26  Brady Eidson  <beid...@apple.com>
 
         Add helper classes and messaging infrastructure to launch webpushd and round trip a message to it

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/BifurcatedGraphicsContextTestsCG.cpp (284887 => 284888)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/BifurcatedGraphicsContextTestsCG.cpp	2021-10-26 19:34:30 UTC (rev 284887)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/BifurcatedGraphicsContextTestsCG.cpp	2021-10-26 19:42:43 UTC (rev 284888)
@@ -219,6 +219,65 @@
     EXPECT_EQ(primaryData[3], 255);
 }
 
+TEST(BifurcatedGraphicsContextTests, TransformedClip)
+{
+    auto colorSpace = DestinationColorSpace::SRGB();
+    auto primaryCGContext = adoptCF(CGBitmapContextCreate(nullptr, 100, 100, 8, 4 * 100, colorSpace.platformColorSpace(), kCGImageAlphaPremultipliedLast));
+
+    GraphicsContextCG primaryContextCG(primaryCGContext.get());
+    GraphicsContext& primaryContext = primaryContextCG;
+
+    InMemoryDisplayList displayList;
+    RecorderImpl secondaryContextDL(displayList, { }, FloatRect(0, 0, 100, 100), { });
+    GraphicsContext& secondaryContext = secondaryContextDL;
+
+    BifurcatedGraphicsContext ctx(primaryContext, secondaryContext);
+
+    ctx.clip(FloatRect(25, 25, 50, 50));
+
+    EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+    EXPECT_EQ(primaryContext.clipBounds(), FloatRect(25, 25, 50, 50));
+
+    ctx.scale({ 1, -1 });
+    ctx.translate(0, -100);
+
+    EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+    EXPECT_EQ(primaryContext.clipBounds(), FloatRect(25, 25, 50, 50));
+
+    ctx.scale(2);
+
+    EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+    EXPECT_EQ(primaryContext.clipBounds(), FloatRect(12, 12, 26, 26));
+
+    {
+        GraphicsContextStateSaver saver(ctx);
+
+        ctx.translate(12, 12);
+
+        EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+        EXPECT_EQ(primaryContext.clipBounds(), FloatRect(0, 0, 26, 26));
+
+        ctx.clip(FloatRect(0, 0, 10, 10));
+
+        EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+        EXPECT_EQ(primaryContext.clipBounds(), FloatRect(0, 0, 10, 10));
+
+        ctx.rotate(M_PI / 6);
+
+        EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+        EXPECT_EQ(primaryContext.clipBounds(), FloatRect(0, -5, 14, 14));
+    }
+
+    EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+    EXPECT_EQ(primaryContext.clipBounds(), FloatRect(12, 12, 26, 26));
+
+    // Make the CTM non-invertible.
+    ctx.scale({ 0, 1 });
+
+    EXPECT_EQ(primaryContext.clipBounds(), secondaryContext.clipBounds());
+    EXPECT_EQ(primaryContext.clipBounds(), FloatRect(25, 25, 50, 50));
+}
+
 } // namespace TestWebKitAPI
 
 #endif // USE(CG)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to