Bug 1560499 - Fix subpixel detection for transparent tile caches. r=kvark
authorGlenn Watson <github@intuitionlibrary.com>
Mon, 24 Jun 2019 17:32:36 +0000
changeset 479994 7c911a7be80e21eef29b4fe3485abcdfbd1433bf
parent 479993 1d5517788ccb535a9d6573e4db1c81417386af2a
child 479995 11459fc963acca1af7f6fc9da8a5a97b0b700257
push id88416
push usergwatson@mozilla.com
push dateMon, 24 Jun 2019 22:17:24 +0000
treeherderautoland@7c911a7be80e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskvark
bugs1560499
milestone69.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 1560499 - Fix subpixel detection for transparent tile caches. r=kvark Some pages from Gecko produce a display list for the main content tile cache that has a transparent background. Detect these cases and disable subpixel text rendering to ensure correct blending. Differential Revision: https://phabricator.services.mozilla.com/D35627
gfx/wr/webrender/src/picture.rs
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use api::{MixBlendMode, PipelineId, PremultipliedColorF};
-use api::{PropertyBinding, PropertyBindingId};
+use api::{PropertyBinding, PropertyBindingId, FontRenderMode};
 use api::{DebugFlags, RasterSpace, ImageKey, ColorF};
 use api::units::*;
 use crate::box_shadow::{BLUR_SAMPLE_SCALE};
 use crate::clip::{ClipStore, ClipDataStore, ClipChainInstance};
 use crate::clip_scroll_tree::{ROOT_SPATIAL_NODE_INDEX,
     ClipScrollTree, CoordinateSpaceMapping, SpatialNodeIndex, VisibleFace, CoordinateSystemId
 };
 use crate::debug_colors;
