Bug 1477831 - Merge items in FLBDisplayItemIterator draft
authorMiko Mynttinen <mikokm@gmail.com>
Tue, 24 Jul 2018 16:31:04 +0200
changeset 822236 fab9ed1279603f9f7733d51336653a2a96686384
parent 821749 fe48e26ca88c7919f0c075dc01d5f9fdccdb1260
child 822785 4e6387f3915acccda37c93b9d410b98d86d63018
push id117317
push userbmo:mikokm@gmail.com
push dateTue, 24 Jul 2018 22:25:49 +0000
bugs1477831
milestone63.0a1
Bug 1477831 - Merge items in FLBDisplayItemIterator MozReview-Commit-ID: AWxVue3tjN1
layout/painting/FrameLayerBuilder.cpp
layout/painting/crashtests/1477831-1.html
layout/painting/crashtests/crashtests.list
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -213,25 +213,17 @@ public:
       mMarkers.pop_front();
       return entry;
     }
 
     nsDisplayItem* next = GetNext();
     return DisplayItemEntry { next, DisplayItemEntryType::ITEM };
   }
 
-  nsDisplayItem* GetNext()
-  {
-    // This function is only supposed to be called if there are no markers set.
-    // Breaking this invariant can potentially break effect flattening and/or
-    // display item merging.
-    MOZ_ASSERT(mMarkers.empty());
-
-    return FlattenedDisplayItemIterator::GetNext();
-  }
+  nsDisplayItem* GetNext();
 
   bool HasNext() const
   {
     return FlattenedDisplayItemIterator::HasNext() || !mMarkers.empty();
   }
 
   nsDisplayItem* PeekNext()
   {
@@ -1744,16 +1736,59 @@ protected:
 
     CachedScrollMetadata()
       : mASR(nullptr), mClip(nullptr)
     {}
   };
   CachedScrollMetadata mCachedScrollMetadata;
 };
 
+nsDisplayItem*
+FLBDisplayItemIterator::GetNext()
+{
+  // This function is only supposed to be called if there are no markers set.
+  // Breaking this invariant can potentially break effect flattening and/or
+  // display item merging.
+  MOZ_ASSERT(mMarkers.empty());
+
+  nsDisplayItem* next = mNext;
+
+  // Advance mNext to the following item
+  if (next) {
+    nsDisplayItem* peek = next->GetAbove();
+
+    // Peek ahead to the next item and see if it can be merged with the
+    // current item.
+    if (peek && next->CanMerge(peek)) {
+      // Create a list of consecutive items that can be merged together.
+      AutoTArray<nsDisplayItem*, 2> mergedItems { next, peek };
+      while ((peek = peek->GetAbove())) {
+        if (!next->CanMerge(peek)) {
+          break;
+        }
+
+        mergedItems.AppendElement(peek);
+      }
+
+      // We have items that can be merged together.
+      // Merge them into a temporary item and process that item immediately.
+      MOZ_ASSERT(mergedItems.Length() > 1);
+      next = mState->mBuilder->MergeItems(mergedItems);
+    }
+
+    // |mNext| is either the first item that could not be merged with |next|,
+    // or a nullptr.
+    mNext = peek;
+
+    ResolveFlattening();
+  }
+
+  return next;
+}
+
 bool
 FLBDisplayItemIterator::NextItemWantsInactiveLayer()
 {
   LayerState layerState = mNext->GetLayerState(mState->mBuilder,
                                                mState->mManager,
                                                mState->mParameters);
 
   return layerState == LayerState::LAYER_INACTIVE;
@@ -4588,42 +4623,16 @@ ContainerState::ProcessDisplayItems(nsDi
         // If this item is inside a flattened effect, everything below is
         // unnecessary processing.
         MOZ_ASSERT(selectedLayer);
         selectedLayer->AccumulateHitTestInfo(this, hitTestInfo, transformNode);
         continue;
       }
     }
 
-    if (marker == DisplayItemEntryType::ITEM) {
-      // Peek ahead to the next item and see if it can be merged with the
-      // current item.
-      nsDisplayItem* peek = iter.PeekNext();
-      if (peek && item->CanMerge(peek)) {
-        // Create a list of consecutive items that can be merged together.
-        AutoTArray<nsDisplayItem*, 2> mergedItems { item };
-        while ((peek = iter.PeekNext())) {
-          if (!item->CanMerge(peek)) {
-            break;
-          }
-
-          mergedItems.AppendElement(peek);
-
-          // Move the iterator forward since we will merge this item.
-          iter.GetNext();
-        }
-
-        // We have items that can be merged together.
-        // Merge them into a temporary item and process that item immediately.
-        MOZ_ASSERT(mergedItems.Length() > 1);
-        item = mBuilder->MergeItems(mergedItems);
-        MOZ_ASSERT(item && itemType == item->GetType());
-      }
-    }
-
     MOZ_ASSERT(item->GetType() != DisplayItemType::TYPE_WRAP_LIST);
 
     NS_ASSERTION(mAppUnitsPerDevPixel == AppUnitsPerDevPixel(item),
       "items in a container layer should all have the same app units per dev pixel");
 
     if (mBuilder->NeedToForceTransparentSurfaceForItem(item)) {
       aList->SetNeedsTransparentSurface();
     }
new file mode 100644
--- /dev/null
+++ b/layout/painting/crashtests/1477831-1.html
@@ -0,0 +1,11 @@
+<style>
+* {
+  margin-left: 1vw;
+  columns: 2;
+  opacity: 0.2;
+  -webkit-transform: rotate(0deg);
+}
+#a { float: left; }
+</style>
+<content id="a">
+<dd>A</dd>
--- a/layout/painting/crashtests/crashtests.list
+++ b/layout/painting/crashtests/crashtests.list
@@ -9,8 +9,9 @@ load 1419917.html
 load 1425271-1.html
 load 1428906-1.html
 load 1430589-1.html
 load 1454105-1.html
 load 1455944-1.html
 load 1465305-1.html
 load 1468124-1.html
 load 1469472.html
+load 1477831-1.html