Bug 1428993 - Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block r=mattwoodrow
authorMiko Mynttinen <mikokm@gmail.com>
Fri, 12 Jan 2018 18:02:14 +0100
changeset 454247 2c2e56a87ad10a77faf799adf4c2ec55882128dd
parent 454246 2eb0a49e31925e7e4931446f17e869be91d4dcf0
child 454248 be1ce4548b2e1fcb34e91fc7b514394de5eb9ddf
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1428993
milestone59.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 1428993 - Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block r=mattwoodrow MozReview-Commit-ID: FoX9uyoiqj4
layout/base/nsLayoutUtils.cpp
layout/generic/nsFrame.cpp
layout/painting/RetainedDisplayListBuilder.cpp
layout/reftests/display-list/1428993-1-ref.html
layout/reftests/display-list/1428993-1.html
layout/reftests/display-list/1428993-2-ref.html
layout/reftests/display-list/1428993-2.html
layout/reftests/display-list/reftest.list
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -2702,17 +2702,19 @@ nsLayoutUtils::GetTransformToAncestor(ns
   nsIFrame* parent;
   Matrix4x4 ctm;
   if (aFrame == aAncestor) {
     return ctm;
   }
   ctm = aFrame->GetTransformMatrix(aAncestor, &parent, aFlags);
   while (parent && parent != aAncestor &&
     (!(aFlags & nsIFrame::STOP_AT_STACKING_CONTEXT_AND_DISPLAY_PORT) ||
-      (!parent->IsStackingContext() && !FrameHasDisplayPort(parent)))) {
+      (!parent->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW) &&
+       !parent->IsStackingContext() &&
+       !FrameHasDisplayPort(parent)))) {
     if (!parent->Extend3DContext()) {
       ctm.ProjectTo2D();
     }
     ctm = ctm * parent->GetTransformMatrix(aAncestor, &parent, aFlags);
   }
   if (aOutAncestor) {
     *aOutAncestor = parent;
   }
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1014,16 +1014,21 @@ nsIFrame::MarkNeedsDisplayItemRebuild()
 {
   if (!nsLayoutUtils::AreRetainedDisplayListsEnabled() ||
       IsFrameModified() ||
       HasAnyStateBits(NS_FRAME_IN_POPUP)) {
     // Skip frames that are already marked modified.
     return;
   }
 
+  if (Type() == LayoutFrameType::Placeholder) {
+    // Do not mark placeholder frames modified.
+    return;
+  }
+
   nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(this);
   MOZ_ASSERT(displayRoot);
 
   RetainedDisplayListBuilder* retainedBuilder =
     displayRoot->GetProperty(RetainedDisplayListBuilder::Cached());
 
   if (!retainedBuilder) {
     return;
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
 #include "RetainedDisplayListBuilder.h"
+#include "nsPlaceholderFrame.h"
 #include "nsSubDocumentFrame.h"
 #include "nsViewManager.h"
 
 /**
  * Code for doing display list building for a modified subset of the window,
  * and then merging it into the existing display list (for the full window).
  *
  * The approach primarily hinges on the observation that the ‘true’ ordering of
@@ -619,32 +620,66 @@ HandlePreserve3D(nsIFrame* aFrame, nsRec
 
   return aFrame;
 }
 
 static void
 ProcessFrame(nsIFrame* aFrame, nsDisplayListBuilder& aBuilder,
              AnimatedGeometryRoot** aAGR, nsRect& aOverflow,
              nsIFrame* aStopAtFrame, nsTArray<nsIFrame*>& aOutFramesWithProps,
-             const bool /* aStopAtStackingContext */)
+             const bool aStopAtStackingContext)
 {
   nsIFrame* currentFrame = aFrame;
 
   while (currentFrame != aStopAtFrame) {
     currentFrame = HandlePreserve3D(currentFrame, aOverflow);
 
     // Convert 'aOverflow' into the coordinate space of the nearest stacking context
     // or display port ancestor and update 'currentFrame' to point to that frame.
-    aOverflow = nsLayoutUtils::TransformFrameRectToAncestor(currentFrame, aOverflow,
-                                                            aStopAtFrame,
-                                                            nullptr, nullptr,
-                                                            /* aStopAtStackingContextAndDisplayPort = */ true,
-                                                            &currentFrame);
+    nsIFrame* previousFrame = currentFrame;
+    aOverflow = nsLayoutUtils::TransformFrameRectToAncestor(currentFrame, aOverflow, aStopAtFrame,
+                                                           nullptr, nullptr,
+                                                           /* aStopAtStackingContextAndDisplayPortAndOOFFrame = */ true,
+                                                           &currentFrame);
     MOZ_ASSERT(currentFrame);
 
+    // If the current frame is an OOF frame, DisplayListBuildingData needs to be
+    // set on all the ancestor stacking contexts of the  placeholder frame, up
+    // to the containing block of the OOF frame. This is done to ensure that the
+    // content that might be behind the OOF frame is built for merging.
+    nsIFrame* placeholder = previousFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)
+                          ? previousFrame->GetPlaceholderFrame()
+                          : nullptr;
+
+    if (placeholder) {
+      nsRect placeholderOverflow =
+        aOverflow + previousFrame->GetOffsetTo(placeholder);
+
+      CRR_LOG("Processing placeholder %p for OOF frame %p\n",
+              placeholder, previousFrame);
+
+      CRR_LOG("OOF frame draw area: %d %d %d %d\n",
+              placeholderOverflow.x, placeholderOverflow.y,
+              placeholderOverflow.width, placeholderOverflow.height);
+
+      // Tracking AGRs for the placeholder processing is not necessary, as the
+      // goal is to only modify the DisplayListBuildingData rect.
+      AnimatedGeometryRoot* dummyAGR = nullptr;
+
+      // Find a common ancestor frame to handle frame continuations.
+      // TODO: It might be possible to write a more specific and efficient
+      // function for this.
+      nsIFrame* ancestor =
+        nsLayoutUtils::FindNearestCommonAncestorFrame(previousFrame->GetParent(),
+                                                      placeholder->GetParent());
+
+      ProcessFrame(placeholder, aBuilder, &dummyAGR, placeholderOverflow,
+                   ancestor, aOutFramesWithProps, false);
+    }
+
     if (nsLayoutUtils::FrameHasDisplayPort(currentFrame)) {
       CRR_LOG("Frame belongs to displayport frame %p\n", currentFrame);
       nsIScrollableFrame* sf = do_QueryFrame(currentFrame);
       MOZ_ASSERT(sf);
       nsRect displayPort;
       DebugOnly<bool> hasDisplayPort =
         nsLayoutUtils::GetDisplayPort(currentFrame->GetContent(), &displayPort,
                                       RelativeTo::ScrollPort);
@@ -657,17 +692,18 @@ ProcessFrame(nsIFrame* aFrame, nsDisplay
           currentFrame->GetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
         if (!rect) {
           rect = new nsRect();
           currentFrame->SetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect(), rect);
           currentFrame->SetHasOverrideDirtyRegion(true);
           aOutFramesWithProps.AppendElement(currentFrame);
         }
         rect->UnionRect(*rect, r);
-        CRR_LOG("Adding area to displayport draw area: %d %d %d %d\n", r.x, r.y, r.width, r.height);
+        CRR_LOG("Adding area to displayport draw area: %d %d %d %d\n",
+                r.x, r.y, r.width, r.height);
 
         // TODO: Can we just use MarkFrameForDisplayIfVisible, plus MarkFramesForDifferentAGR to
         // ensure that this displayport, plus any items that move relative to it get rebuilt,
         // and then not contribute to the root dirty area?
         aOverflow = sf->GetScrollPortRect();
       } else {
         // Don't contribute to the root dirty area at all.
         aOverflow.SetEmpty();
@@ -675,50 +711,60 @@ ProcessFrame(nsIFrame* aFrame, nsDisplay
       }
     }
 
     if (currentFrame->IsStackingContext()) {
       CRR_LOG("Frame belongs to stacking context frame %p\n", currentFrame);
       // If we found an intermediate stacking context with an existing display item
       // then we can store the dirty rect there and stop. If we couldn't find one then
       // we need to keep bubbling up to the next stacking context.
-      if (currentFrame != aBuilder.RootReferenceFrame() &&
-          currentFrame->HasDisplayItems()) {
-        aBuilder.MarkFrameForDisplayIfVisible(currentFrame,
-                                              aBuilder.RootReferenceFrame());
+      if (currentFrame == aBuilder.RootReferenceFrame() ||
+          !currentFrame->HasDisplayItems()) {
+        continue;
+      }
+
+      aBuilder.MarkFrameForDisplayIfVisible(currentFrame,
+                                            aBuilder.RootReferenceFrame());
 
-        // Store the stacking context relative dirty area such
-        // that display list building will pick it up when it
-        // gets to it.
-        nsDisplayListBuilder::DisplayListBuildingData* data =
-          currentFrame->GetProperty(nsDisplayListBuilder::DisplayListBuildingRect());
-        if (!data) {
-          data = new nsDisplayListBuilder::DisplayListBuildingData;
-          currentFrame->SetProperty(nsDisplayListBuilder::DisplayListBuildingRect(), data);
-          currentFrame->SetHasOverrideDirtyRegion(true);
-          aOutFramesWithProps.AppendElement(currentFrame);
-        }
-        data->mDirtyRect.UnionRect(data->mDirtyRect, aOverflow);
-        CRR_LOG("Adding area to stacking context draw area: %d %d %d %d\n",
-                aOverflow.x, aOverflow.y, aOverflow.width, aOverflow.height);
-        if (!data->mModifiedAGR) {
-          data->mModifiedAGR = *aAGR;
-        } else if (data->mModifiedAGR != *aAGR) {
-          data->mDirtyRect = currentFrame->GetVisualOverflowRectRelativeToSelf();
-          CRR_LOG("Found multiple modified AGRs within this stacking context, giving up\n");
-        }
+      // Store the stacking context relative dirty area such
+      // that display list building will pick it up when it
+      // gets to it.
+      nsDisplayListBuilder::DisplayListBuildingData* data =
+        currentFrame->GetProperty(nsDisplayListBuilder::DisplayListBuildingRect());
+      if (!data) {
+        data = new nsDisplayListBuilder::DisplayListBuildingData();
+        currentFrame->SetProperty(nsDisplayListBuilder::DisplayListBuildingRect(), data);
+        currentFrame->SetHasOverrideDirtyRegion(true);
+        aOutFramesWithProps.AppendElement(currentFrame);
+      }
+      CRR_LOG("Adding area to stacking context draw area: %d %d %d %d\n",
+              aOverflow.x, aOverflow.y, aOverflow.width, aOverflow.height);
+      data->mDirtyRect.UnionRect(data->mDirtyRect, aOverflow);
 
-        // Don't contribute to the root dirty area at all.
-        *aAGR = nullptr;
-        aOverflow.SetEmpty();
-        break;
+      if (!aStopAtStackingContext) {
+        // Continue ascending the frame tree until we reach aStopAtFrame.
+        continue;
       }
+
+      if (!data->mModifiedAGR) {
+        data->mModifiedAGR = *aAGR;
+      } else if (data->mModifiedAGR != *aAGR) {
+        data->mDirtyRect = currentFrame->GetVisualOverflowRectRelativeToSelf();
+        CRR_LOG("Found multiple modified AGRs within this stacking context, giving up\n");
+      }
+
+      // Don't contribute to the root dirty area at all.
+      aOverflow.SetEmpty();
+      *aAGR = nullptr;
+
+      break;
     }
   }
 }
