Backed out changeset 58ebeaf27cf6 (bug 1505934) for wrench bustages. CLOSED TREE
authorNarcis Beleuzu <nbeleuzu@mozilla.com>
Mon, 14 Jan 2019 21:57:30 +0200
changeset 510894 bb6af9b92cdf9aa3becad03e149ee1c265805452
parent 510893 837ae1da7e4de5ba8ae3b4dfce0e4576dcd0adbb
child 510895 a7f3a2c57318fe4cea11857b2fa5bee1411bc96c
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1505934
milestone66.0a1
backs out58ebeaf27cf6c123bfd8de1f29c4034ad42435d5
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
Backed out changeset 58ebeaf27cf6 (bug 1505934) for wrench bustages. CLOSED TREE
gfx/tests/crashtests/1505934-1.html
gfx/tests/crashtests/crashtests.list
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/spatial_node.rs
deleted file mode 100644
--- a/gfx/tests/crashtests/1505934-1.html
+++ /dev/null
@@ -1,22 +0,0 @@
-<!DOCTYPE html>
-<style>
-  body {
-      background-color: limegreen;
-  }
-  .A {
-      transform: scale(0.01) perspective(1000px);
-      transform-origin: 0% 0%;
-      filter: grayscale(40%);
-  }
-  .B {
-      background-color: black;
-      width: 10000px;
-      height: 5000px;
-      border-radius: 2000px;
-  }
-</style>
-<body>
-  <div class="A">
-      <div class = "B"/>
-  </div>
-</body>
--- a/gfx/tests/crashtests/crashtests.list
+++ b/gfx/tests/crashtests/crashtests.list
@@ -174,11 +174,10 @@ load 1490704-1.html
 load 1501518.html
 load 1503986-1.html
 load 1505426-1.html
 load 1508811.html
 load 1508822.html
 load 1509099.html
 load 1513133.html
 load 1496194.html
-load 1505934-1.html
 load 1509123.html
 load texture-allocator-zero-region.html
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -81,27 +81,16 @@ pub struct TileIndex(pub usize);
 
 /// The size in device pixels of a cached tile. The currently chosen
 /// size is arbitrary. We should do some profiling to find the best
 /// size for real world pages.
 pub const TILE_SIZE_WIDTH: i32 = 1024;
 pub const TILE_SIZE_HEIGHT: i32 = 256;
 const FRAMES_BEFORE_CACHING: usize = 2;
 
-/// The maximum size per axis of texture cache item,
-///  in WorldPixel coordinates.
-// TODO(gw): This size is quite arbitrary - we should do some
-//           profiling / telemetry to see when it makes sense
-//           to cache a picture.
-const MAX_CACHE_SIZE: f32 = 2048.0;
-/// The maximum size per axis of a surface,
-///  in WorldPixel coordinates.
-const MAX_SURFACE_SIZE: f32 = 4096.0;
-
-
 #[derive(Debug)]
 pub struct GlobalTransformInfo {
     /// Current (quantized) value of the transform, that is
     /// independent of the value of the spatial node index.
     /// Only calculated on first use.
     current: Option<TransformKey>,
     /// Tiles check this to see if the dependencies have changed.
     changed: bool,
@@ -1284,21 +1273,16 @@ impl SurfaceInfo {
             surface: None,
             raster_spatial_node_index,
             surface_spatial_node_index,
             tasks: Vec::new(),
             inflation_factor,
         }
     }
 
-    pub fn fits_surface_size_limits(&self) -> bool {
-        self.map_local_to_surface.bounds.size.width <= MAX_SURFACE_SIZE &&
-        self.map_local_to_surface.bounds.size.height <= MAX_SURFACE_SIZE
-    }
-
     /// Take the set of child render tasks for this surface. This is
     /// used when constructing the render task tree.
     pub fn take_render_tasks(&mut self) -> Vec<RenderTaskId> {
         mem::replace(&mut self.tasks, Vec::new())
     }
 }
 
 #[derive(Debug)]
