Backed out changeset 7295ca89e880 (bug 1608280) for causing bug 1608912 and bug 1608871 a=backout
authorAndreea Pavel <apavel@mozilla.com>
Mon, 13 Jan 2020 22:40:49 +0200
changeset 509920 0690f68a8d9984d2653792e3294bf876eecef651
parent 509919 7828a10a94b6afb78d18d9b7b83e7aa79337cc24
child 510021 a8ad1f6e802cca44a74e2d959ed888ae425108d3
push id37011
push userapavel@mozilla.com
push dateMon, 13 Jan 2020 20:41:42 +0000
treeherdermozilla-central@0690f68a8d99 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1608280, 1608912, 1608871
milestone74.0a1
backs out7295ca89e880c1c930643e72ff0600cb71cebb9e
first release with
nightly linux32
0690f68a8d99 / 74.0a1 / 20200113204142 / files
nightly linux64
0690f68a8d99 / 74.0a1 / 20200113204142 / files
nightly mac
0690f68a8d99 / 74.0a1 / 20200113204142 / files
nightly win32
0690f68a8d99 / 74.0a1 / 20200113204142 / files
nightly win64
0690f68a8d99 / 74.0a1 / 20200113204142 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 7295ca89e880 (bug 1608280) for causing bug 1608912 and bug 1608871 a=backout
gfx/wr/webrender/src/composite.rs
gfx/wr/webrender/src/picture.rs
--- a/gfx/wr/webrender/src/composite.rs
+++ b/gfx/wr/webrender/src/composite.rs
@@ -117,26 +117,16 @@ pub enum CompositorKind {
     },
     /// Native OS compositor.
     Native {
         /// Maximum dirty rects per compositor surface.
         max_update_rects: usize,
     },
 }
 
-impl CompositorKind {
-    /// Returns true if this compositor is native (uses OS compositor)
-    pub fn is_native(&self) -> bool {
-        match self {
-            CompositorKind::Draw { .. } => false,
-            CompositorKind::Native { .. } => true,
-        }
-    }
-}
-
 impl Default for CompositorKind {
     /// Default compositor config is full present without partial present.
     fn default() -> Self {
         CompositorKind::Draw {
             max_partial_present_rects: 0,
         }
     }
 }
@@ -306,27 +296,18 @@ impl CompositeState {
                 // This can occur when a tile is found to be occluded during frame building.
                 continue;
             }
 
             visible_tile_count += 1;
 
             let device_rect = (tile.world_rect * global_device_pixel_scale).round();
             let dirty_rect = (tile.world_dirty_rect * global_device_pixel_scale).round();
-            let valid_rect = (tile.world_valid_rect * global_device_pixel_scale).round_out();
             let surface = tile.surface.as_ref().expect("no tile surface set!");
 
