Bug 1582645. Only set mLastVisibleRect after we've sent it to WebRender. r=nical
authorJeff Muizelaar <jrmuizel@gmail.com>
Thu, 26 Sep 2019 21:08:29 +0000
changeset 495299 43ac974f69dbfeb53f706953646b2ed2af8183ef
parent 495298 d140c5dfb5797c4fa2ec3a28dc4a135b50297380
child 495300 f8020435c9fd343965d80a040e0f9f78d933142b
push id114134
push userccoroiu@mozilla.com
push dateMon, 30 Sep 2019 09:57:15 +0000
treeherdermozilla-inbound@b19e0c207cfd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1582645
milestone71.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 1582645. Only set mLastVisibleRect after we've sent it to WebRender. r=nical This avoids us setting when we don't send it. e.g. When it's empty. Differential Revision: https://phabricator.services.mozilla.com/D47028
gfx/layers/wr/WebRenderCommandBuilder.cpp
layout/svg/crashtests/crashtests.list
layout/svg/crashtests/invalidation-of-opacity-0.html
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -290,18 +290,22 @@ struct DIGroup {
   //    We'll just need to be careful when iterating.
   //    The advantage of a Vec is that everything stays compact
   //    and we don't need to heap allocate the BlobItemData's
   nsTHashtable<nsPtrHashKey<BlobItemData>> mDisplayItems;
 
   IntRect mInvalidRect;
   nsRect mGroupBounds;
   LayerIntRect mVisibleRect;
+  // This is the last visible rect sent to WebRender. It's used
+  // to compute the invalid rect and ensure that we send
+  // the appropriate data to WebRender for merging.
   LayerIntRect mLastVisibleRect;
-  // this is the intersection of mVisibleRect and mLastVisibleRect
+
+  // This is the intersection of mVisibleRect and mLastVisibleRect
   // we ensure that mInvalidRect is contained in mPreservedRect
   IntRect mPreservedRect;
   int32_t mAppUnitsPerDevPixel;
   gfx::Size mScale;
   ScrollableLayerGuid::ViewID mScrollId;
   LayerPoint mResidualOffset;
   LayerIntRect mLayerBounds; // mGroupBounds converted to Layer space
   // The current bounds of the blob image
@@ -625,16 +629,17 @@ struct DIGroup {
       if (mKey) {
         // Although the contents haven't changed, the visible area *may* have,
         // so request it be updated unconditionally (wr should be able to easily
         // detect if this is a no-op on its side, if that matters)
         aResources.SetBlobImageVisibleArea(
             mKey.value().second(),
             ViewAs<ImagePixel>(mVisibleRect,
                                PixelCastJustification::LayerIsImage));
+        mLastVisibleRect = mVisibleRect;
         PushImage(aBuilder, itemBounds);
       }
       return;
     }
 
     gfx::SurfaceFormat format = gfx::SurfaceFormat::B8G8R8A8;
     std::vector<RefPtr<ScaledFont>> fonts;
     bool validFonts = true;
@@ -736,16 +741,17 @@ struct DIGroup {
         return;
       }
     }
     mFonts = std::move(fonts);
     mInvalidRect.SetEmpty();
     aResources.SetBlobImageVisibleArea(
         mKey.value().second(),
         ViewAs<ImagePixel>(mVisibleRect, PixelCastJustification::LayerIsImage));
+    mLastVisibleRect = mVisibleRect;
     PushImage(aBuilder, itemBounds);
     GP("End EndGroup\n\n");
   }
 
   void PushImage(wr::DisplayListBuilder& aBuilder,
                  const LayoutDeviceRect& bounds) {
     wr::LayoutRect dest = wr::ToLayoutRect(bounds);
     GP("PushImage: %f %f %f %f\n", dest.origin.x, dest.origin.y,
@@ -1519,17 +1525,16 @@ void WebRenderCommandBuilder::DoGrouping
   if (const ActiveScrolledRoot* asr = aWrappingItem->GetActiveScrolledRoot()) {
     scrollId = asr->GetViewId();
   }
 
   g.mAppUnitsPerDevPixel = appUnitsPerDevPixel;
   group.mResidualOffset = residualOffset;
   group.mGroupBounds = groupBounds;
   group.mLayerBounds = layerBounds;
-  group.mLastVisibleRect = group.mVisibleRect;
   group.mVisibleRect = visibleRect;
   group.mPreservedRect = group.mVisibleRect.Intersect(group.mLastVisibleRect).ToUnknownRect();
   group.mAppUnitsPerDevPixel = appUnitsPerDevPixel;
   group.mImageBounds = layerBounds.ToUnknownRect();
   group.mClippedImageBounds = group.mImageBounds;
 
   g.mTransform = Matrix::Scaling(scale.width, scale.height)
                      .PostTranslate(residualOffset.x, residualOffset.y);
--- a/layout/svg/crashtests/crashtests.list
+++ b/layout/svg/crashtests/crashtests.list
@@ -225,8 +225,9 @@ load invalid_url.html
 load 1535517-1.svg
 load 1504072.html
 load 1072758.html
 load 1536892.html
 load 1539318-1.svg
 load 1548985-1.html
 load 1548985-2.svg
 load 1555851.html
+load invalidation-of-opacity-0.html
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/invalidation-of-opacity-0.html
@@ -0,0 +1,26 @@
+<html class="reftest-wait">
+  <script>
+  var i = 0;
+  var opac = [0.3, 0.2, 0, 0.3];
+
+  function f() {
+    document.getElementById("rim").setAttribute("opacity", opac[i]);
+    document.getElementById("circ").setAttribute("r", i + 10);
+    i++;
+    if (i > opac.length) {
+      document.documentElement.className = ""
+    } else {
+      requestAnimationFrame(f);
+    }
+  }
+  onload = () => requestAnimationFrame(f);
+</script>
+
+<body>
+  <svg height="1000" width="1000">
+    <circle cx="50" cy="50" r="40" fill="red" />
+    <g id=rim clip-path="url(#myClip)" opacity=0>
+      <circle id="circ" cx="150" cy="150" r="40" fill="red" />
+    </g>
+    <circle cx="250" cy="250" r="40" fill="red" />
+  </svg>