@@ -1832,18 +1816,17 @@ impl PicturePrimitive {
                 Some(PlaneSplitter::new())
             }
             Picture3DContext::In { root_data: None, .. } => {
                 None
             }
         };
 
         let state = PictureState {
-            is_cacheable: pic_bounds.size.width <= MAX_CACHE_SIZE &&
-                pic_bounds.size.height <= MAX_CACHE_SIZE,
+            is_cacheable: true,
             map_local_to_pic,
             map_pic_to_world,
             map_pic_to_raster,
             map_raster_to_world,
             plane_splitter,
         };
 
         // Disallow subpixel AA if an intermediate surface is needed.
@@ -2032,85 +2015,85 @@ impl PicturePrimitive {
             mode => mode,
         };
 
         if let Some(composite_mode) = actual_composite_mode {
             // Retrieve the positioning node information for the parent surface.
             let parent_raster_spatial_node_index = state.current_surface().raster_spatial_node_index;
             let surface_spatial_node_index = self.spatial_node_index;
 
+            // Check if there is perspective, and thus whether a new
+            // rasterization root should be established.
+            let xf = frame_context.clip_scroll_tree.get_relative_transform(
+                parent_raster_spatial_node_index,
+                surface_spatial_node_index,
+            ).expect("BUG: unable to get relative transform");
+
+            // TODO(gw): A temporary hack here to revert behavior to
+            //           always raster in screen-space. This is not
+            //           a problem yet, since we're not taking advantage
+            //           of this for caching yet. This is a workaround
+            //           for some existing issues with handling scale
+            //           when rasterizing in local space mode. Once
+            //           the fixes for those are in-place, we can
+            //           remove this hack!
+            //let local_scale = raster_space.local_scale();
+            // let wants_raster_root = xf.has_perspective_component() ||
+            //                         local_scale.is_some();
+            let establishes_raster_root = xf.has_perspective_component();
+
             // TODO(gw): For now, we always raster in screen space. Soon,
             //           we will be able to respect the requested raster
             //           space, and/or override the requested raster root
             //           if it makes sense to.
             let raster_space = RasterSpace::Screen;
 
+            let raster_spatial_node_index = if establishes_raster_root {
+                surface_spatial_node_index
+            } else {
+                parent_raster_spatial_node_index
+            };
+
             let inflation_factor = match composite_mode {
                 PictureCompositeMode::Filter(FilterOp::Blur(blur_radius)) => {
                     // The amount of extra space needed for primitives inside
                     // this picture to ensure the visibility check is correct.
                     BLUR_SAMPLE_SCALE * blur_radius
                 }
                 _ => {
                     0.0
                 }
             };
 
-            let mut surface = {
-                // Check if there is perspective, and thus whether a new
-                // rasterization root should be established.
-                let xf = frame_context.clip_scroll_tree.get_relative_transform(
-                    parent_raster_spatial_node_index,
-                    surface_spatial_node_index,
-                ).expect("BUG: unable to get relative transform");
-
-                let establishes_raster_root = xf.has_perspective_component();
-
+            let surface_index = state.push_surface(
                 SurfaceInfo::new(
                     surface_spatial_node_index,
-                    if establishes_raster_root {
-                        surface_spatial_node_index
-                    } else {
-                        parent_raster_spatial_node_index
-                    },
+                    raster_spatial_node_index,
                     inflation_factor,
                     frame_context.screen_world_rect,
                     &frame_context.clip_scroll_tree,
                 )
-            };
+            );
 
-            if surface_spatial_node_index != parent_raster_spatial_node_index &&
-                !surface.fits_surface_size_limits()
-            {
-                // fall back to the parent raster root
-                surface = SurfaceInfo::new(
-                    surface_spatial_node_index,
-                    parent_raster_spatial_node_index,
-                    inflation_factor,
-                    frame_context.screen_world_rect,
-                    &frame_context.clip_scroll_tree,
-                );
-            };
+            self.raster_config = Some(RasterConfig {
+                composite_mode,
+                surface_index,
+                establishes_raster_root,
+            });
 
             // If we have a cache key / descriptor for this surface,
             // update any transforms it cares about.
             if let Some(ref mut surface_desc) = self.surface_desc {
                 surface_desc.update(
                     surface_spatial_node_index,
-                    surface.raster_spatial_node_index,
+                    raster_spatial_node_index,
                     frame_context.clip_scroll_tree,
                     raster_space,
                 );
             }
-
-            self.raster_config = Some(RasterConfig {
-                composite_mode,
-                establishes_raster_root: surface_spatial_node_index == surface.raster_spatial_node_index,
-                surface_index: state.push_surface(surface),
-            });
         }
 
         Some(mem::replace(&mut self.prim_list.pictures, SmallVec::new()))
     }
 
     /// Update the primitive dependencies for any active tile caches,
     /// but only *if* the transforms have made the mappings out of date.
     pub fn update_prim_dependencies(
@@ -2166,17 +2149,17 @@ impl PicturePrimitive {
                     Picture3DContext::In { root_data: None, ancestor_index } => {
                         ancestor_index
                     }
                 };
 
                 let map_local_to_containing_block: SpaceMapper<LayoutPixel, LayoutPixel> = SpaceMapper::new_with_target(
                     containing_block_index,
                     cluster.spatial_node_index,
-                    LayoutRect::zero(), // bounds aren't going to be used for this mapping
+                    LayoutRect::zero(),     // bounds aren't going to be used for this mapping
                     &frame_context.clip_scroll_tree,
                 );
 
                 match map_local_to_containing_block.visible_face() {
                     VisibleFace::Back => continue,
                     VisibleFace::Front => {}
                 }
             }
