Use Region for event region even when it is a rectangle
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 20:06:04 +0000 (20:06 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 20:06:04 +0000 (20:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195803

Reviewed by Simon Fraser.

Source/WebCore:

Region type is now optimized for the common single-rectangle case so we can simplify code.

* platform/graphics/GraphicsLayer.cpp:
(WebCore::GraphicsLayer::setEventRegion):
* platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::eventRegion const):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::setEventRegion):
* platform/graphics/ca/GraphicsLayerCA.h:
* platform/graphics/ca/PlatformCALayer.h:
* platform/graphics/ca/cocoa/PlatformCALayerCocoa.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::paintIntoLayer):

Source/WebKit:

* Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyProperties):
* Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
* Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTransaction::LayerProperties::LayerProperties):
(WebKit::RemoteLayerTreeTransaction::LayerProperties::encode const):
(WebKit::RemoteLayerTreeTransaction::LayerProperties::decode):
* UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
(WebKit::RemoteLayerTreeNode::eventRegion const):
* UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm:
(WebKit::RemoteLayerTreeNode::setEventRegion):
* UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(WebKit::collectDescendantViewsAtPoint):
* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
(WebKit::PlatformCALayerRemote::setEventRegion):
(WebKit::PlatformCALayerRemote::eventRegion const): Deleted.
* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243007 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsLayer.cpp
Source/WebCore/platform/graphics/GraphicsLayer.h
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
Source/WebCore/platform/graphics/ca/PlatformCALayer.h
Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.h
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.h
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebKit/ChangeLog
Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm
Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h
Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h

index 56328f8..e85fc16 100644 (file)
@@ -1,3 +1,24 @@
+2019-03-15  Antti Koivisto  <antti@apple.com>
+
+        Use Region for event region even when it is a rectangle
+        https://bugs.webkit.org/show_bug.cgi?id=195803
+
+        Reviewed by Simon Fraser.
+
+        Region type is now optimized for the common single-rectangle case so we can simplify code.
+
+        * platform/graphics/GraphicsLayer.cpp:
+        (WebCore::GraphicsLayer::setEventRegion):
+        * platform/graphics/GraphicsLayer.h:
+        (WebCore::GraphicsLayer::eventRegion const):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::setEventRegion):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        * platform/graphics/ca/PlatformCALayer.h:
+        * platform/graphics/ca/cocoa/PlatformCALayerCocoa.h:
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::paintIntoLayer):
+
 2019-03-15  Jer Noble  <jer.noble@apple.com>
 
         Unreviewed unified build fix: GPUBindGroup has a default public constructor and destructor, so all its member
index f39f371..00c24ba 100644 (file)
@@ -31,7 +31,6 @@
 #include "FloatRect.h"
 #include "GraphicsContext.h"
 #include "LayoutRect.h"
-#include "Region.h"
 #include "RotateTransformOperation.h"
 #include <wtf/HashMap.h>
 #include <wtf/NeverDestroyed.h>
@@ -412,9 +411,9 @@ void GraphicsLayer::setShapeLayerWindRule(WindRule windRule)
 #endif
 }
 
-void GraphicsLayer::setEventRegion(std::unique_ptr<Region>&& eventRegion)
+void GraphicsLayer::setEventRegion(Region&& eventRegion)
 {
-    m_eventRegion = WTFMove(eventRegion);
+    m_eventRegion = eventRegion;
 }
 
 void GraphicsLayer::noteDeviceOrPageScaleFactorChangedIncludingDescendants()
index 94a1956..c84d75f 100644 (file)
@@ -35,6 +35,7 @@
 #include "GraphicsLayerClient.h"
 #include "Path.h"
 #include "PlatformLayer.h"
+#include "Region.h"
 #include "ScrollableArea.h"
 #include "TransformOperations.h"
 #include "WindRule.h"
@@ -54,7 +55,6 @@ namespace WebCore {
 class GraphicsContext;
 class GraphicsLayerFactory;
 class Image;
-class Region;
 class TiledBacking;
 class TimingFunction;
 class TransformationMatrix;
@@ -454,8 +454,8 @@ public:
     virtual void setShapeLayerWindRule(WindRule);
 
     // Non-null if the event sensitive region of the layer differs from the layer bounds.
-    const Region* eventRegion() const { return m_eventRegion.get(); }
-    virtual void setEventRegion(std::unique_ptr<Region>&&);
+    const Region& eventRegion() const { return m_eventRegion; }
+    virtual void setEventRegion(Region&&);
 
     // Transitions are identified by a special animation name that cannot clash with a keyframe identifier.
     static String animationNameForTransition(AnimatedPropertyID);
@@ -731,7 +731,7 @@ protected:
     FloatRoundedRect m_backdropFiltersRect;
     Optional<FloatRect> m_animationExtent;
 
-    std::unique_ptr<Region> m_eventRegion;
+    Region m_eventRegion;
 
 #if USE(CA)
     WindRule m_shapeLayerWindRule { WindRule::NonZero };
index d378fc4..5f6744e 100644 (file)
@@ -984,9 +984,9 @@ void GraphicsLayerCA::setShapeLayerWindRule(WindRule windRule)
     noteLayerPropertyChanged(WindRuleChanged);
 }
 
