Title: [280095] trunk
Revision
280095
Author
timothy_hor...@apple.com
Date
2021-07-20 12:30:40 -0700 (Tue, 20 Jul 2021)

Log Message

REGRESSION (r278062): Misplaced text in Wikipedia search field
https://bugs.webkit.org/show_bug.cgi?id=228098
<rdar://problem/80495165>

Reviewed by Simon Fraser.

Source/WebCore:

New test: GraphicsContextTests.DrawNativeImageDoesNotLeakCompositeOperator

* platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::setCGCompositeOperator):
(WebCore::GraphicsContextCG::drawNativeImage):
drawNativeImage uses CGContextStateSaver (not GraphicsContextStateSaver),
so any state changes it made must only be made to the CGContext, not
the WebCore::GraphicsContext.

However, r278062 mistakenly swapped setPlatformCompositeOperation out for
GraphicsContext::setCompositeOperation(), which mutates the state of
*both* contexts, and then (only sometimes!) depends on the state saver
to reset the state (but, of course, as mentioned, it only resets
the CGContext's state).

(WebCore::GraphicsContextCG::updateState):

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebCore/cg/GraphicsContextTestsCG.cpp: Added.
(TestWebKitAPI::greenImage):
(TestWebKitAPI::TEST):
Add a test for the issue described in the WebCore ChangeLog.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (280094 => 280095)


--- trunk/Source/WebCore/ChangeLog	2021-07-20 19:29:56 UTC (rev 280094)
+++ trunk/Source/WebCore/ChangeLog	2021-07-20 19:30:40 UTC (rev 280095)
@@ -1,3 +1,28 @@
+2021-07-20  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r278062): Misplaced text in Wikipedia search field
+        https://bugs.webkit.org/show_bug.cgi?id=228098
+        <rdar://problem/80495165>
+
+        Reviewed by Simon Fraser.
+
+        New test: GraphicsContextTests.DrawNativeImageDoesNotLeakCompositeOperator
+
+        * platform/graphics/cg/GraphicsContextCG.cpp:
+        (WebCore::setCGCompositeOperator):
+        (WebCore::GraphicsContextCG::drawNativeImage):
+        drawNativeImage uses CGContextStateSaver (not GraphicsContextStateSaver),
+        so any state changes it made must only be made to the CGContext, not
+        the WebCore::GraphicsContext.
+
+        However, r278062 mistakenly swapped setPlatformCompositeOperation out for
+        GraphicsContext::setCompositeOperation(), which mutates the state of
+        *both* contexts, and then (only sometimes!) depends on the state saver
+        to reset the state (but, of course, as mentioned, it only resets
+        the CGContext's state).
+
+        (WebCore::GraphicsContextCG::updateState):
+
 2021-07-20  Said Abou-Hallawa  <s...@apple.com>
 
         Make SVGPropertyAnimator::computeCSSPropertyValue() take a reference to SVGElement

Modified: trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp (280094 => 280095)


--- trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp	2021-07-20 19:29:56 UTC (rev 280094)
+++ trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp	2021-07-20 19:30:40 UTC (rev 280095)
@@ -179,6 +179,11 @@
     return kCGBlendModeNormal;
 }
 
+static void setCGBlendMode(CGContextRef context, CompositeOperator op, BlendMode blendMode)
+{
+    CGContextSetBlendMode(context, selectCGBlendMode(op, blendMode));
+}
+
 GraphicsContextCG::GraphicsContextCG(CGContextRef cgContext)
 {
     if (!cgContext)
@@ -334,7 +339,7 @@
 
     auto oldCompositeOperator = compositeOperation();
     auto oldBlendMode = blendModeOperation();
-    setCompositeOperation(options.compositeOperator(), options.blendMode());
+    setCGBlendMode(context, options.compositeOperator(), options.blendMode());
 
     // Make the origin be at adjustedDestRect.location()
     CGContextTranslateCTM(context, adjustedDestRect.x(), adjustedDestRect.y());
@@ -361,7 +366,7 @@
 #if PLATFORM(IOS_FAMILY)
         CGContextSetShouldAntialias(context, wasAntialiased);
 #endif
-        setCompositeOperation(oldCompositeOperator, oldBlendMode);
+        setCGBlendMode(context, oldCompositeOperator, oldBlendMode);
     }
 
     LOG_WITH_STREAM(Images, stream << "GraphicsContextCG::drawNativeImage " << image.get() << " size " << imageSize << " into " << destRect << " took " << (MonotonicTime::now() - startTime).milliseconds() << "ms");