-            // When compositing in simple (draw) mode, each tile only needs to write pixels
-            // where (a) the valid region of the tile is and (b) the overall clip rect of
-            // the picture cache surface.
-            let clip_rect = match valid_rect.intersection(&device_clip_rect) {
-                Some(clip_rect) => clip_rect,
-                None => continue,
-            };
-
             let (surface, is_opaque) = match surface {
                 TileSurface::Color { color } => {
                     (CompositeTileSurface::Color { color: *color }, true)
                 }
                 TileSurface::Clear => {
                     (CompositeTileSurface::Clear, false)
                 }
                 TileSurface::Texture { descriptor, .. } => {
@@ -337,17 +318,17 @@ impl CompositeState {
                     )
                 }
             };
 
             let tile = CompositeTile {
                 surface,
                 rect: device_rect,
                 dirty_rect,
-                clip_rect,
+                clip_rect: device_clip_rect,
                 z_id,
                 tile_id: tile.id,
             };
 
             self.push_tile(tile, is_opaque);
         }
 
         if visible_tile_count > 0 {
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -333,41 +333,38 @@ pub struct SpatialNodeDependency {
     /// The current value retrieved from the clip-scroll tree.
     value: TransformKey,
     /// True if it was changed (or is new) since the last frame build.
     changed: bool,
 }
 
 // Immutable context passed to picture cache tiles during pre_update
 struct TilePreUpdateContext {
+    /// The local rect of the overall picture cache
+    local_rect: PictureRect,
+
+    /// The local clip rect (in picture space) of the entire picture cache
+    local_clip_rect: PictureRect,
+
     /// Maps from picture cache coords -> world space coords.
     pic_to_world_mapper: SpaceMapper<PicturePixel, WorldPixel>,
 
     /// The fractional position of the picture cache, which may
     /// require invalidation of all tiles.
     fract_offset: PictureVector2D,
 
     /// The optional background color of the picture cache instance
     background_color: Option<ColorF>,
 
     /// The visible part of the screen in world coords.
     global_screen_world_rect: WorldRect,
-
-    /// The current compositing mode.
-    compositor_kind: CompositorKind,
 }
 
 // Immutable context passed to picture cache tiles during post_update
 struct TilePostUpdateContext<'a> {
-    /// The local rect of the overall picture cache
-    local_rect: PictureRect,
-
-    /// The local clip rect (in picture space) of the entire picture cache
-    local_clip_rect: PictureRect,
-
     /// The calculated backdrop information for this cache instance.
     backdrop: BackdropInfo,
 
     /// Information about transform node differences from last frame.
     spatial_nodes: &'a FastHashMap<SpatialNodeIndex, SpatialNodeDependency>,
 
     /// Information about opacity bindings from the picture cache.
     opacity_bindings: &'a FastHashMap<PropertyBindingId, OpacityBindingInfo>,
@@ -574,20 +571,18 @@ enum InvalidationReason {
 }
 
 /// Information about a cached tile.
 pub struct Tile {
     /// The current world rect of this tile.
     pub world_rect: WorldRect,
     /// The current local rect of this tile.
     pub rect: PictureRect,
-    /// The valid region of this tile (parts of the tile affected by primitives).
-    pub valid_rect: PictureRect,
-    /// The world-space valid region of this tile.
-    pub world_valid_rect: WorldRect,
+    /// The local rect of the tile clipped to the overall picture local rect.
+    clipped_rect: PictureRect,
     /// Uniquely describes the content of this tile, in a way that can be
     /// (reasonably) efficiently hashed and compared.
     pub current_descriptor: TileDescriptor,
     /// The content descriptor for this tile from the previous frame.
     pub prev_descriptor: TileDescriptor,
     /// Handle to the backing surface for this tile.
     pub surface: Option<TileSurface>,
     /// If true, this tile is marked valid, and the existing texture
@@ -623,18 +618,17 @@ pub struct Tile {
 
 impl Tile {
     /// Construct a new, invalid tile.
     fn new(
         id: TileId,
     ) -> Self {
         Tile {
             rect: PictureRect::zero(),
-            valid_rect: PictureRect::zero(),
-            world_valid_rect: WorldRect::zero(),
+            clipped_rect: PictureRect::zero(),
             world_rect: WorldRect::zero(),
             surface: None,
             current_descriptor: TileDescriptor::new(),
             prev_descriptor: TileDescriptor::new(),
             is_valid: false,
             is_visible: false,
             fract_offset: PictureVector2D::zero(),
             id,
@@ -740,16 +734,21 @@ impl Tile {
     fn pre_update(
         &mut self,
         rect: PictureRect,
         ctx: &TilePreUpdateContext,
     ) {
         self.rect = rect;
         self.invalidation_reason  = None;
 
+        self.clipped_rect = self.rect
+            .intersection(&ctx.local_rect)
+            .and_then(|r| r.intersection(&ctx.local_clip_rect))
+            .unwrap_or_else(PictureRect::zero);
+
         self.world_rect = ctx.pic_to_world_mapper
             .map(&self.rect)
             .expect("bug: map local tile rect");
 
         // Check if this tile is currently on screen.
         self.is_visible = self.world_rect.intersects(&ctx.global_screen_world_rect);
 
         // If the tile isn't visible, early exit, skipping the normal set up to
@@ -776,46 +775,29 @@ impl Tile {
         // Clear any dependencies so that when we rebuild them we
         // can compare if the tile has the same content.
         mem::swap(
             &mut self.current_descriptor,
             &mut self.prev_descriptor,
         );
         self.current_descriptor.clear();
         self.root.clear(rect);
-
-        // Reset which part of the tile contains valid primitives. If the tile
-        // has a background color, the entire primitive must always be drawn
-        // composited. Otherwise, we will update the valid region of the tile
-        // during `add_prim_dependency`. For now, this optimization only applies
-        // to Draw compositor kinds - native compositors will be implemented
-        // as a follow up.
-        let has_background_color = self.background_color.is_some();
-        let is_native_compositor = ctx.compositor_kind.is_native();
-        self.valid_rect = if has_background_color || is_native_compositor {
-            self.rect
-        } else {
-            PictureRect::zero()
-        };
     }
 
     /// Add dependencies for a given primitive to this tile.
     fn add_prim_dependency(
         &mut self,
         info: &PrimitiveDependencyInfo,
     ) {
         // If this tile isn't currently visible, we don't want to update the dependencies
         // for this tile, as an optimization, since it won't be drawn anyway.
         if !self.is_visible {
             return;
         }
 
-        // Update the picture space valid region of this tile.
-        self.valid_rect = self.valid_rect.union(&info.prim_clip_rect);
-
         // Include any image keys this tile depends on.
         self.current_descriptor.images.extend_from_slice(&info.images);
 
         // Include any opacity bindings this primitive depends on.
         self.current_descriptor.opacity_bindings.extend_from_slice(&info.opacity_bindings);
 
         // Include any clip nodes that this primitive depends on.
         self.current_descriptor.clips.extend_from_slice(&info.clips);
@@ -911,22 +893,17 @@ impl Tile {
             }
 
             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.
-        let clipped_rect = self.rect
-            .intersection(&ctx.local_rect)
-            .and_then(|r| r.intersection(&ctx.local_clip_rect))
-            .and_then(|r| r.intersection(&self.valid_rect))
-            .unwrap_or_else(PictureRect::zero);
-        self.is_opaque = ctx.backdrop.rect.contains_rect(&clipped_rect);
+        self.is_opaque = ctx.backdrop.rect.contains_rect(&self.clipped_rect);
 
         // Check if the selected composite mode supports dirty rect updates. For Draw composite
         // mode, we can always update the content with smaller dirty rects. For native composite
         // mode, we can only use dirty rects if the compositor supports partial surface updates.
         let (supports_dirty_rects, supports_simple_prims) = match state.composite_state.compositor_kind {
             CompositorKind::Draw { .. } => {
                 (true, true)
             }
@@ -1873,21 +1850,22 @@ impl TileCacheInstance {
         self.tile_bounds_p0 = TileOffset::new(x0, y0);
         self.tile_bounds_p1 = TileOffset::new(x1, y1);
 
         let mut world_culling_rect = WorldRect::zero();
 
         mem::swap(&mut self.tiles, &mut self.old_tiles);
 
         let ctx = TilePreUpdateContext {
+            local_rect: self.local_rect,
+            local_clip_rect: self.local_clip_rect,
             pic_to_world_mapper,
             fract_offset: self.fract_offset,
             background_color: self.background_color,
             global_screen_world_rect: frame_context.global_screen_world_rect,
-            compositor_kind: frame_state.composite_state.compositor_kind,
         };
 
         self.tiles.clear();
         for y in y0 .. y1 {
             for x in x0 .. x1 {
                 let key = TileOffset::new(x, y);
 
                 let mut tile = self.old_tiles
@@ -2341,18 +2319,16 @@ impl TileCacheInstance {
 
             self.spatial_nodes.insert(spatial_node_index, SpatialNodeDependency {
                 changed,
                 value,
             });
         }
 
         let ctx = TilePostUpdateContext {
-            local_rect: self.local_rect,
-            local_clip_rect: self.local_clip_rect,
             backdrop: self.backdrop,
             spatial_nodes: &self.spatial_nodes,
             opacity_bindings: &self.opacity_bindings,
             current_tile_size: self.current_tile_size,
         };
 
         let mut state = TilePostUpdateState {
             resource_cache: frame_state.resource_cache,
@@ -3672,17 +3648,16 @@ impl PicturePrimitive {
                                 tile.is_visible = false;
                                 continue;
                             }
 
                             if frame_context.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) {
                                 tile.root.draw_debug_rects(
                                     &map_pic_to_world,
                                     tile.is_opaque,
-                                    &tile.valid_rect,
                                     scratch,
                                     frame_context.global_device_pixel_scale,
                                 );
 
                                 let label_offset = DeviceVector2D::new(20.0, 30.0);
                                 let tile_device_rect = tile.world_rect * frame_context.global_device_pixel_scale;
                                 if tile_device_rect.size.height >= label_offset.y {
                                     let surface = tile.surface.as_ref().expect("no tile surface set!");
@@ -3724,19 +3699,18 @@ impl PicturePrimitive {
                                         if id.is_none() {
                                             // There is no current surface allocation, so ensure the entire tile is invalidated
                                             tile.invalidate(None, InvalidationReason::NoSurface);
                                         }
                                     }
                                 }
                             }
 
-                            // Update the world space dirty and valid rects
+                            // Update the world dirty rect
                             tile.world_dirty_rect = map_pic_to_world.map(&tile.dirty_rect).expect("bug");
-                            tile.world_valid_rect = map_pic_to_world.map(&tile.valid_rect).expect("bug");
 
                             if tile.is_valid {
                                 continue;
                             }
 
                             // Ensure that this texture is allocated.
                             if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = tile.surface.as_mut().unwrap() {
                                 match descriptor {
@@ -3805,38 +3779,25 @@ impl PicturePrimitive {
 
                                 let content_origin_f = tile.world_rect.origin * device_pixel_scale;
                                 let content_origin = content_origin_f.round();
                                 debug_assert!((content_origin_f.x - content_origin.x).abs() < 0.01);
                                 debug_assert!((content_origin_f.y - content_origin.y).abs() < 0.01);
 
                                 // Get a task-local scissor rect for the dirty region of this
                                 // picture cache task.
-                                let dirty_rect = tile.world_dirty_rect
-                                    .translate(-tile.world_rect.origin.to_vector());
-                                let dirty_rect = (dirty_rect * device_pixel_scale).round();
-
-                                // The valid rect isn't guaranteed to be aligned to the device pixel
-                                // grid. To ensure we don't skip any valid pixels, round this out, and
-                                // then intersect it with the dirty rect (which _is_ device pixel
-                                // aligned) below when creating the true scissor rect.
-                                let valid_rect = tile.world_valid_rect
-                                    .translate(-tile.world_rect.origin.to_vector());
-                                let valid_rect = (valid_rect * device_pixel_scale).round_out();
-
+                                let scissor_rect = tile.world_dirty_rect.translate(
+                                    -tile.world_rect.origin.to_vector()
+                                );
                                 // The world rect is guaranteed to be device pixel aligned, by the tile
                                 // sizing code in tile::pre_update. However, there might be some
                                 // small floating point accuracy issues (these were observed on ARM
                                 // CPUs). Round the rect here before casting to integer device pixels
                                 // to ensure the scissor rect is correct.
-                                let scissor_rect = dirty_rect
-                                    .intersection(&valid_rect)
-                                    .unwrap_or(DeviceRect::zero())
-                                    .round();
-
+                                let scissor_rect = (scissor_rect * device_pixel_scale).round();
                                 let surface = descriptor.resolve(
                                     frame_state.resource_cache,
                                     tile_cache.current_tile_size,
                                 );
 
                                 let task = RenderTask::new_picture(
                                     RenderTaskLocation::PictureCache {
                                         size: tile_cache.current_tile_size,
@@ -4915,50 +4876,45 @@ impl TileNode {
         }
     }
 
     /// Draw debug information about this tile node
     fn draw_debug_rects(
         &self,
         pic_to_world_mapper: &SpaceMapper<PicturePixel, WorldPixel>,
         is_opaque: bool,
-        valid_rect: &PictureRect,
         scratch: &mut PrimitiveScratchBuffer,
         global_device_pixel_scale: DevicePixelScale,
     ) {
         match self.kind {
             TileNodeKind::Leaf { dirty_tracker, .. } => {
                 let color = if (dirty_tracker & 1) != 0 {
                     debug_colors::RED
                 } else if is_opaque {
                     debug_colors::GREEN
                 } else {
                     debug_colors::YELLOW
                 };
 
-                let local_rect = self.rect.intersection(valid_rect);
-                if let Some(local_rect) = local_rect {
-                    let world_rect = pic_to_world_mapper.map(&local_rect).unwrap();
-                    let device_rect = world_rect * global_device_pixel_scale;
-
-                    let outer_color = color.scale_alpha(0.3);
-                    let inner_color = outer_color.scale_alpha(0.5);
-                    scratch.push_debug_rect(
-                        device_rect.inflate(-3.0, -3.0),
-                        outer_color,
-                        inner_color
-                    );
-                }
+                let world_rect = pic_to_world_mapper.map(&self.rect).unwrap();
+                let device_rect = world_rect * global_device_pixel_scale;
+
+                let outer_color = color.scale_alpha(0.3);
+                let inner_color = outer_color.scale_alpha(0.5);
+                scratch.push_debug_rect(
+                    device_rect.inflate(-3.0, -3.0),
+                    outer_color,
+                    inner_color
+                );
             }
             TileNodeKind::Node { ref children, .. } => {
                 for child in children.iter() {
                     child.draw_debug_rects(
                         pic_to_world_mapper,
                         is_opaque,
-                        valid_rect,
                         scratch,
                         global_device_pixel_scale,
                     );
                 }
             }
         }
     }