-void GraphicsLayerCA::setEventRegion(std::unique_ptr<Region>&& eventRegion)
+void GraphicsLayerCA::setEventRegion(Region&& eventRegion)
 {
-    if (arePointingToEqualData(eventRegion, m_eventRegion))
+    if (eventRegion == m_eventRegion)
         return;
 
     GraphicsLayer::setEventRegion(WTFMove(eventRegion));
index 8886b37..dff55c2 100644 (file)
@@ -124,7 +124,7 @@ public:
     WEBCORE_EXPORT void setShapeLayerPath(const Path&) override;
     WEBCORE_EXPORT void setShapeLayerWindRule(WindRule) override;
 
-    WEBCORE_EXPORT void setEventRegion(std::unique_ptr<Region>&&) override;
+    WEBCORE_EXPORT void setEventRegion(Region&&) override;
 
     WEBCORE_EXPORT void suspendAnimations(MonotonicTime) override;
     WEBCORE_EXPORT void resumeAnimations() override;
index 3486c1d..04cb53c 100644 (file)
@@ -234,8 +234,7 @@ public:
     virtual WindRule shapeWindRule() const = 0;
     virtual void setShapeWindRule(WindRule) = 0;
 
-    virtual const Region* eventRegion() const = 0;
-    virtual void setEventRegion(const Region*) = 0;
+    virtual void setEventRegion(const Region&) = 0;
     
     virtual GraphicsLayer::CustomAppearance customAppearance() const = 0;
     virtual void updateCustomAppearance(GraphicsLayer::CustomAppearance) = 0;
index fd2dded..8fba4c5 100644 (file)
@@ -168,8 +168,7 @@ public:
     GraphicsLayer::CustomAppearance customAppearance() const override { return m_customAppearance; }
     void updateCustomAppearance(GraphicsLayer::CustomAppearance) override;
 
-    const Region* eventRegion() const override { return nullptr; }
-    void setEventRegion(const Region*) override { }
+    void setEventRegion(const Region&) override { }
 
     GraphicsLayer::EmbeddedViewID embeddedViewID() const override;
 
index f09f7a9..4b1d426 100644 (file)
@@ -157,8 +157,7 @@ public:
     GraphicsLayer::CustomAppearance customAppearance() const override { return m_customAppearance; }
     void updateCustomAppearance(GraphicsLayer::CustomAppearance customAppearance) override { m_customAppearance = customAppearance; }
 
-    const Region* eventRegion() const override { return nullptr; }
-    void setEventRegion(const Region*) override { }
+    void setEventRegion(const Region&) override { }
 
     GraphicsLayer::EmbeddedViewID embeddedViewID() const override;
 
index f45103c..5e0d752 100644 (file)
@@ -2586,20 +2586,15 @@ void RenderLayerBacking::paintIntoLayer(const GraphicsLayer* graphicsLayer, Grap
     RenderLayer::LayerPaintingInfo paintingInfo(&m_owningLayer, paintDirtyRect, paintBehavior, -m_subpixelOffsetFromRenderer);
 
 #if PLATFORM(IOS_FAMILY)
-    auto eventRegion = std::make_unique<Region>();
-    paintingInfo.eventRegion = eventRegion.get();
+    Region eventRegion;
+    paintingInfo.eventRegion = &eventRegion;
 #endif
 
     m_owningLayer.paintLayerContents(context, paintingInfo, paintFlags);
 
 #if PLATFORM(IOS_FAMILY)
     paintingInfo.eventRegion = nullptr;
-    // Use null event region to indicate the entire layer is sensitive to events (the common case).
-    // FIXME: We could optimize Region so it doesn't use lots of memory if it contains a single rect only.
-    if (eventRegion->contains(roundedIntRect(compositedBounds())))
-        eventRegion = nullptr;
-    else
-        eventRegion->translate(roundedIntSize(contentOffsetInCompositingLayer()));
+    eventRegion.translate(roundedIntSize(contentOffsetInCompositingLayer()));
     m_graphicsLayer->setEventRegion(WTFMove(eventRegion));
 #endif
 
index dbcd749..72474c0 100644 (file)
@@ -1,3 +1,28 @@
+2019-03-15  Antti Koivisto  <antti@apple.com>
+
+        Use Region for event region even when it is a rectangle
+        https://bugs.webkit.org/show_bug.cgi?id=195803
+
+        Reviewed by Simon Fraser.
+
+        * Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
+        (WebKit::RemoteLayerTreePropertyApplier::applyProperties):
+        * Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
+        * Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm:
+        (WebKit::RemoteLayerTreeTransaction::LayerProperties::LayerProperties):
+        (WebKit::RemoteLayerTreeTransaction::LayerProperties::encode const):
+        (WebKit::RemoteLayerTreeTransaction::LayerProperties::decode):
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
+        (WebKit::RemoteLayerTreeNode::eventRegion const):
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm:
+        (WebKit::RemoteLayerTreeNode::setEventRegion):
+        * UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
+        (WebKit::collectDescendantViewsAtPoint):
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
+        (WebKit::PlatformCALayerRemote::setEventRegion):
+        (WebKit::PlatformCALayerRemote::eventRegion const): Deleted.
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
+
 2019-03-15  Shawn Roberts  <sroberts@apple.com>
 
         Unreviewed, rolling out r242952.
index 0a79977..cafa5f6 100644 (file)
@@ -262,7 +262,7 @@ void RemoteLayerTreePropertyApplier::applyProperties(RemoteLayerTreeNode& node,
     updateMask(node, properties, relatedLayers);
 
     if (properties.changedProperties & RemoteLayerTreeTransaction::EventRegionChanged)
-        node.setEventRegion(properties.eventRegion ? std::make_unique<WebCore::Region>(*properties.eventRegion) : nullptr);
+        node.setEventRegion(properties.eventRegion);
 
 #if PLATFORM(IOS_FAMILY)
     applyPropertiesToUIView(node.uiView(), properties, relatedLayers);
index 2d37ecc..c32696e 100644 (file)
@@ -172,7 +172,7 @@ public:
         bool opaque;
         bool contentsHidden;
         bool userInteractionEnabled;
-        std::unique_ptr<WebCore::Region> eventRegion;
+        WebCore::Region eventRegion;
     };
 
     explicit RemoteLayerTreeTransaction();
index 0e40244..78edd18 100644 (file)
@@ -143,6 +143,7 @@ RemoteLayerTreeTransaction::LayerProperties::LayerProperties(const LayerProperti
     , opaque(other.opaque)
     , contentsHidden(other.contentsHidden)
     , userInteractionEnabled(other.userInteractionEnabled)
+    , eventRegion(other.eventRegion)
 {
     // FIXME: LayerProperties should reference backing store by ID, so that two layers can have the same backing store (for clones).
     // FIXME: LayerProperties shouldn't be copyable; PlatformCALayerRemote::clone should copy the relevant properties.
@@ -155,10 +156,6 @@ RemoteLayerTreeTransaction::LayerProperties::LayerProperties(const LayerProperti
 
     if (other.filters)
         filters = std::make_unique<WebCore::FilterOperations>(*other.filters);
-
-    if (other.eventRegion)
-        eventRegion = std::make_unique<WebCore::Region>(*other.eventRegion);
-
 }
 
 void RemoteLayerTreeTransaction::LayerProperties::encode(IPC::Encoder& encoder) const
@@ -282,11 +279,8 @@ void RemoteLayerTreeTransaction::LayerProperties::encode(IPC::Encoder& encoder)
     if (changedProperties & UserInteractionEnabledChanged)
         encoder << userInteractionEnabled;
 
-    if (changedProperties & EventRegionChanged) {
-        encoder << !!eventRegion;
-        if (eventRegion)
-            encoder << *eventRegion;
-    }
+    if (changedProperties & EventRegionChanged)
+        encoder << eventRegion;
 }
 
 bool RemoteLayerTreeTransaction::LayerProperties::decode(IPC::Decoder& decoder, LayerProperties& result)
@@ -511,16 +505,11 @@ bool RemoteLayerTreeTransaction::LayerProperties::decode(IPC::Decoder& decoder,
     }
 
     if (result.changedProperties & EventRegionChanged) {
-        bool hasEventRegion = false;
-        if (!decoder.decode(hasEventRegion))
+        Optional<WebCore::Region> eventRegion;
+        decoder >> eventRegion;
+        if (!eventRegion)
             return false;
-        if (hasEventRegion) {
-            Optional<WebCore::Region> eventRegion;
-            decoder >> eventRegion;
-            if (!eventRegion)
-                return false;
-            result.eventRegion = std::make_unique<Region>(WTFMove(*eventRegion));
-        }
+        result.eventRegion = WTFMove(*eventRegion);
     }
 
     return true;
index c705aac..5380481 100644 (file)
@@ -55,8 +55,8 @@ public:
 
     WebCore::GraphicsLayer::PlatformLayerID layerID() const { return m_layerID; }
 
-    const WebCore::Region* eventRegion() const { return m_eventRegion.get(); }
-    void setEventRegion(std::unique_ptr<WebCore::Region>&&);
+    const WebCore::Region& eventRegion() const { return m_eventRegion; }
+    void setEventRegion(const WebCore::Region&);
 
     void detachFromParent();
 
@@ -75,7 +75,7 @@ private:
     RetainPtr<UIView> m_uiView;
 #endif
 
-    std::unique_ptr<WebCore::Region> m_eventRegion;
+    WebCore::Region m_eventRegion;
 };
 
 }
index 22cdaa1..9200a68 100644 (file)
@@ -87,9 +87,9 @@ void RemoteLayerTreeNode::detachFromParent()
     [layer() removeFromSuperlayer];
 }
 
