Bug 1625249 - Do a better job of setting hit-test info on blobs. r=jrmuizel
☠☠ backed out by 4631dfa1abbe ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 28 Apr 2020 18:43:13 +0000
changeset 526545 8c02d6099aa347ea174816b0101aae9948707d90
parent 526544 7b3b764a0e4d63788d4bc7b0a24fc45310ed273a
child 526546 5d9701c4fb3b4fe9ccc3f2a052af66aa883bf19b
push id37358
push useropoprus@mozilla.com
push dateWed, 29 Apr 2020 03:05:14 +0000
treeherdermozilla-central@6bb8423186c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1625249
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1625249 - Do a better job of setting hit-test info on blobs. r=jrmuizel In particular this will pick up any hit-test flags set on individual items inside a blob, and ensure the hit-test info emitted to APZ for the blob includes those flags. Differential Revision: https://phabricator.services.mozilla.com/D69205
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/wr/WebRenderCommandBuilder.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -2809,17 +2809,16 @@ APZCTreeManager::HitTestResult APZCTreeM
       wr->HitTest(wr::ToWorldPoint(aHitTestPoint));
 
   Maybe<wr::WrHitResult> chosenResult;
   for (const wr::WrHitResult& result : results) {
     ScrollableLayerGuid guid{result.mLayersId, 0, result.mScrollId};
     APZCTM_LOG("Examining result with guid %s hit info 0x%d... ",
                Stringify(guid).c_str(), result.mHitInfo.serialize());
     if (result.mHitInfo == CompositorHitTestInvisibleToHit) {
-      MOZ_ASSERT(false);
       APZCTM_LOG("skipping due to invisibility.\n");
       continue;
     }
     RefPtr<HitTestingTreeNode> node =
         GetTargetNode(guid, &GuidComparatorIgnoringPresShell);
     if (!node) {
       APZCTM_LOG("no corresponding node found, falling back to root.\n");
       // We can enter here during normal codepaths for cases where the
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -297,28 +297,30 @@ struct DIGroup {
 
   // This is the intersection of mVisibleRect and mLastVisibleRect
   // we ensure that mInvalidRect is contained in mPreservedRect
   IntRect mPreservedRect;
   IntRect mActualBounds;
   int32_t mAppUnitsPerDevPixel;
   gfx::Size mScale;
   ScrollableLayerGuid::ViewID mScrollId;
+  CompositorHitTestInfo mHitInfo;
   LayerPoint mResidualOffset;
   LayerIntRect mLayerBounds;  // mGroupBounds converted to Layer space
   // mLayerBounds clipped to the container/parent of the
   // current item being processed.
   IntRect mClippedImageBounds;  // mLayerBounds with the clipping of any
                                 // containers applied
   Maybe<std::pair<wr::RenderRoot, wr::BlobImageKey>> mKey;
   std::vector<RefPtr<ScaledFont>> mFonts;
 
   DIGroup()
       : mAppUnitsPerDevPixel(0),
-        mScrollId(ScrollableLayerGuid::NULL_SCROLL_ID) {}
+        mScrollId(ScrollableLayerGuid::NULL_SCROLL_ID),
+        mHitInfo(CompositorHitTestInvisibleToHit) {}
 
   void InvalidateRect(const IntRect& aRect) {
     auto r = aRect.Intersect(mPreservedRect);
     // Empty rects get dropped
     if (!r.IsEmpty()) {
       mInvalidRect = mInvalidRect.Union(r);
     }
   }
@@ -662,16 +664,19 @@ struct DIGroup {
     RenderRootStateManager* rootManager =
         aWrManager->GetRenderRootStateManager(aBuilder.GetRenderRoot());
     bool empty = aStartItem == aEndItem;
     if (empty) {
       ClearImageKey(rootManager, true);
       return;
     }
 
+    // Reset mHitInfo, it will get updated inside PaintItemRange
+    mHitInfo = CompositorHitTestInvisibleToHit;
+
     PaintItemRange(aGrouper, aStartItem, aEndItem, context, recorder,
                    rootManager, aResources);
 
     // XXX: set this correctly perhaps using
     // aItem->GetOpaqueRegion(aDisplayListBuilder, &snapped).
     //   Contains(paintBounds);?
     wr::OpacityType opacity = wr::OpacityType::HasAlphaChannel;
 
@@ -736,19 +741,22 @@ struct DIGroup {
     wr::LayoutRect dest = wr::ToLayoutRect(bounds);
     GP("PushImage: %f %f %f %f\n", dest.origin.x, dest.origin.y,
        dest.size.width, dest.size.height);
     gfx::SamplingFilter sampleFilter = gfx::SamplingFilter::
         LINEAR;  // nsLayoutUtils::GetSamplingFilterForFrame(aItem->Frame());
     bool backfaceHidden = false;
 
     // We don't really know the exact shape of this blob because it may contain
-    // SVG shapes so generate an irregular-area hit-test region for it.
-    CompositorHitTestInfo hitInfo(CompositorHitTestFlags::eVisibleToHitTest,
-                                  CompositorHitTestFlags::eIrregularArea);
+    // SVG shapes. Also mHitInfo may be a combination of hit info flags from
+    // different shapes so generate an irregular-area hit-test region for it.
+    CompositorHitTestInfo hitInfo = mHitInfo;
+    if (hitInfo.contains(CompositorHitTestFlags::eVisibleToHitTest)) {
+      hitInfo += CompositorHitTestFlags::eIrregularArea;
+    }
 
     // XXX - clipping the item against the paint rect breaks some content.
     // cf. Bug 1455422.
     // wr::LayoutRect clip = wr::ToLayoutRect(bounds.Intersect(mVisibleRect));
 
     aBuilder.SetHitTestInfo(mScrollId, hitInfo, SideBits::eNone);
     aBuilder.PushImage(dest, dest, !backfaceHidden,
                        wr::ToImageRendering(sampleFilter),
@@ -766,16 +774,29 @@ struct DIGroup {
          item = item->GetAbove()) {
       BlobItemData* data = GetBlobItemData(item);
       IntRect bounds = data->mRect;
       auto bottomRight = bounds.BottomRight();
 
       GP("Trying %s %p-%d %d %d %d %d\n", item->Name(), item->Frame(),
          item->GetPerFrameKey(), bounds.x, bounds.y, bounds.XMost(),
          bounds.YMost());
+
+      if (item->GetType() == DisplayItemType::TYPE_COMPOSITOR_HITTEST_INFO) {
+        // Accumulate the hit-test info flags. In cases where there are multiple
+        // hittest-info display items with different flags, mHitInfo will have
+        // the union of all those flags. If that is the case, we will
+        // additionally set eIrregularArea (at the site that we use mHitInfo)
+        // so that downstream consumers of this (primarily APZ) will know that
+        // the exact shape of what gets hit with what is unknown.
+        mHitInfo +=
+            static_cast<nsDisplayCompositorHitTestInfo*>(item)->HitTestFlags();
+        continue;
+      }
+
       GP("paint check invalid %d %d - %d %d\n", bottomRight.x, bottomRight.y,
          size.width, size.height);
       // skip empty items
       if (bounds.IsEmpty()) {
         continue;
       }
 
       bool dirty = true;
@@ -795,21 +816,16 @@ struct DIGroup {
       nsDisplayList* children = item->GetChildren();
       if (children) {
         GP("doing children in EndGroup\n");
         aGrouper->PaintContainerItem(this, item, data, bounds, children,
                                      aContext, aRecorder, aRootManager,
                                      aResources);
         continue;
       }
-      if (item->GetType() == DisplayItemType::TYPE_COMPOSITOR_HITTEST_INFO) {
-        // Hit test items don't have anything to paint so skip them.
-        // Ideally we would drop these items earlier...
-        continue;
-      }
       nsPaintedDisplayItem* paintedItem = item->AsPaintedDisplayItem();
       if (!paintedItem) {
         continue;
       }
       if (dirty) {
         // What should the clip settting strategy be? We can set the full
         // clip everytime. this is probably easiest for now. An alternative
         // would be to put the push and the pop into separate items and let