Bug 1606685 - Support empty tiles within compositor surfaces. r=sotaro
authorGlenn Watson <git@intuitionlibrary.com>
Wed, 08 Jan 2020 04:57:31 +0000
changeset 509301 e9d191b5eb8a78b117f55c0dd6993cb0d136c7c8
parent 509300 12fb5e522dd32b5ff50c8196c05b3ba9d244417e
child 509302 6f33d915fcf503f8d200430d6e0496b59d3bf82f
push id36994
push userccoroiu@mozilla.com
push dateWed, 08 Jan 2020 16:45:33 +0000
treeherdermozilla-central@6378942bfb04 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1606685
milestone74.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 1606685 - Support empty tiles within compositor surfaces. r=sotaro This adds support for holes within virtual surfaces. On platforms that don't use virtual surfaces, this just works by destroying the tile that is empty so it never gets composited. Differential Revision: https://phabricator.services.mozilla.com/D59059
gfx/webrender_bindings/DCLayerTree.cpp
gfx/wr/webrender/src/picture.rs
--- a/gfx/webrender_bindings/DCLayerTree.cpp
+++ b/gfx/webrender_bindings/DCLayerTree.cpp
@@ -419,35 +419,33 @@ void DCSurface::DestroyTile(int aX, int 
   mVisual->RemoveVisual(layer->GetVisual());
 #endif
   mDCLayers.erase(key);
 }
 
 #ifdef USE_VIRTUAL_SURFACES
 void DCSurface::UpdateAllocatedRect() {
   if (mAllocatedRectDirty) {
-    RECT rect = {1000000, 1000000, -1000000, -1000000};
+    // The virtual surface may have holes in it (for example, an empty tile
+    // that has no primitives). Instead of trimming to a single bounding
+    // rect, supply the rect of each valid tile to handle this case.
+    std::vector<RECT> validRects;
 
     for (auto it = mDCLayers.begin(); it != mDCLayers.end(); ++it) {
-      int x = it->first.mX;
-      int y = it->first.mY;
+      RECT rect;
 
-      rect.left = std::min((int)rect.left, x * mTileSize.width);
-      rect.right = std::max((int)rect.right, (x + 1) * mTileSize.width);
+      rect.left = (LONG) (VIRTUAL_OFFSET + it->first.mX * mTileSize.width);
+      rect.top = (LONG) (VIRTUAL_OFFSET + it->first.mY * mTileSize.height);
+      rect.right = rect.left + mTileSize.width;
+      rect.bottom = rect.top + mTileSize.height;
 
-      rect.top = std::min((int)rect.top, y * mTileSize.height);
-      rect.bottom = std::max((int)rect.bottom, (y + 1) * mTileSize.height);
+      validRects.push_back(rect);
     }
 
-    rect.left += VIRTUAL_OFFSET;
-    rect.top += VIRTUAL_OFFSET;
-    rect.bottom += VIRTUAL_OFFSET;
-    rect.right += VIRTUAL_OFFSET;
-
-    mVirtualSurface->Trim(&rect, 1);
+    mVirtualSurface->Trim(validRects.data(), validRects.size());
     mAllocatedRectDirty = false;
   }
 }
 #endif
 
 DCLayer* DCSurface::GetLayer(int aX, int aY) const {
   TileKey key(aX, aY);
   auto layer_it = mDCLayers.find(key);
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -875,31 +875,29 @@ impl Tile {
         // (and thus updated / invalidated) until it is on screen again.
         if !self.is_visible {
             return false;
         }
 
         // Invalidate the tile based on the content changing.
         self.update_content_validity(ctx, state);
 
-        // TODO(gw): This is a hack / temporary bug fix. With the recent changes
-        //           to treat native surfaces as an entire surface, we need to
-        //           skip the optimization that drops empty tiles within the
-        //           surface area. This has some unfortunate performance implications
-        //           in some cases, so we'll need a proper fix for this, but this
-        //           should fix correctness for now, at least.
-        match state.composite_state.compositor_kind {
-            CompositorKind::Draw { .. } => {
-                // If there are no primitives there is no need to draw or cache it.
-                if self.current_descriptor.prims.is_empty() {
-                    return false;
+        // If there are no primitives there is no need to draw or cache it.
+        if self.current_descriptor.prims.is_empty() {
+            // If there is a native compositor surface allocated for this (now empty) tile
+            // it must be freed here, otherwise the stale tile with previous contents will
+            // be composited. If the tile subsequently gets new primitives added to it, the
+            // surface will be re-allocated when it's added to the composite draw list.
+            if let Some(TileSurface::Texture { descriptor: SurfaceTextureDescriptor::Native { mut id, .. }, .. }) = self.surface.take() {
+                if let Some(id) = id.take() {
+                    state.resource_cache.destroy_compositor_tile(id);
                 }
             }
-            CompositorKind::Native { .. } => {
-            }
+
+            return false;
         }
 
         // Check if this tile can be considered opaque. Opacity state must be updated only
         // after all early out checks have been performed. Otherwise, we might miss updating
         // the native surface next time this tile becomes visible.
         self.is_opaque = ctx.backdrop.rect.contains_rect(&self.clipped_rect);
 
         // Check if the selected composite mode supports dirty rect updates. For Draw composite