Bug 1585898 - Fix broken tab bar with picture caching on mac. r=mstange
authorGlenn Watson <git@intuitionlibrary.com>
Fri, 04 Oct 2019 01:20:49 +0000
changeset 496265 7a6d4a2a715a265c55b0493b6f3166749753df51
parent 496264 79e31fdfc7d1d6e2a180570d1a9f43c79bbb36db
child 496266 a96c48260a909224aa4653cb984cdbf9be4781e1
push id36648
push usercbrindusan@mozilla.com
push dateFri, 04 Oct 2019 09:46:56 +0000
treeherdermozilla-central@81ebffadd73a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1585898
milestone71.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 1585898 - Fix broken tab bar with picture caching on mac. r=mstange Promote clear primitives to be picture cache slices that can be drawn during the composite step. Without this, the clear primitive is not correct since it only operates on the slice it is assigned to, not the entire background before it. Differential Revision: https://phabricator.services.mozilla.com/D48139
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/composite.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/renderer.rs
gfx/wr/webrender/src/scene_building.rs
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -1199,42 +1199,52 @@ impl BatchBuilder {
                                     .map(&local_tile_clip_rect)
                                     .expect("bug: unable to map clip rect");
                                 let device_clip_rect = (world_clip_rect * ctx.global_device_pixel_scale).round();
                                 let z_id = composite_config.z_generator.next();
                                 for key in &tile_cache.tiles_to_draw {
                                     let tile = &tile_cache.tiles[key];
                                     let device_rect = (tile.world_rect * ctx.global_device_pixel_scale).round();
                                     let surface = tile.surface.as_ref().expect("no tile surface set!");
-                                    let (surface, is_opaque) = match surface {
+                                    match surface {
                                         TileSurface::Color { color } => {
-                                            (CompositeTileSurface::Color { color: *color }, true)
+                                            composite_config.opaque_tiles.push(CompositeTile {
+                                                surface: CompositeTileSurface::Color { color: *color },
+                                                rect: device_rect,
+                                                clip_rect: device_clip_rect,
+                                                z_id,
+                                            });
+                                        }
+                                        TileSurface::Clear => {
+                                            composite_config.clear_tiles.push(CompositeTile {
+                                                surface: CompositeTileSurface::Clear,
+                                                rect: device_rect,
+                                                clip_rect: device_clip_rect,
+                                                z_id,
+                                            });
                                         }
                                         TileSurface::Texture { handle, .. } => {
                                             let cache_item = ctx.resource_cache.texture_cache.get(handle);
 
-                                            (
-                                                CompositeTileSurface::Texture {
+                                            let composite_tile = CompositeTile {
+                                                surface: CompositeTileSurface::Texture {
                                                     texture_id: cache_item.texture_id,
                                                     texture_layer: cache_item.texture_layer,
                                                 },
-                                                tile.is_opaque || tile_cache.is_opaque(),
-                                            )
+                                                rect: device_rect,
+                                                clip_rect: device_clip_rect,
+                                                z_id,
+                                            };
+
+                                            if tile.is_opaque || tile_cache.is_opaque() {
+                                                composite_config.opaque_tiles.push(composite_tile);
+                                            } else {
+                                                composite_config.alpha_tiles.push(composite_tile);
+                                            }
                                         }
-                                    };
-                                    let composite_tile = CompositeTile {
-                                        surface,
-                                        rect: device_rect,
-                                        clip_rect: device_clip_rect,
-                                        z_id,
-                                    };
-                                    if is_opaque {
-                                        composite_config.opaque_tiles.push(composite_tile);
-                                    } else {
-                                        composite_config.alpha_tiles.push(composite_tile);
                                     }
                                 }
                             }
                             PictureCompositeMode::Filter(ref filter) => {
                                 assert!(filter.is_visible());
                                 match filter {
                                     Filter::Blur(..) => {
                                         let kind = BatchKind::Brush(
--- a/gfx/wr/webrender/src/composite.rs
+++ b/gfx/wr/webrender/src/composite.rs
@@ -18,16 +18,17 @@ use crate::internal_types::TextureSource
 pub enum CompositeTileSurface {
     Texture {
         texture_id: TextureSource,
         texture_layer: i32,
     },
     Color {
         color: ColorF,
     },
+    Clear,
 }
 
 /// Describes the geometry and surface of a tile to be composited
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct CompositeTile {
     pub surface: CompositeTileSurface,
     pub rect: DeviceRect,
@@ -36,20 +37,22 @@ pub struct CompositeTile {
 }
 
 /// The list of tiles to be drawn this frame
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct CompositeConfig {
     pub opaque_tiles: Vec<CompositeTile>,
     pub alpha_tiles: Vec<CompositeTile>,
+    pub clear_tiles: Vec<CompositeTile>,
     pub z_generator: ZBufferIdGenerator,
 }
 
 impl CompositeConfig {
     pub fn new() -> Self {
         CompositeConfig {
             opaque_tiles: Vec::new(),
             alpha_tiles: Vec::new(),
+            clear_tiles: Vec::new(),
             z_generator: ZBufferIdGenerator::new(0),
         }
     }
 }
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -374,23 +374,25 @@ pub enum TileSurface {
         /// Handle to the texture cache entry which gets drawn to.
         handle: TextureCacheHandle,
         /// Bitfield specifying the dirty region(s) that are relevant to this tile.
         visibility_mask: PrimitiveVisibilityMask,
     },
     Color {
         color: ColorF,
     },
+    Clear,
 }
 
 impl TileSurface {
     fn kind(&self) -> &'static str {
         match *self {
             TileSurface::Color { .. } => "Color",
             TileSurface::Texture { .. } => "Texture",
+            TileSurface::Clear => "Clear",
         }
     }
 }
 
 /// Information about a cached tile.
 pub struct Tile {
     /// The current world rect of this tile.
     pub world_rect: WorldRect,
@@ -650,36 +652,43 @@ impl Tile {
         self.root.maybe_merge_or_split(
             0,
             &self.current_descriptor.prims,
             max_split_level,
         );
 
         // See if this tile is a simple color, in which case we can just draw
         // it as a rect, and avoid allocating a texture surface and drawing it.
-        let is_solid_color = self.current_descriptor.prims.len() == 1 && self.is_opaque;
+        let is_simple_prim = self.current_descriptor.prims.len() == 1 && self.is_opaque;
 
         // Set up the backing surface for this tile.
-        let mut surface = if is_solid_color {
+        let mut surface = if is_simple_prim {
             // If we determine the tile can be represented by a color, set the
             // surface unconditionally (this will drop any previously used
             // texture cache backing surface).
-            TileSurface::Color {
-                color: ctx.backdrop.color,
+            match ctx.backdrop.kind {
+                BackdropKind::Color { color } => {
+                    TileSurface::Color {
+                        color,
+                    }
+                }
+                BackdropKind::Clear => {
+                    TileSurface::Clear
+                }
             }
         } else {
             // If this tile will be backed by a surface, we want to retain
             // the texture handle from the previous frame, if possible. If
             // the tile was previously a color, or not set, then just set
             // up a new texture cache handle.
             match self.surface.take() {
                 Some(old_surface @ TileSurface::Texture { .. }) => {
                     old_surface
                 }
-                Some(TileSurface::Color { .. }) | None => {
+                Some(TileSurface::Color { .. }) | Some(TileSurface::Clear) | None => {
                     TileSurface::Texture {
                         handle: TextureCacheHandle::invalid(),
                         visibility_mask: PrimitiveVisibilityMask::empty(),
                     }
                 }
             }
         };
 
@@ -1062,32 +1071,42 @@ impl ::std::fmt::Display for RecordedDir
 }
 
 impl ::std::fmt::Debug for RecordedDirtyRegion {
     fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
         ::std::fmt::Display::fmt(self, f)
     }
 }
 
+#[derive(Debug, Copy, Clone)]
+enum BackdropKind {
+    Color {
+        color: ColorF,
+    },
+    Clear,
+}
+
 /// Stores information about the calculated opaque backdrop of this slice.
 #[derive(Debug, Copy, Clone)]
 struct BackdropInfo {
     /// The picture space rectangle that is known to be opaque. This is used
     /// to determine where subpixel AA can be used, and where alpha blending
     /// can be disabled.
     rect: PictureRect,
-    /// Color of the backdrop.
-    color: ColorF,
+    /// Kind of the backdrop
+    kind: BackdropKind,
 }
 
 impl BackdropInfo {
     fn empty() -> Self {
         BackdropInfo {
             rect: PictureRect::zero(),
-            color: ColorF::BLACK,
+            kind: BackdropKind::Color {
+                color: ColorF::BLACK,
+            },
         }
     }
 }
 
 /// Represents a cache of tiles that make up a picture primitives.
 pub struct TileCacheInstance {
     /// Index of the tile cache / slice for this frame builder. It's determined
     /// by the setup_picture_caching method during flattening, which splits the
@@ -1614,17 +1633,19 @@ impl TileCacheInstance {
                         prim_spatial_node.coordinate_system_id == surface_spatial_node.coordinate_system_id
                     };
 
                     if let Some(ref clip_chain) = prim_clip_chain {
                         if prim_is_opaque && same_coord_system && !clip_chain.needs_mask && on_picture_surface {
                             if clip_chain.pic_clip_rect.contains_rect(&self.backdrop.rect) {
                                 self.backdrop = BackdropInfo {
                                     rect: clip_chain.pic_clip_rect,
-                                    color,
+                                    kind: BackdropKind::Color {
+                                        color,
+                                    },
                                 };
                             }
                         }
                     };
                 } else {
                     let opacity_binding = &opacity_binding_store[opacity_binding_index];
                     for binding in &opacity_binding.bindings {
                         prim_info.opacity_bindings.push(OpacityBinding::from(*binding));
@@ -1680,18 +1701,25 @@ impl TileCacheInstance {
 
                     if on_picture_surface && subpx_requested {
                         if !self.backdrop.rect.contains_rect(&prim_info.prim_clip_rect) {
                             self.subpixel_mode = SubpixelMode::Deny;
                         }
                     }
                 }
             }
+            PrimitiveInstanceKind::Clear { .. } => {
+                if let Some(ref clip_chain) = prim_clip_chain {
+                    self.backdrop = BackdropInfo {
+                        rect: clip_chain.pic_clip_rect,
+                        kind: BackdropKind::Clear,
+                    };
+                }
+            }
             PrimitiveInstanceKind::LineDecoration { .. } |
-            PrimitiveInstanceKind::Clear { .. } |
             PrimitiveInstanceKind::NormalBorder { .. } |
             PrimitiveInstanceKind::LinearGradient { .. } |
             PrimitiveInstanceKind::RadialGradient { .. } |
             PrimitiveInstanceKind::Backdrop { .. } => {
                 // These don't contribute dependencies
             }
         };
 
@@ -2282,16 +2310,18 @@ bitflags! {
         /// Is a backdrop-filter cluster that requires special handling during post_update.
         const IS_BACKDROP_FILTER = 8;
         /// Force creation of a picture caching slice before this cluster.
         const CREATE_PICTURE_CACHE_PRE = 16;
         /// Force creation of a picture caching slice after this cluster.
         const CREATE_PICTURE_CACHE_POST = 32;
         /// If set, this cluster represents a scroll bar container.
         const SCROLLBAR_CONTAINER = 64;
+        /// If set, this cluster contains clear rectangle primitives.
+        const IS_CLEAR_PRIMITIVE = 128;
     }
 }
 
 /// Descriptor for a cluster of primitives. For now, this is quite basic but will be
 /// extended to handle more spatial clustering of primitives.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct PrimitiveCluster {
     /// The positioning node for this cluster.
@@ -2395,16 +2425,19 @@ impl PrimitiveList {
         // iterate all pictures in a given primitive list.
         match prim_instance.kind {
             PrimitiveInstanceKind::Picture { .. } => {
                 flags.insert(ClusterFlags::IS_PICTURE);
             }
             PrimitiveInstanceKind::Backdrop { .. } => {
                 flags.insert(ClusterFlags::IS_BACKDROP_FILTER);
             }
+            PrimitiveInstanceKind::Clear { .. } => {
+                flags.insert(ClusterFlags::IS_CLEAR_PRIMITIVE);
+            }
             _ => {}
         }
 
         if prim_flags.contains(PrimitiveFlags::IS_BACKFACE_VISIBLE) {
             flags.insert(ClusterFlags::IS_BACKFACE_VISIBLE);
         }
 
         if prim_flags.contains(PrimitiveFlags::IS_SCROLLBAR_CONTAINER) {
--- a/gfx/wr/webrender/src/renderer.rs
+++ b/gfx/wr/webrender/src/renderer.rs
@@ -3844,16 +3844,19 @@ impl Renderer {
         let mut instances = Vec::new();
 
         for tile in tiles_iter {
             // Work out the draw params based on the tile surface
             let (texture, layer, color) = match tile.surface {
                 CompositeTileSurface::Color { color } => {
                     (TextureSource::Dummy, 0.0, color)
                 }
+                CompositeTileSurface::Clear => {
+                    (TextureSource::Dummy, 0.0, ColorF::BLACK)
+                }
                 CompositeTileSurface::Texture { texture_id, texture_layer } => {
                     (texture_id, texture_layer as f32, ColorF::WHITE)
                 }
             };
             let textures = BatchTextures::color(texture);
 
             // Flush this batch if the textures aren't compatible
             if !current_textures.is_compatible_with(&textures) {
@@ -3920,16 +3923,28 @@ impl Renderer {
             self.set_blend(false, FramebufferKind::Main);
             self.draw_tile_list(
                 composite_config.opaque_tiles.iter().rev(),
                 stats,
             );
             self.gpu_profile.finish_sampler(opaque_sampler);
         }
 
+        if !composite_config.clear_tiles.is_empty() {
+            let transparent_sampler = self.gpu_profile.start_sampler(GPU_SAMPLER_TAG_TRANSPARENT);
+            self.device.disable_depth_write();
+            self.set_blend(true, FramebufferKind::Main);
+            self.device.set_blend_mode_premultiplied_dest_out();
+            self.draw_tile_list(
+                composite_config.clear_tiles.iter(),
+                stats,
+            );
+            self.gpu_profile.finish_sampler(transparent_sampler);
+        }
+
         // Draw alpha tiles
         if !composite_config.alpha_tiles.is_empty() {
             let transparent_sampler = self.gpu_profile.start_sampler(GPU_SAMPLER_TAG_TRANSPARENT);
             self.device.disable_depth_write();
             self.set_blend(true, FramebufferKind::Main);
             self.set_blend_mode_premultiplied_alpha(FramebufferKind::Main);
             self.draw_tile_list(
                 composite_config.alpha_tiles.iter(),
--- a/gfx/wr/webrender/src/scene_building.rs
+++ b/gfx/wr/webrender/src/scene_building.rs
@@ -506,17 +506,19 @@ impl<'a> SceneBuilder<'a> {
         // If true, the cache is out of date and needs to be rebuilt.
         let mut update_shared_clips = true;
         // The last prim clip chain we build prim_clips for.
         let mut last_prim_clip_chain_id = ClipChainId::NONE;
 
         // Walk the supplied top level of clusters, slicing into slices as appropriate
         for cluster in main_prim_list.clusters.drain(..) {
             // Check if this cluster requires a new slice
-            create_slice |= cluster.flags.contains(ClusterFlags::CREATE_PICTURE_CACHE_PRE);
+            create_slice |= cluster.flags.intersects(
+                ClusterFlags::CREATE_PICTURE_CACHE_PRE | ClusterFlags::IS_CLEAR_PRIMITIVE
+            );
 
             if create_slice {
                 // When creating a slice, close off any open clip chains on prev slice.
                 if let Some(prev_slice) = slices.last_mut() {
                     prev_slice.pop_clip_instances(&clip_chain_instance_stack);
                 }
 
                 let mut slice = Slice {
@@ -603,17 +605,19 @@ impl<'a> SceneBuilder<'a> {
                         *shared_clips = Some(prim_clips.clone());
                     }
                 }
 
                 update_shared_clips = false;
             }
 
             // If this cluster creates a slice after, then note that for next cluster
-            create_slice |= cluster.flags.contains(ClusterFlags::CREATE_PICTURE_CACHE_POST);
+            create_slice |= cluster.flags.intersects(
+                ClusterFlags::CREATE_PICTURE_CACHE_POST | ClusterFlags::IS_CLEAR_PRIMITIVE
+            );
 
             // Finally, add this cluster to the current slice
             slices.last_mut().unwrap().prim_list.add_cluster(cluster);
         }
 
         // Close off any open clip chains on prev slice.
         if let Some(prev_slice) = slices.last_mut() {
             prev_slice.pop_clip_instances(&clip_chain_instance_stack);