@@ -2353,16 +2336,20 @@ impl PicturePrimitive {
                 // dimensions, so that it can be reused as it scrolls into
                 // view etc. However, if the unclipped size of the surface is
                 // too big, then it will be very expensive to draw, and may
                 // even be bigger than the maximum hardware render target
                 // size. In these cases, it's probably best to not cache the
                 // picture, and just draw a minimal portion of the picture
                 // (clipped to screen bounds) to a temporary target each frame.
 
+                // TODO(gw): This size is quite arbitrary - we should do some
+                //           profiling / telemetry to see when it makes sense
+                //           to cache a picture.
+                const MAX_CACHE_SIZE: f32 = 2048.0;
                 let too_big_to_cache = unclipped.size.width > MAX_CACHE_SIZE ||
                                        unclipped.size.height > MAX_CACHE_SIZE;
 
                 // If we can't create a valid cache key for this descriptor (e.g.
                 // due to it referencing old non-interned style primitives), then
                 // don't try to cache it.
                 let has_valid_cache_key = self.surface_desc.is_some();
 
--- a/gfx/wr/webrender/src/spatial_node.rs
+++ b/gfx/wr/webrender/src/spatial_node.rs
@@ -126,17 +126,17 @@ impl SpatialNode {
             LayoutFastTransform::identity, |perspective| perspective.into());
         let info = ReferenceFrameInfo {
             transform_style,
             source_transform: source_transform.unwrap_or(PropertyBinding::Value(identity)),
             source_perspective,
             origin_in_parent_reference_frame,
             invertible: true,
         };
-        Self::new(pipeline_id, parent_index, SpatialNodeType::ReferenceFrame(info))
+        Self::new(pipeline_id, parent_index, SpatialNodeType:: ReferenceFrame(info))
     }
 
     pub fn new_sticky_frame(
         parent_index: SpatialNodeIndex,
         sticky_frame_info: StickyFrameInfo,
         pipeline_id: PipelineId,
     ) -> Self {
         Self::new(pipeline_id, Some(parent_index), SpatialNodeType::StickyFrame(sticky_frame_info))