@@ -1095,7 +1100,7 @@
         CGContextSetAlpha(context, state.alpha);
 
     if (flags.containsAny({ GraphicsContextState::CompositeOperationChange, GraphicsContextState::BlendModeChange }))
-        CGContextSetBlendMode(context, selectCGBlendMode(state.compositeOperator, state.blendMode));
+        setCGBlendMode(context, state.compositeOperator, state.blendMode);
 
     if (flags.contains(GraphicsContextState::TextDrawingModeChange))
         CGContextSetTextDrawingMode(context, cgTextDrawingMode(state.textDrawingMode));

Modified: trunk/Tools/ChangeLog (280094 => 280095)


--- trunk/Tools/ChangeLog	2021-07-20 19:29:56 UTC (rev 280094)
+++ trunk/Tools/ChangeLog	2021-07-20 19:30:40 UTC (rev 280095)
@@ -1,3 +1,17 @@
+2021-07-20  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r278062): Misplaced text in Wikipedia search field
+        https://bugs.webkit.org/show_bug.cgi?id=228098
+        <rdar://problem/80495165>
+
+        Reviewed by Simon Fraser.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebCore/cg/GraphicsContextTestsCG.cpp: Added.
+        (TestWebKitAPI::greenImage):
+        (TestWebKitAPI::TEST):
+        Add a test for the issue described in the WebCore ChangeLog.
+
 2021-07-20  Sam Sneddon  <gsnedd...@apple.com>
 
         Change referrer-policy default to strict-origin-when-cross-origin

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (280094 => 280095)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2021-07-20 19:29:56 UTC (rev 280094)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2021-07-20 19:30:40 UTC (rev 280095)
@@ -149,6 +149,7 @@
 		2D00065F1C1F589A0088E6A7 /* WKPDFView.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D00065D1C1F58940088E6A7 /* WKPDFView.mm */; };
 		2D01D06E23218FEE0039AA3A /* WKWebViewPrintFormatter.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D01D06D23218FEE0039AA3A /* WKWebViewPrintFormatter.mm */; };
 		2D08E9372267D0F4002518DA /* ReparentWebViewTimeout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D08E9362267D0F3002518DA /* ReparentWebViewTimeout.mm */; };
+		2D09CF0026A68297009C43C0 /* GraphicsContextTestsCG.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2D09CEFF26A68296009C43C0 /* GraphicsContextTestsCG.cpp */; };
 		2D116E1323E0CB3A00208900 /* iOSMouseSupport.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D116E1223E0CB3900208900 /* iOSMouseSupport.mm */; };
 		2D1646E21D1862CD00015A1A /* DeferredViewInWindowStateChange.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D1646E11D1862CD00015A1A /* DeferredViewInWindowStateChange.mm */; };
 		2D2BEB2D22324E5F005544CA /* RequestTextInputContext.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D2BEB2C22324E5F005544CA /* RequestTextInputContext.mm */; };
@@ -1937,6 +1938,7 @@
 		2D00065D1C1F58940088E6A7 /* WKPDFView.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKPDFView.mm; sourceTree = "<group>"; };
 		2D01D06D23218FEE0039AA3A /* WKWebViewPrintFormatter.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewPrintFormatter.mm; sourceTree = "<group>"; };
 		2D08E9362267D0F3002518DA /* ReparentWebViewTimeout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ReparentWebViewTimeout.mm; sourceTree = "<group>"; };
+		2D09CEFF26A68296009C43C0 /* GraphicsContextTestsCG.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = GraphicsContextTestsCG.cpp; path = cg/GraphicsContextTestsCG.cpp; sourceTree = "<group>"; };
 		2D116E1223E0CB3900208900 /* iOSMouseSupport.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = iOSMouseSupport.mm; sourceTree = "<group>"; };
 		2D1646E11D1862CD00015A1A /* DeferredViewInWindowStateChange.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = DeferredViewInWindowStateChange.mm; path = WebKit/DeferredViewInWindowStateChange.mm; sourceTree = "<group>"; };
 		2D1C04A51D76298B000A6816 /* TestNavigationDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = TestNavigationDelegate.h; path = cocoa/TestNavigationDelegate.h; sourceTree = "<group>"; };
@@ -4974,6 +4976,7 @@
 			children = (
 				2DD87144265F23B4005F997C /* BifurcatedGraphicsContextTestsCG.cpp */,
 				F457275D25578D06007ACA34 /* DisplayListTestsCG.cpp */,
