From d18d440e7284cbd1c1fabb03e5f27bb66479694c Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Tue, 10 Mar 2020 09:51:49 +0000 Subject: [PATCH] Bug 1620147 - Fix virtual surface coords being outside bounds. r=Bert,sotaro This adds support for tracking and invalidating tiles based on a movable virtual offset. Differential Revision: https://phabricator.services.mozilla.com/D65687 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/2a28c94f3db2355dd50815d70f2ad6966839326c --- webrender/src/composite.rs | 23 ++++++++++ webrender/src/lib.rs | 3 +- webrender/src/picture.rs | 77 ++++++++++++++++++++++++++++++++- webrender/src/renderer.rs | 23 ++++++++-- webrender/src/resource_cache.rs | 2 + webrender/src/scene_building.rs | 3 ++ 6 files changed, 125 insertions(+), 6 deletions(-) diff --git a/webrender/src/composite.rs b/webrender/src/composite.rs index a65a38f457..47880abac6 100644 --- a/webrender/src/composite.rs +++ b/webrender/src/composite.rs @@ -27,6 +27,7 @@ use std::{ops, u64}; pub enum NativeSurfaceOperationDetails { CreateSurface { id: NativeSurfaceId, + virtual_offset: DeviceIntPoint, tile_size: DeviceIntSize, is_opaque: bool, }, @@ -213,6 +214,8 @@ pub enum CompositorKind { Native { /// Maximum dirty rects per compositor surface. max_update_rects: usize, + /// The virtual surface size used by underlying platform. + virtual_surface_size: i32, }, } @@ -225,6 +228,15 @@ impl Default for CompositorKind { } } +impl CompositorKind { + pub fn get_virtual_surface_size(&self) -> i32 { + match self { + CompositorKind::Draw { .. } => 0, + CompositorKind::Native { virtual_surface_size, .. } => *virtual_surface_size, + } + } +} + /// Information about an opaque surface used to occlude tiles. #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] @@ -651,6 +663,11 @@ pub struct NativeSurfaceInfo { pub fbo_id: u32, } +#[repr(C)] +pub struct CompositorCapabilities { + pub virtual_surface_size: i32, +} + /// Defines an interface to a native (OS level) compositor. If supplied /// by the client application, then picture cache slices will be /// composited by the OS compositor, rather than drawn via WR batches. @@ -659,6 +676,7 @@ pub trait Compositor { fn create_surface( &mut self, id: NativeSurfaceId, + virtual_offset: DeviceIntPoint, tile_size: DeviceIntSize, is_opaque: bool, ); @@ -737,6 +755,11 @@ pub trait Compositor { /// Enable/disable native compositor usage fn enable_native_compositor(&mut self, enable: bool); + + /// Get the capabilities struct for this compositor. This is used to + /// specify what features a compositor supports, depending on the + /// underlying platform + fn get_capabilities(&self) -> CompositorCapabilities; } /// Return the total area covered by a set of occluders, accounting for diff --git a/webrender/src/lib.rs b/webrender/src/lib.rs index f5b4e1194f..a77069f927 100644 --- a/webrender/src/lib.rs +++ b/webrender/src/lib.rs @@ -204,7 +204,8 @@ pub extern crate api; extern crate webrender_build; #[doc(hidden)] -pub use crate::composite::{CompositorConfig, Compositor, NativeSurfaceId, NativeTileId, NativeSurfaceInfo}; +pub use crate::composite::{CompositorConfig, Compositor, CompositorCapabilities}; +pub use crate::composite::{NativeSurfaceId, NativeTileId, NativeSurfaceInfo}; pub use crate::device::{build_shader_strings, UploadMethod, VertexUsageHint, get_gl_target}; pub use crate::device::{ProgramBinary, ProgramCache, ProgramCacheObserver, FormatDesc}; pub use crate::device::Device; diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index d4e6c75853..f4faf055fe 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -109,7 +109,7 @@ use crate::debug_colors; use euclid::{vec3, Point2D, Scale, Size2D, Vector2D, Rect, Transform3D}; use euclid::approxeq::ApproxEq; use crate::filterdata::SFilterData; -use crate::frame_builder::{FrameVisibilityContext, FrameVisibilityState}; +use crate::frame_builder::{FrameBuilderConfig, FrameVisibilityContext, FrameVisibilityState}; use crate::intern::ItemUid; use crate::internal_types::{FastHashMap, FastHashSet, PlaneSplitter, Filter, PlaneSplitAnchor, TextureSource}; use crate::frame_builder::{FrameBuildingContext, FrameBuildingState, PictureState, PictureContext}; @@ -275,6 +275,8 @@ pub struct PictureCacheState { pub native_surface: Option, /// A cache of compositor surfaces that are retained between display lists pub external_native_surface_cache: FastHashMap, + /// The retained virtual offset for this slice between display lists. + virtual_offset: DeviceIntPoint, } pub struct PictureCacheRecycledAllocations { @@ -2234,6 +2236,13 @@ pub struct TileCacheInstance { frames_until_size_eval: usize, /// The current fractional offset of the cached picture fract_offset: PictureVector2D, + /// For DirectComposition, virtual surfaces don't support negative coordinates. However, + /// picture cache tile coordinates can be negative. To handle this, we apply an offset + /// to each tile in DirectComposition. We want to change this as little as possible, + /// to avoid invalidating tiles. However, if we have a picture cache tile coordinate + /// which is outside the virtual surface bounds, we must change this to allow + /// correct remapping of the coordinates passed to BeginDraw in DC. + virtual_offset: DeviceIntPoint, /// keep around the hash map used as compare_cache to avoid reallocating it each /// frame. compare_cache: FastHashMap, @@ -2264,7 +2273,10 @@ impl TileCacheInstance { background_color: Option, shared_clips: Vec, shared_clip_chain: ClipChainId, + fb_config: &FrameBuilderConfig, ) -> Self { + let virtual_surface_size = fb_config.compositor_kind.get_virtual_surface_size(); + TileCacheInstance { slice, slice_flags, @@ -2302,6 +2314,11 @@ impl TileCacheInstance { current_tile_size: DeviceIntSize::zero(), frames_until_size_eval: 0, fract_offset: PictureVector2D::zero(), + // Default to centering the virtual offset in the middle of the DC virtual surface + virtual_offset: DeviceIntPoint::new( + virtual_surface_size / 2, + virtual_surface_size / 2, + ), compare_cache: FastHashMap::default(), native_surface: None, device_position: DevicePoint::zero(), @@ -2438,6 +2455,7 @@ impl TileCacheInstance { self.current_tile_size = prev_state.current_tile_size; self.native_surface = prev_state.native_surface; self.external_native_surface_cache = prev_state.external_native_surface_cache; + self.virtual_offset = prev_state.virtual_offset; fn recycle_map( ideal_len: usize, @@ -2634,6 +2652,59 @@ impl TileCacheInstance { TileSize::new(x_tiles, y_tiles), ); + // Determine whether the current bounds of the tile grid will exceed the + // bounds of the DC virtual surface, taking into account the current + // virtual offset. If so, we need to invalidate all tiles, and set up + // a new virtual offset, centered around the current tile grid. + + if let CompositorKind::Native { virtual_surface_size, .. } = frame_context.config.compositor_kind { + // We only need to invalidate in this case if the underlying platform + // uses virtual surfaces. + if virtual_surface_size > 0 { + // Get the extremities of the tile grid after virtual offset is applied + let tx0 = self.virtual_offset.x + x0 * self.current_tile_size.width; + let ty0 = self.virtual_offset.y + y0 * self.current_tile_size.height; + let tx1 = self.virtual_offset.x + (x1+1) * self.current_tile_size.width; + let ty1 = self.virtual_offset.y + (y1+1) * self.current_tile_size.height; + + let need_new_virtual_offset = tx0 < 0 || + ty0 < 0 || + tx1 >= virtual_surface_size || + ty1 >= virtual_surface_size; + + if need_new_virtual_offset { + // Calculate a new virtual offset, centered around the middle of the + // current tile grid. This means we won't need to invalidate and get + // a new offset for a long time! + self.virtual_offset = DeviceIntPoint::new( + (virtual_surface_size/2) - ((x0 + x1) / 2) * self.current_tile_size.width, + (virtual_surface_size/2) - ((y0 + y1) / 2) * self.current_tile_size.height, + ); + + // Invalidate all native tile surfaces. They will be re-allocated next time + // they are scheduled to be rasterized. + for tile in self.tiles.values_mut() { + if let Some(TileSurface::Texture { descriptor: SurfaceTextureDescriptor::Native { ref mut id, .. }, .. }) = tile.surface { + if let Some(id) = id.take() { + frame_state.resource_cache.destroy_compositor_tile(id); + tile.surface = None; + // Invalidate the entire tile to force a redraw. + // TODO(gw): Add a new invalidation reason for virtual offset changing + tile.invalidate(None, InvalidationReason::CompositorKindChanged); + } + } + } + + // Destroy the native virtual surfaces. They will be re-allocated next time a tile + // that references them is scheduled to draw. + if let Some(native_surface) = self.native_surface.take() { + frame_state.resource_cache.destroy_compositor_surface(native_surface.opaque); + frame_state.resource_cache.destroy_compositor_surface(native_surface.alpha); + } + } + } + } + // Rebuild the tile grid if the picture cache rect has changed. if new_tile_rect != self.tile_rect { let mut old_tiles = mem::replace(&mut self.tiles, FastHashMap::default()); @@ -3064,6 +3135,7 @@ impl TileCacheInstance { // a single compositor tile that covers the entire compositor surface. let native_surface_id = resource_cache.create_compositor_surface( + DeviceIntPoint::zero(), native_surface_size, true, ); @@ -4285,6 +4357,7 @@ impl PicturePrimitive { current_tile_size: tile_cache.current_tile_size, native_surface: tile_cache.native_surface, external_native_surface_cache: tile_cache.external_native_surface_cache, + virtual_offset: tile_cache.virtual_offset, allocations: PictureCacheRecycledAllocations { old_opacity_bindings: tile_cache.old_opacity_bindings, old_color_bindings: tile_cache.old_color_bindings, @@ -4938,6 +5011,7 @@ impl PicturePrimitive { let opaque = frame_state .resource_cache .create_compositor_surface( + tile_cache.virtual_offset, tile_cache.current_tile_size, true, ); @@ -4945,6 +5019,7 @@ impl PicturePrimitive { let alpha = frame_state .resource_cache .create_compositor_surface( + tile_cache.virtual_offset, tile_cache.current_tile_size, false, ); diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index a7e1da350c..c1860cc142 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2214,8 +2214,17 @@ impl Renderer { }; let compositor_kind = match options.compositor_config { - CompositorConfig::Draw { max_partial_present_rects } => CompositorKind::Draw { max_partial_present_rects }, - CompositorConfig::Native { max_update_rects, .. } => CompositorKind::Native { max_update_rects }, + CompositorConfig::Draw { max_partial_present_rects } => { + CompositorKind::Draw { max_partial_present_rects } + } + CompositorConfig::Native { ref compositor, max_update_rects, .. } => { + let capabilities = compositor.get_capabilities(); + + CompositorKind::Native { + max_update_rects, + virtual_surface_size: capabilities.virtual_surface_size, + } + } }; let config = FrameBuilderConfig { @@ -3052,6 +3061,7 @@ impl Renderer { if self.debug_overlay_state.is_enabled && self.debug_overlay_state.current_size.is_none() { compositor.create_surface( NativeSurfaceId::DEBUG_OVERLAY, + DeviceIntPoint::zero(), framebuffer_size, false, ); @@ -5460,10 +5470,15 @@ impl Renderer { CompositorConfig::Native { ref mut compositor, .. } => { for op in self.pending_native_surface_updates.drain(..) { match op.details { - NativeSurfaceOperationDetails::CreateSurface { id, tile_size, is_opaque } => { + NativeSurfaceOperationDetails::CreateSurface { id, virtual_offset, tile_size, is_opaque } => { let _inserted = self.allocated_native_surfaces.insert(id); debug_assert!(_inserted, "bug: creating existing surface"); - compositor.create_surface(id, tile_size, is_opaque); + compositor.create_surface( + id, + virtual_offset, + tile_size, + is_opaque, + ); } NativeSurfaceOperationDetails::DestroySurface { id } => { let _existed = self.allocated_native_surfaces.remove(&id); diff --git a/webrender/src/resource_cache.rs b/webrender/src/resource_cache.rs index 096bfcd7ec..c6a2da8a6d 100644 --- a/webrender/src/resource_cache.rs +++ b/webrender/src/resource_cache.rs @@ -1783,6 +1783,7 @@ impl ResourceCache { /// specified tile size. pub fn create_compositor_surface( &mut self, + virtual_offset: DeviceIntPoint, tile_size: DeviceIntSize, is_opaque: bool, ) -> NativeSurfaceId { @@ -1792,6 +1793,7 @@ impl ResourceCache { NativeSurfaceOperation { details: NativeSurfaceOperationDetails::CreateSurface { id, + virtual_offset, tile_size, is_opaque, }, diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index 04c3ad498b..257c8c02d8 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -686,6 +686,7 @@ impl<'a> SceneBuilder<'a> { &mut self.prim_store, &mut self.clip_store, &mut self.picture_cache_spatial_nodes, + &self.config, ); main_prim_list.add_prim( @@ -4112,6 +4113,7 @@ fn create_tile_cache( prim_store: &mut PrimitiveStore, clip_store: &mut ClipStore, picture_cache_spatial_nodes: &mut FastHashSet, + frame_builder_config: &FrameBuilderConfig, ) -> PrimitiveInstance { // Add this spatial node to the list to check for complex transforms // at the start of a frame build. @@ -4163,6 +4165,7 @@ fn create_tile_cache( background_color, shared_clips, parent_clip_chain_id, + frame_builder_config, )); let pic_index = prim_store.pictures.alloc().init(PicturePrimitive::new_image(