+
 /**
  * Given a list of frames that has been modified, computes the region that we need to
  * do display list building for in order to build all modified display items.
  *
  * When a modified frame is within a stacking context (with an existing display item),
  * then we only contribute to the build area within the stacking context, as well as forcing
  * display list building to descend to the stacking context. We don't need to add build
  * area outside of the stacking context (and force items above/below the stacking context
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1428993-1-ref.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<meta charset="utf-8">
+<title>Retained display list test</title>
+<style type="text/css">
+.container {
+    position: fixed;
+    top: 0px;
+    left: 0px;
+
+    width: 400px;
+    height: 400px;
+
+    background-color: green;
+}
+</style>
+</head>
+
+<body>
+<div class="container">
+</div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1428993-1.html
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<meta charset="utf-8">
+<title>Retained display list test</title>
+<style type="text/css">
+.box {
+    left: 0px;
+    top: 0px;
+    width: 400px;
+    height: 400px;
+}
+
+button {
+    position: fixed;
+    outline: none;
+    background-color: green;
+    border: none;
+}
+
+.red {
+    position: absolute;
+    background-color: red;
+}
+
+.translate {
+    transform: translateX(0px);
+}
+
+.container {
+    position: absolute;
+    top: 0px;
+    left: 0px;
+    z-index: 1;
+}
+</style>
+</head>
+
+<body>
+<div class="container">
+    <div class="box red"></div>
+    <button class="box" id="green"></button>
+</div>
+
+<script type="text/javascript">
+function doTest() {
+  document.getElementById("green").classList.add("translate");
+  document.documentElement.removeAttribute("class");
+}
+
+window.addEventListener("MozReftestInvalidate", doTest);
+
+// setTimeout(doTest, 5000);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1428993-2-ref.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<meta charset="utf-8">
+<title>Retained display list test</title>
+<style type="text/css">
+#child {
+    position: absolute;
+    width: 100px;
+    height: 100px;
+    top: 300px;
+    left: 0px;
+    background-color: green;
+}
+
+.container {
+    position: absolute;
+    top: 0px;
+    left: 0px;
+    width: 200px;
+    height: 200px;
+    z-index: 1;
+    background-color: green;
+}
+</style>
+</head>
+
+<body>
+<div class="container">
+    <div id="child"></div>
+</div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1428993-2.html
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<meta charset="utf-8">
+<title>Retained display list test</title>
+<style type="text/css">
+.back {
+    width: 200px;
+    height: 200px;
+    background-color: red;
+}
+
+#parent {
+    position: fixed;
+    width: 200px;
+    height: 200px;
+    top: 0px;
+    left: 0px;
+    background-color: green;
+}
+
+#child {
+    position: fixed;
+    width: 100px;
+    height: 100px;
+    top: 300px;
+    background-color: green;
+}
+
+.translate {
+    transform: translateX(0px);
+}
+
+.container {
+    position: absolute;
+    top: 0px;
+    left: 0px;
+    z-index: 1;
+}
+</style>
+</head>
+
+<body>
+<div class="container">
+    <div class="back"></div>
+    <div id="parent">
+        <div id="child"></div>
+    </div>
+</div>
+<script type="text/javascript">
+function doTest() {
+  document.getElementById("parent").classList.add("translate");
+  document.documentElement.removeAttribute("class");
+}
+
+window.addEventListener("MozReftestInvalidate", doTest);
+
+// setTimeout(doTest, 5000);
+</script>
+</body>
+</html>
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -7,8 +7,10 @@ skip-if(!retainedDisplayList) == retaine
 skip-if(!retainedDisplayList) == retained-dl-scroll-out-of-view-1.html retained-dl-scroll-out-of-view-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-displayport-1.html retained-dl-displayport-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-prerender-transform-1.html retained-dl-prerender-transform-1-ref.html
 == retained-dl-wrap-list.html retained-dl-wrap-list-ref.html
 fuzzy(1,235200) == 1413073.html 1413073-ref.html
 == 1416291.html 1416291-ref.html
 == 1417601-1.html 1417601-1-ref.html
 == 1418945-1.html 1418945-1-ref.html
+skip-if(Android) == 1428993-1.html 1428993-1-ref.html
+== 1428993-2.html 1428993-2-ref.html