+				2D09CEFF26A68296009C43C0 /* GraphicsContextTestsCG.cpp */,
 			);
 			name = cg;
 			sourceTree = "<group>";
@@ -5539,6 +5542,7 @@
 				465E2807255B2A6A0063A787 /* GPUProcess.mm in Sources */,
 				E394AE6F23F2303E005B4936 /* GrantAccessToMobileAssets.mm in Sources */,
 				E34A6D192412DE390012AB6E /* GrantAccessToPreferencesService.mm in Sources */,
+				2D09CF0026A68297009C43C0 /* GraphicsContextTestsCG.cpp in Sources */,
 				8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */,
 				51EB125924C68592000CB030 /* HIDGamepads.mm in Sources */,
 				7CCE7EFA1A411AE600447C4C /* HitTestResultNodeHandle.cpp in Sources */,

Added: trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/GraphicsContextTestsCG.cpp (0 => 280095)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/GraphicsContextTestsCG.cpp	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/GraphicsContextTestsCG.cpp	2021-07-20 19:30:40 UTC (rev 280095)
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2021 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#if USE(CG)
+
+#include <WebCore/DestinationColorSpace.h>
+#include <WebCore/GraphicsContextCG.h>
+
+namespace TestWebKitAPI {
+using namespace WebCore;
+
+constexpr CGFloat contextWidth = 1;
+constexpr CGFloat contextHeight = 1;
+
+RetainPtr<CGImageRef> greenImage()
+{
+    auto colorSpace = DestinationColorSpace::SRGB();
+    auto cgContext = adoptCF(CGBitmapContextCreate(nullptr, contextWidth, contextHeight, 8, 4 * contextWidth, colorSpace.platformColorSpace(), kCGImageAlphaPremultipliedLast));
+    GraphicsContextCG ctx(cgContext.get());
+    ctx.fillRect(FloatRect(0, 0, contextWidth, contextHeight), Color::green);
+    return adoptCF(CGBitmapContextCreateImage(cgContext.get()));
+}
+
+TEST(GraphicsContextTests, DrawNativeImageDoesNotLeakCompositeOperator)
+{
+    auto colorSpace = DestinationColorSpace::SRGB();
+    auto cgContext = adoptCF(CGBitmapContextCreate(nullptr, contextWidth, contextHeight, 8, 4 * contextWidth, colorSpace.platformColorSpace(), kCGImageAlphaPremultipliedLast));
+    GraphicsContextCG ctx(cgContext.get());
+
+    EXPECT_EQ(ctx.compositeOperation(), CompositeOperator::SourceOver);
+    EXPECT_EQ(ctx.blendModeOperation(), BlendMode::Normal);
+
+    auto greenPixelCG = greenImage();
+    auto greenPixelNative = NativeImage::create(greenPixelCG.get());
+    FloatRect rect(0, 0, contextWidth, contextHeight);
+    FloatRect sourceRect(0, 0, contextWidth / 2, contextHeight / 2);
+    ctx.drawNativeImage(*greenPixelNative, { contextWidth, contextHeight }, rect, sourceRect, { WebCore::CompositeOperator::Copy });
+
+    CGContextSetRGBFillColor(cgContext.get(), 0, 0, 0, 0);
+    CGContextFillRect(cgContext.get(), rect);
+
+    // The context should have one green pixel.
+    CGContextFlush(cgContext.get());
+    uint8_t* primaryData = static_cast<uint8_t*>(CGBitmapContextGetData(cgContext.get()));
+    EXPECT_EQ(primaryData[0], 0);
+    EXPECT_EQ(primaryData[1], 255);
+    EXPECT_EQ(primaryData[2], 0);
+
+    EXPECT_EQ(ctx.compositeOperation(), CompositeOperator::SourceOver);
+    EXPECT_EQ(ctx.blendModeOperation(), BlendMode::Normal);
+}
+
+} // 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