-void RemoteLayerTreeNode::setEventRegion(std::unique_ptr<WebCore::Region>&& eventRegion)
+void RemoteLayerTreeNode::setEventRegion(const WebCore::Region& eventRegion)
 {
-    m_eventRegion = WTFMove(eventRegion);
+    m_eventRegion = eventRegion;
 }
 
 void RemoteLayerTreeNode::initializeLayer()
index 20d3691..9a4f34e 100644 (file)
@@ -46,23 +46,19 @@ static void collectDescendantViewsAtPoint(Vector<UIView *, 16>& viewsAtPoint, UI
     for (UIView *view in [parent subviews]) {
         CGPoint subviewPoint = [view convertPoint:point fromView:parent];
 
-        // FIXME: This doesn't cover all possible cases yet.
-        auto isTransparent = [&] {
-            if ([view isKindOfClass:[WKTiledBackingView class]])
+        auto handlesEvent = [&] {
+            if (![view pointInside:subviewPoint withEvent:event])
                 return false;
+            if ([view isKindOfClass:[WKTiledBackingView class]])
+                return true;
             if (![view isKindOfClass:[WKCompositingView class]])
-                return false;
-            if (view.layer.contents)
-                return false;
-            return true;
+                return true;
+            auto* node = RemoteLayerTreeNode::forCALayer(view.layer);
+            return node->eventRegion().contains(WebCore::IntPoint(subviewPoint));
         }();
 
-        if (!isTransparent && [view pointInside:subviewPoint withEvent:event]) {
-            auto* node = RemoteLayerTreeNode::forCALayer(view.layer);
-            auto* eventRegion = node ? node->eventRegion() : nullptr;
-            if (!eventRegion || eventRegion->contains(WebCore::IntPoint(subviewPoint)))
-                viewsAtPoint.append(view);
-        }
+        if (handlesEvent)
+            viewsAtPoint.append(view);
 
         if (![view subviews])
             return;
index 90e1f87..157061d 100644 (file)
@@ -871,18 +871,12 @@ void PlatformCALayerRemote::updateCustomAppearance(GraphicsLayer::CustomAppearan
     m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::CustomAppearanceChanged);
 }
 
-const Region* PlatformCALayerRemote::eventRegion() const
+void PlatformCALayerRemote::setEventRegion(const WebCore::Region& eventRegion)
 {
-    return m_properties.eventRegion.get();
-}
-
-void PlatformCALayerRemote::setEventRegion(const WebCore::Region* eventRegion)
-{
-    const auto* oldEventRegion = m_properties.eventRegion.get();
-    if (arePointingToEqualData(oldEventRegion, eventRegion))
+    if (m_properties.eventRegion == eventRegion)
         return;
 
-    m_properties.eventRegion = eventRegion ? std::make_unique<WebCore::Region>(*eventRegion) : nullptr;
+    m_properties.eventRegion = eventRegion;
     m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::EventRegionChanged);
 }
 
index 2172f0d..d15ab63 100644 (file)
@@ -174,8 +174,7 @@ public:
     WebCore::GraphicsLayer::CustomAppearance customAppearance() const override;
     void updateCustomAppearance(WebCore::GraphicsLayer::CustomAppearance) override;
 
-    const WebCore::Region* eventRegion() const override;
-    void setEventRegion(const WebCore::Region*) override;
+    void setEventRegion(const WebCore::Region&) override;
 
     WebCore::GraphicsLayer::EmbeddedViewID embeddedViewID() const override;