@@ -518,30 +518,34 @@ pub struct TileCacheInstance {
     /// Pre-calculated versions of the tile_rect above, used to speed up the
     /// calculations in get_tile_coords_for_rect.
     tile_bounds_p0: TileOffset,
     tile_bounds_p1: TileOffset,
     /// Local rect (unclipped) of the picture this cache covers.
     pub local_rect: PictureRect,
     /// Local clip rect for this tile cache.
     pub local_clip_rect: PictureRect,
-    /// If true, the entire tile cache was determined to be opaque. This allows
-    /// WR to enable subpixel AA text rendering for text runs on this slice.
-    pub is_opaque: bool,
     /// A list of tiles that are valid and visible, which should be drawn to the main scene.
     pub tiles_to_draw: Vec<TileKey>,
     /// The world space viewport that this tile cache draws into.
     /// Any clips outside this viewport can be ignored (and must be removed so that
     /// we can draw outside the bounds of the viewport).
     pub world_viewport_rect: WorldRect,
     /// The surface index that this tile cache will be drawn into.
     surface_index: SurfaceIndex,
     /// The background color from the renderer. If this is set opaque, we know it's
     /// fine to clear the tiles to this and allow subpixel text on the first slice.
     pub background_color: Option<ColorF>,
+    /// 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.
+    pub opaque_rect: PictureRect,
+    /// The allowed subpixel mode for this surface, which depends on the detected
+    /// opacity of the background.
+    pub subpixel_mode: SubpixelMode,
 }
 
 impl TileCacheInstance {
     pub fn new(
         slice: usize,
         spatial_node_index: SpatialNodeIndex,
         background_color: Option<ColorF>,
     ) -> Self {
@@ -556,31 +560,27 @@ impl TileCacheInstance {
             opacity_bindings: FastHashMap::default(),
             dirty_region: DirtyRegion::new(),
             tile_size: PictureSize::zero(),
             tile_rect: TileRect::zero(),
             tile_bounds_p0: TileOffset::zero(),
             tile_bounds_p1: TileOffset::zero(),
             local_rect: PictureRect::zero(),
             local_clip_rect: PictureRect::zero(),
-            is_opaque: false,
             tiles_to_draw: Vec::new(),
             world_viewport_rect: WorldRect::zero(),
             surface_index: SurfaceIndex(0),
             background_color,
+            opaque_rect: PictureRect::zero(),
+            subpixel_mode: SubpixelMode::Allow,
         }
     }
 
     /// Returns true if this tile cache is considered opaque.
     pub fn is_opaque(&self) -> bool {
-        // If we detected an explicit background rect that is opaque and covers all tiles.
-        if self.is_opaque {
-            return true;
-        }
-
         // If known opaque due to background clear color and being the first slice.
         // The background_color will only be Some(..) if this is the first slice.
         match self.background_color {
             Some(color) => color.a >= 1.0,
             None => false
         }
     }
 
@@ -616,16 +616,21 @@ impl TileCacheInstance {
         surface_index: SurfaceIndex,
         frame_context: &FrameVisibilityContext,
         frame_state: &mut FrameVisibilityState,
     ) -> WorldRect {
         let tile_width = TILE_SIZE_WIDTH;
         let tile_height = TILE_SIZE_HEIGHT;
         self.surface_index = surface_index;
 
+        // Reset the opaque rect + subpixel mode, as they are calculated
+        // during the prim dependency checks.
+        self.opaque_rect = PictureRect::zero();
+        self.subpixel_mode = SubpixelMode::Allow;
+
         self.map_local_to_surface = SpaceMapper::new(
             self.spatial_node_index,
             PictureRect::from_untyped(&pic_rect.to_untyped()),
         );
 
         let pic_to_world_mapper = SpaceMapper::new_with_target(
             ROOT_SPATIAL_NODE_INDEX,
             self.spatial_node_index,
@@ -799,19 +804,16 @@ impl TileCacheInstance {
                 self.tiles.insert(key, tile);
             }
         }
 
         // Do tile invalidation for any dependencies that we know now.
         for (_, tile) in &mut self.tiles {
             // Start frame assuming that the tile has the same content.
             tile.is_same_content = true;
-            // Reset the opacity of the tile to false, since it may change each frame.
-            // The primitive dependency updates will determine if it is opaque again.
-            tile.is_opaque = false;
 
             // Content has changed if any opacity bindings changed.
             for binding in tile.descriptor.opacity_bindings.items() {
                 if let OpacityBinding::Binding(id) = binding {
                     let changed = match self.opacity_bindings.get(id) {
                         Some(info) => info.changed,
                         None => true,
                     };
@@ -871,25 +873,43 @@ impl TileCacheInstance {
         }
 
         // Build the list of resources that this primitive has dependencies on.
         let mut opacity_bindings: SmallVec<[OpacityBinding; 4]> = SmallVec::new();
         let mut clip_chain_uids: SmallVec<[ItemUid; 8]> = SmallVec::new();
         let mut clip_vertices: SmallVec<[LayoutPoint; 8]> = SmallVec::new();
         let mut image_keys: SmallVec<[ImageKey; 8]> = SmallVec::new();
         let mut clip_spatial_nodes = FastHashSet::default();
-        let mut opaque_rect = None;
         let mut prim_clip_rect = PictureRect::zero();
 
         // Some primitives can not be cached (e.g. external video images)
         let is_cacheable = prim_instance.is_cacheable(
             &data_stores,
             resource_cache,
         );
 
+        // If there was a clip chain, add any clip dependencies to the list for this tile.
+        if let Some(prim_clip_chain) = prim_clip_chain {
+            prim_clip_rect = prim_clip_chain.pic_clip_rect;
+
+            let clip_instances = &clip_store
+                .clip_node_instances[prim_clip_chain.clips_range.to_range()];
+            for clip_instance in clip_instances {
+                clip_chain_uids.push(clip_instance.handle.uid());
+
+                // If the clip has the same spatial node, the relative transform
+                // will always be the same, so there's no need to depend on it.
+                if clip_instance.spatial_node_index != self.spatial_node_index {
+                    clip_spatial_nodes.insert(clip_instance.spatial_node_index);
+                }
+
+                clip_vertices.push(clip_instance.local_pos);
+            }
+        }
+
         // For pictures, we don't (yet) know the valid clip rect, so we can't correctly
         // use it to calculate the local bounding rect for the tiles. If we include them
         // then we may calculate a bounding rect that is too large, since it won't include
         // the clip bounds of the picture. Excluding them from the bounding rect here
         // fixes any correctness issues (the clips themselves are considered when we
         // consider the bounds of the primitives that are *children* of the picture),
         // however it does potentially result in some un-necessary invalidations of a
         // tile (in cases where the picture local rect affects the tile, but the clip
@@ -931,17 +951,19 @@ impl TileCacheInstance {
                         let surface_spatial_node = &clip_scroll_tree
                             .spatial_nodes[self.spatial_node_index.0 as usize];
 
                         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 {
-                            opaque_rect = Some(clip_chain.pic_clip_rect);
+                            if clip_chain.pic_clip_rect.contains_rect(&self.opaque_rect) {
+                                self.opaque_rect = clip_chain.pic_clip_rect;
+                            }
                         }
                     };
                 } else {
                     let opacity_binding = &opacity_binding_store[opacity_binding_index];
                     for binding in &opacity_binding.bindings {
                         opacity_bindings.push(OpacityBinding::from(*binding));
                     }
                 }
@@ -968,67 +990,65 @@ impl TileCacheInstance {
                 image_keys.extend_from_slice(&yuv_image_data.yuv_key);
                 false
             }
             PrimitiveInstanceKind::PushClipChain |
             PrimitiveInstanceKind::PopClipChain => {
                 // Early exit to ensure this doesn't get added as a dependency on the tile.
                 return false;
             }
-            PrimitiveInstanceKind::TextRun { .. } |
+            PrimitiveInstanceKind::TextRun { data_handle, .. } => {
+                // Only do these checks if we haven't already disabled subpx
+                // text rendering for this slice.
+                if self.subpixel_mode == SubpixelMode::Allow && !self.is_opaque() {
+                    let run_data = &data_stores.text_run[data_handle];
+
+                    // If a text run is on a child surface, the subpx mode will be
+                    // correctly determined as we recurse through pictures in take_context.
+                    let on_picture_surface = surface_index == self.surface_index;
+
+                    // Only care about text runs that have requested subpixel rendering.
+                    // This is conservative - it may still end up that a subpx requested
+                    // text run doesn't get subpx for other reasons (e.g. glyph size).
+                    let subpx_requested = match run_data.font.render_mode {
+                        FontRenderMode::Subpixel => true,
+                        FontRenderMode::Alpha | FontRenderMode::Mono => false,
+                    };
+
+                    if on_picture_surface && subpx_requested {
+                        if !self.opaque_rect.contains_rect(&prim_clip_rect) {
+                            self.subpixel_mode = SubpixelMode::Deny;
+                        }
+                    }
+                }
+
+                false
+            }
             PrimitiveInstanceKind::LineDecoration { .. } |
             PrimitiveInstanceKind::Clear { .. } |
             PrimitiveInstanceKind::NormalBorder { .. } |
             PrimitiveInstanceKind::LinearGradient { .. } |
             PrimitiveInstanceKind::RadialGradient { .. } |
             PrimitiveInstanceKind::ImageBorder { .. } => {
                 // These don't contribute dependencies
                 false
             }
         };
 
-        // If there was a clip chain, add any clip dependencies to the list for this tile.
-        if let Some(prim_clip_chain) = prim_clip_chain {
-            prim_clip_rect = prim_clip_chain.pic_clip_rect;
-
-            let clip_instances = &clip_store
-                .clip_node_instances[prim_clip_chain.clips_range.to_range()];
-            for clip_instance in clip_instances {
-                clip_chain_uids.push(clip_instance.handle.uid());
-
-                // If the clip has the same spatial node, the relative transform
-                // will always be the same, so there's no need to depend on it.
-                if clip_instance.spatial_node_index != self.spatial_node_index {
-                    clip_spatial_nodes.insert(clip_instance.spatial_node_index);
-                }
-
-                clip_vertices.push(clip_instance.local_pos);
-            }
-        }
-
         // Normalize the tile coordinates before adding to tile dependencies.
         // For each affected tile, mark any of the primitive dependencies.
         for y in p0.y .. p1.y {
             for x in p0.x .. p1.x {
                 // TODO(gw): Convert to 2d array temporarily to avoid hash lookups per-tile?
                 let key = TileKey {
                     slice: self.slice,
                     offset: TileOffset::new(x, y),
                 };
                 let tile = self.tiles.get_mut(&key).expect("bug: no tile");
 
-                // Check if this tile becomes opaque due to this primitive.
-                if !tile.is_opaque {
-                    if let Some(ref opaque_rect) = opaque_rect {
-                        if opaque_rect.contains_rect(&tile.clipped_rect) {
-                            tile.is_opaque = true;
-                        }
-                    }
-                }
-
                 // Mark if the tile is cacheable at all.
                 tile.is_same_content &= is_cacheable;
 
                 // Include any image keys this tile depends on.
                 tile.descriptor.image_keys.extend_from_slice(&image_keys);
 
                 // // Include any opacity bindings this primitive depends on.
                 tile.descriptor.opacity_bindings.extend_from_slice(&opacity_bindings);
@@ -1104,23 +1124,20 @@ impl TileCacheInstance {
         gpu_cache: &mut GpuCache,
         frame_context: &FrameVisibilityContext,
         scratch: &mut PrimitiveScratchBuffer,
     ) {
         self.tiles_to_draw.clear();
         self.dirty_region.clear();
         let mut dirty_region_index = 0;
 
-        // Track if all tiles are detected as opaque. Only when this occurs will
-        // we allow subpixel AA on this surface.
-        self.is_opaque = true;
-
         // Step through each tile and invalidate if the dependencies have changed.
         for (key, tile) in self.tiles.iter_mut() {
-            self.is_opaque &= tile.is_opaque;
+            // Check if this tile can be considered opaque.
+            tile.is_opaque = self.opaque_rect.contains_rect(&tile.clipped_rect);
 
             // Update tile transforms
             let mut transform_spatial_nodes: Vec<SpatialNodeIndex> = tile.transforms.drain().collect();
             transform_spatial_nodes.sort();
             for spatial_node_index in transform_spatial_nodes {
                 // Note: this is the only place where we don't know beforehand if the tile-affecting
                 // spatial node is below or above the current picture.
                 let transform = if self.spatial_node_index >= spatial_node_index {
@@ -2443,26 +2460,26 @@ impl PicturePrimitive {
         }
 
         // Disallow subpixel AA if an intermediate surface is needed.
         // TODO(lsalzman): allow overriding parent if intermediate surface is opaque
         let (is_passthrough, subpixel_mode) = match self.raster_config {
             Some(RasterConfig { ref composite_mode, .. }) => {
                 let subpixel_mode = match composite_mode {
                     PictureCompositeMode::TileCache { .. } => {
-                        // TODO(gw): Maintaining old behaviour, we know that any slice
-                        //           that was created is allowed to have subpixel text on it.
-                        //           Once we enable multiple slices, we'll need to handle this
-                        //           condition properly.
-                        SubpixelMode::Allow
+                        self.tile_cache.as_ref().unwrap().subpixel_mode
                     }
                     PictureCompositeMode::Blit(..) |
                     PictureCompositeMode::ComponentTransferFilter(..) |
                     PictureCompositeMode::Filter(..) |
                     PictureCompositeMode::MixBlend(..) => {
+                        // TODO(gw): We can take advantage of the same logic that
+                        //           exists in the opaque rect detection for tile
+                        //           caches, to allow subpixel text on other surfaces
+                        //           that can be detected as opaque.
                         SubpixelMode::Deny
                     }
                 };
 
                 (false, subpixel_mode)
             }
             None => {
                 (true, SubpixelMode::Allow)