Bug 1535540 - Remove the identity transform condition while checking redundant stacking contexts. r=emilio
authorGlenn Watson <github@intuitionlibrary.com>
Mon, 18 Mar 2019 03:08:26 +0000
changeset 522616 cdd0849c3c1121ed0f943844ec56994cbdec4515
parent 522615 a9a91a32262e099d6366cb2c7500db10f551bf13
child 522620 091fdeb8c9ca58a3ab6c00368828448fed62b4bd
push id10871
push usercbrindusan@mozilla.com
push dateMon, 18 Mar 2019 15:49:32 +0000
treeherdermozilla-beta@018abdd16060 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1535540
milestone67.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 1535540 - Remove the identity transform condition while checking redundant stacking contexts. r=emilio The only time that the ancestor spatial node index is read is during push_stacking_context. This means that even if it was used as an ancestor for a 3d context, we can safely collapse it in to the parent stacking context during flattening, if it is otherwise redundant. This is a partial fix for picture caching heuristics failing with the display list produced on mobile devices. Differential Revision: https://phabricator.services.mozilla.com/D23633
gfx/wr/webrender/src/clip_scroll_tree.rs
gfx/wr/webrender/src/display_list_flattener.rs
--- a/gfx/wr/webrender/src/clip_scroll_tree.rs
+++ b/gfx/wr/webrender/src/clip_scroll_tree.rs
@@ -525,59 +525,16 @@ impl ClipScrollTree {
 
     #[allow(dead_code)]
     pub fn print(&self) {
         if !self.spatial_nodes.is_empty() {
             let mut pt = PrintTree::new("clip_scroll tree");
             self.print_with(&mut pt);
         }
     }
-
-    /// Return true if this is a guaranteed identity transform. This
-    /// is conservative, it assumes not identity if a property
-    /// binding animation, or scroll frame is found, for example.
-    pub fn node_is_identity(
-        &self,
-        spatial_node_index: SpatialNodeIndex,
-    ) -> bool {
-        let mut current = spatial_node_index;
-
-        while current != ROOT_SPATIAL_NODE_INDEX {
-            let node = &self.spatial_nodes[current.0 as usize];
-
-            match node.node_type {
-                SpatialNodeType::ReferenceFrame(ref info) => {
-                    match info.source_transform {
-                        PropertyBinding::Value(transform) => {
-                            if transform != LayoutTransform::identity() {
-                                return false;
-                            }
-                        }
-                        PropertyBinding::Binding(..) => {
-                            // Assume not identity since it may change with animation.
-                            return false;
-                        }
-                    }
-                }
-                SpatialNodeType::ScrollFrame(ref info) => {
-                    // Assume not identity since it may change with scrolling.
-                    if let ScrollFrameKind::Explicit = info.frame_kind {
-                        return false;
-                    }
-                }
-                SpatialNodeType::StickyFrame(..) => {
-                    // Assume not identity since it may change with scrolling.
-                    return false;
-                }
-            }
-            current = node.parent.unwrap();
-        }
-
-        true
-    }
 }
 
 impl PrintableTree for ClipScrollTree {
     fn print_with<T: PrintTreePrinter>(&self, pt: &mut T) {
         if !self.spatial_nodes.is_empty() {
             self.print_node(self.root_reference_frame_index(), pt);
         }
     }
--- a/gfx/wr/webrender/src/display_list_flattener.rs
+++ b/gfx/wr/webrender/src/display_list_flattener.rs
@@ -1429,20 +1429,17 @@ impl<'a> DisplayListFlattener<'a> {
         // (a) It's an optimization to reduce picture count and allocations, as display lists
         //     often contain a lot of these stacking contexts that don't require pictures or
         //     off-screen surfaces.
         // (b) It's useful for the initial version of picture caching in gecko, by enabling
         //     is to just look for interesting scroll roots on the root stacking context,
         //     without having to consider cuts at stacking context boundaries.
         let parent_is_empty = match self.sc_stack.last_mut() {
             Some(parent_sc) => {
-                if stacking_context.is_redundant(
-                    parent_sc,
-                    self.clip_scroll_tree,
-                ) {
+                if stacking_context.is_redundant(parent_sc) {
                     // If the parent context primitives list is empty, it's faster
                     // to assign the storage of the popped context instead of paying
                     // the copying cost for extend.
                     if parent_sc.primitives.is_empty() {
                         parent_sc.primitives = stacking_context.primitives;
                     } else {
                         parent_sc.primitives.extend(stacking_context.primitives);
                     }
@@ -2732,17 +2729,16 @@ impl FlattenedStackingContext {
     pub fn is_3d(&self) -> bool {
         self.transform_style == TransformStyle::Preserve3D && self.composite_ops.is_empty()
     }
 
     /// Return true if the stacking context isn't needed.
     pub fn is_redundant(
         &self,
         parent: &FlattenedStackingContext,
-        clip_scroll_tree: &ClipScrollTree,
     ) -> bool {
         // Any 3d context is required
         if let Picture3DContext::In { .. } = self.context_3d {
             return false;
         }
 
         // If there are filters / mix-blend-mode
         if !self.composite_ops.filters.is_empty() {
@@ -2771,21 +2767,16 @@ impl FlattenedStackingContext {
             return false;
         }
 
         // If need to isolate in surface due to clipping / mix-blend-mode
         if !self.blit_reason.is_empty() {
             return false;
         }
 
-        // If represents a transform, it may affect backface visibility of children
-        if !clip_scroll_tree.node_is_identity(self.spatial_node_index) {
-            return false;
-        }
-
         // If this stacking context gets picture caching, we need it.
         if self.create_tile_cache {
             return false;
         }
 
         // It is redundant!
         true
     }