From 6b11aea70d1da1aa922a715ec1a54cc8efdc633b Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Sat, 9 Nov 2019 09:51:44 +0000 Subject: [PATCH 01/12] Bug 1595036 - Put GPU debug markers behind a pref r=kvark Differential Revision: https://phabricator.services.mozilla.com/D52339 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/093ad83d0ab40951603cb3df46dc29877d51ca75 --- webrender/src/renderer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 53f2bcd05d..9a609580e5 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2306,7 +2306,10 @@ impl Renderer { } })?; - let debug_support = if device.supports_extension("GL_KHR_debug") { + let debug_method = if !options.enable_gpu_markers { + // The GPU markers are disabled. + GpuDebugMethod::None + } else if device.supports_extension("GL_KHR_debug") { GpuDebugMethod::KHR } else if device.supports_extension("GL_EXT_debug_marker") { GpuDebugMethod::MarkerEXT @@ -2314,9 +2317,9 @@ impl Renderer { GpuDebugMethod::None }; - info!("using {:?}", debug_support); + info!("using {:?}", debug_method); - let gpu_profile = GpuProfiler::new(Rc::clone(device.rc_gl()), debug_support); + let gpu_profile = GpuProfiler::new(Rc::clone(device.rc_gl()), debug_method); #[cfg(feature = "capture")] let read_fbo = device.create_fbo(); @@ -6164,6 +6167,7 @@ pub struct RendererOptions { pub surface_origin_is_top_left: bool, /// The configuration options defining how WR composites the final scene. pub compositor_config: CompositorConfig, + pub enable_gpu_markers: bool, } impl Default for RendererOptions { @@ -6218,6 +6222,7 @@ impl Default for RendererOptions { dump_shader_source: None, surface_origin_is_top_left: false, compositor_config: CompositorConfig::default(), + enable_gpu_markers: true, } } } From df363ba1aff937a3b5ce183f701adfcc7bd42d69 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Sun, 10 Nov 2019 10:02:20 +0000 Subject: [PATCH 02/12] Bug 1593970 - scale subpixel contribution based on ClearType level setting. r=jfkthame Differential Revision: https://phabricator.services.mozilla.com/D52441 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/d967ac6c56aeec99c2fba3814c8550a1c5971384 --- webrender/src/gamma_lut.rs | 27 ++++++++++++++++++++++++++ webrender/src/platform/windows/font.rs | 12 +++++++++--- webrender_api/src/font.rs | 4 +++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/webrender/src/gamma_lut.rs b/webrender/src/gamma_lut.rs index b4005515d5..b5df7029e6 100644 --- a/webrender/src/gamma_lut.rs +++ b/webrender/src/gamma_lut.rs @@ -311,6 +311,33 @@ impl GammaLut { } } + // Assumes pixels are in BGRA format. Assumes pixel values are in linear space already. + pub fn preblend_scaled(&self, pixels: &mut [u8], color: ColorU, percent: u8) { + if percent >= 100 { + self.preblend(pixels, color); + return; + } + + let table_r = self.get_table(color.r); + let table_g = self.get_table(color.g); + let table_b = self.get_table(color.b); + let scale = (percent as i32 * 256) / 100; + + for pixel in pixels.chunks_mut(4) { + let (mut b, g, mut r) = ( + table_b[pixel[0] as usize] as i32, + table_g[pixel[1] as usize] as i32, + table_r[pixel[2] as usize] as i32, + ); + b = g + (((b - g) * scale) >> 8); + r = g + (((r - g) * scale) >> 8); + pixel[0] = b as u8; + pixel[1] = g as u8; + pixel[2] = r as u8; + pixel[3] = max(max(b, g), r) as u8; + } + } + #[cfg(target_os="macos")] pub fn coregraphics_convert_to_linear(&self, pixels: &mut [u8]) { for pixel in pixels.chunks_mut(4) { diff --git a/webrender/src/platform/windows/font.rs b/webrender/src/platform/windows/font.rs index ae082cbf67..03f45193a5 100644 --- a/webrender/src/platform/windows/font.rs +++ b/webrender/src/platform/windows/font.rs @@ -72,7 +72,7 @@ struct FontFace { pub struct FontContext { fonts: FastHashMap, variations: FastHashMap<(FontKey, dwrote::DWRITE_FONT_SIMULATIONS, Vec), dwrote::FontFace>, - gamma_luts: FastHashMap<(u16, u16), GammaLut>, + gamma_luts: FastHashMap<(u16, u8), GammaLut>, } // DirectWrite is safe to use on multiple threads and non-shareable resources are @@ -552,7 +552,8 @@ impl FontContext { let mut bgra_pixels = self.convert_to_bgra(&pixels, texture_type, font.render_mode, bitmaps, font.flags.contains(FontInstanceFlags::SUBPIXEL_BGR)); - let FontInstancePlatformOptions { gamma, contrast, .. } = font.platform_options.unwrap_or_default(); + let FontInstancePlatformOptions { gamma, contrast, cleartype_level, .. } = + font.platform_options.unwrap_or_default(); let gamma_lut = self.gamma_luts .entry((gamma, contrast)) .or_insert_with(|| @@ -561,7 +562,12 @@ impl FontContext { gamma as f32 / 100.0, gamma as f32 / 100.0, )); - gamma_lut.preblend(&mut bgra_pixels, font.color); + if bitmaps || texture_type == dwrote::DWRITE_TEXTURE_ALIASED_1x1 || + font.render_mode != FontRenderMode::Subpixel { + gamma_lut.preblend(&mut bgra_pixels, font.color); + } else { + gamma_lut.preblend_scaled(&mut bgra_pixels, font.color, cleartype_level); + } let format = if bitmaps { GlyphFormat::Bitmap diff --git a/webrender_api/src/font.rs b/webrender_api/src/font.rs index 528e1d3277..2774fab671 100644 --- a/webrender_api/src/font.rs +++ b/webrender_api/src/font.rs @@ -288,7 +288,8 @@ impl Default for FontInstanceOptions { #[derive(Clone, Copy, Debug, Deserialize, Hash, Eq, MallocSizeOf, PartialEq, PartialOrd, Ord, Serialize)] pub struct FontInstancePlatformOptions { pub gamma: u16, // percent - pub contrast: u16, // percent + pub contrast: u8, // percent + pub cleartype_level: u8, // percent } #[cfg(target_os = "windows")] @@ -297,6 +298,7 @@ impl Default for FontInstancePlatformOptions { FontInstancePlatformOptions { gamma: 180, // Default DWrite gamma contrast: 100, + cleartype_level: 100, } } } From 8d803121eea6e250d3810bcc3c05c9f43842844e Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Mon, 11 Nov 2019 21:59:30 +0000 Subject: [PATCH 03/12] Bug 1594644 - Add debugging infrastructure for picture cache invalidation. r=nical Add a first pass at invalidation debugging infrastructure. Also tidy up the locations that invalidate into a common method, that sets the invalidation reason. Differential Revision: https://phabricator.services.mozilla.com/D52128 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/260dfcecdb6287707bd90040a4d2bf7988274ef1 --- webrender/src/intern.rs | 5 + webrender/src/picture.rs | 286 ++++++++++++++++++++++++++++++++------- webrender_api/src/api.rs | 2 + wrench/src/main.rs | 5 + 4 files changed, 252 insertions(+), 46 deletions(-) diff --git a/webrender/src/intern.rs b/webrender/src/intern.rs index 3676b50fac..2e436c1b03 100644 --- a/webrender/src/intern.rs +++ b/webrender/src/intern.rs @@ -74,6 +74,11 @@ impl ItemUid { let uid = NEXT_UID.fetch_add(1, Ordering::Relaxed); ItemUid { uid } } + + // Intended for debug usage only + pub fn get_uid(&self) -> usize { + self.uid + } } #[cfg_attr(feature = "capture", derive(Serialize))] diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index bd103bd3ac..be9e11dc47 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -94,7 +94,7 @@ use crate::prim_store::{SpaceMapper, PrimitiveVisibilityMask, PointKey, Primitiv use crate::prim_store::{SpaceSnapper, PictureIndex, PrimitiveInstance, PrimitiveInstanceKind}; use crate::prim_store::{get_raster_rects, PrimitiveScratchBuffer, RectangleKey}; use crate::prim_store::{OpacityBindingStorage, ImageInstanceStorage, OpacityBindingIndex}; -use crate::print_tree::PrintTreePrinter; +use crate::print_tree::{PrintTree, PrintTreePrinter}; use crate::render_backend::DataStores; use crate::render_task_graph::RenderTaskId; use crate::render_target::RenderTargetKind; @@ -517,6 +517,50 @@ impl TileSurface { } } +/// The result of a primitive dependency comparison. Size is a u8 +/// since this is a hot path in the code, and keeping the data small +/// is a performance win. +#[derive(Debug, Copy, Clone, PartialEq)] +#[repr(u8)] +enum PrimitiveCompareResult { + /// Primitives match + Equal, + /// Something in the PrimitiveDescriptor was different + Descriptor, + /// The clip node content or spatial node changed + Clip, + /// The value of the transform changed + Transform, + /// An image dependency was dirty + Image, + /// The value of an opacity binding changed + OpacityBinding, +} + +/// Debugging information about why a tile was invalidated +#[derive(Debug)] +enum InvalidationReason { + /// The fractional offset changed + FractionalOffset, + /// The background color changed + BackgroundColor, + /// Tile was not cacheable (e.g. video element) + NonCacheable, + /// The opaque state of the backing native surface changed + SurfaceOpacityChanged, + /// There was no backing texture (evicted or never rendered) + NoTexture, + /// There was no backing native surface (never rendered, or recreated) + NoSurface, + /// The primitive count in the dependency list was different + PrimCount, + /// The content of one of the primitives was different + Content { + /// What changed in the primitive that was different + prim_compare_result: PrimitiveCompareResult, + }, +} + /// Information about a cached tile. pub struct Tile { /// The current world rect of this tile. @@ -543,8 +587,6 @@ pub struct Tile { /// all tiles need to be invalidated and redrawn, since snapping differences are /// likely to occur. fract_offset: PictureVector2D, - /// If true, the content on this tile is the same as last frame. - is_same_content: bool, /// The tile id is stable between display lists and / or frames, /// if the tile is retained. Useful for debugging tile evictions. pub id: TileId, @@ -561,6 +603,8 @@ pub struct Tile { pub world_dirty_rect: WorldRect, /// The last rendered background color on this tile. background_color: Option, + /// The first reason the tile was invalidated this frame. + invalidation_reason: Option, } impl Tile { @@ -575,7 +619,6 @@ impl Tile { surface: None, current_descriptor: TileDescriptor::new(), prev_descriptor: TileDescriptor::new(), - is_same_content: false, is_valid: false, is_visible: false, fract_offset: PictureVector2D::zero(), @@ -585,16 +628,29 @@ impl Tile { dirty_rect: PictureRect::zero(), world_dirty_rect: WorldRect::zero(), background_color: None, + invalidation_reason: None, } } + /// Print debug information about this tile to a tree printer. + fn print(&self, pt: &mut dyn PrintTreePrinter) { + pt.new_level(format!("Tile {:?}", self.id)); + pt.add_item(format!("rect: {}", self.rect)); + pt.add_item(format!("fract_offset: {:?}", self.fract_offset)); + pt.add_item(format!("background_color: {:?}", self.background_color)); + pt.add_item(format!("invalidation_reason: {:?}", self.invalidation_reason)); + self.current_descriptor.print(pt); + pt.end_level(); + } + /// Check if the content of the previous and current tile descriptors match fn update_dirty_rects( &mut self, ctx: &TilePostUpdateContext, state: &TilePostUpdateState, - compare_cache: &mut FastHashMap, - ) { + compare_cache: &mut FastHashMap, + invalidation_reason: &mut Option, + ) -> PictureRect { let mut prim_comparer = PrimitiveComparer::new( &self.prev_descriptor, &self.current_descriptor, @@ -603,13 +659,18 @@ impl Tile { ctx.opacity_bindings, ); + let mut dirty_rect = PictureRect::zero(); + self.root.update_dirty_rects( &self.prev_descriptor.prims, &self.current_descriptor.prims, &mut prim_comparer, - &mut self.dirty_rect, + &mut dirty_rect, compare_cache, + invalidation_reason, ); + + dirty_rect } /// Invalidate a tile based on change in content. This @@ -624,9 +685,42 @@ impl Tile { // Check if the contents of the primitives, clips, and // other dependencies are the same. let mut compare_cache = FastHashMap::default(); - self.update_dirty_rects(ctx, state, &mut compare_cache); - self.is_same_content &= self.dirty_rect.is_empty(); - self.is_valid &= self.is_same_content; + let mut invalidation_reason = None; + let dirty_rect = self.update_dirty_rects( + ctx, + state, + &mut compare_cache, + &mut invalidation_reason, + ); + if !dirty_rect.is_empty() { + self.invalidate( + Some(dirty_rect), + invalidation_reason.expect("bug: no invalidation_reason"), + ); + } + } + + /// Invalidate this tile. If `invalidation_rect` is None, the entire + /// tile is invalidated. + fn invalidate( + &mut self, + invalidation_rect: Option, + reason: InvalidationReason, + ) { + self.is_valid = false; + + match invalidation_rect { + Some(rect) => { + self.dirty_rect = self.dirty_rect.union(&rect); + } + None => { + self.dirty_rect = self.rect; + } + } + + if self.invalidation_reason.is_none() { + self.invalidation_reason = Some(reason); + } } /// Called during pre_update of a tile cache instance. Allows the @@ -637,6 +731,7 @@ impl Tile { ctx: &TilePreUpdateContext, ) { self.rect = rect; + self.invalidation_reason = None; self.clipped_rect = self.rect .intersection(&ctx.local_rect) @@ -657,23 +752,18 @@ impl Tile { return; } - // Start frame assuming that the tile has the same content. - self.is_same_content = true; - // Determine if the fractional offset of the transform is different this frame // from the currently cached tile set. let fract_changed = (self.fract_offset.x - ctx.fract_offset.x).abs() > 0.001 || (self.fract_offset.y - ctx.fract_offset.y).abs() > 0.001; if fract_changed { + self.invalidate(None, InvalidationReason::FractionalOffset); self.fract_offset = ctx.fract_offset; } - // If the fractional offset of the transform root changed, or tthe background - // color of this tile changed, invalidate the whole thing. - if fract_changed || ctx.background_color != self.background_color { + if ctx.background_color != self.background_color { + self.invalidate(None, InvalidationReason::BackgroundColor); self.background_color = ctx.background_color; - self.is_same_content = false; - self.dirty_rect = rect; } // Clear any dependencies so that when we rebuild them we @@ -699,8 +789,7 @@ impl Tile { // Mark if the tile is cacheable at all. if !info.is_cacheable { - self.is_same_content = false; - self.dirty_rect = self.dirty_rect.union(&info.prim_rect); + self.invalidate(Some(info.prim_rect), InvalidationReason::NonCacheable); } // Include any image keys this tile depends on. @@ -882,8 +971,7 @@ impl Tile { if let SurfaceTextureDescriptor::NativeSurface { ref mut id, .. } = descriptor { // Reset the dirty rect and tile validity in this case, to // force the new tile to be completely redrawn. - self.dirty_rect = self.rect; - self.is_valid = false; + self.invalidate(None, InvalidationReason::SurfaceOpacityChanged); // If this tile has a currently allocated native surface, destroy it. It // will be re-allocated next time it's determined to be visible. @@ -1102,6 +1190,69 @@ impl TileDescriptor { } } + /// Print debug information about this tile descriptor to a tree printer. + fn print(&self, pt: &mut dyn PrintTreePrinter) { + pt.new_level("current_descriptor".to_string()); + + pt.new_level("prims".to_string()); + for prim in &self.prims { + pt.new_level(format!("prim uid={}", prim.prim_uid.get_uid())); + pt.add_item(format!("origin: {},{}", prim.origin.x, prim.origin.y)); + pt.add_item(format!("clip: origin={},{} size={}x{}", + prim.prim_clip_rect.x, + prim.prim_clip_rect.y, + prim.prim_clip_rect.w, + prim.prim_clip_rect.h, + )); + pt.add_item(format!("deps: t={} i={} o={} c={}", + prim.transform_dep_count, + prim.image_dep_count, + prim.opacity_binding_dep_count, + prim.clip_dep_count, + )); + pt.end_level(); + } + pt.end_level(); + + if !self.clips.is_empty() { + pt.new_level("clips".to_string()); + for clip in &self.clips { + pt.new_level(format!("clip uid={}", clip.get_uid())); + pt.end_level(); + } + pt.end_level(); + } + + if !self.image_keys.is_empty() { + pt.new_level("image_keys".to_string()); + for key in &self.image_keys { + pt.new_level(format!("key={:?}", key)); + pt.end_level(); + } + pt.end_level(); + } + + if !self.opacity_bindings.is_empty() { + pt.new_level("opacity_bindings".to_string()); + for opacity_binding in &self.opacity_bindings { + pt.new_level(format!("binding={:?}", opacity_binding)); + pt.end_level(); + } + pt.end_level(); + } + + if !self.transforms.is_empty() { + pt.new_level("transforms".to_string()); + for transform in &self.transforms { + pt.new_level(format!("spatial_node={:?}", transform)); + pt.end_level(); + } + pt.end_level(); + } + + pt.end_level(); + } + /// Clear the dependency information for a tile, when the dependencies /// are being rebuilt. fn clear(&mut self) { @@ -1341,6 +1492,8 @@ pub struct TileCacheInstance { /// we don't want to constantly invalidate and reallocate different tile size /// configuration each frame. frames_until_size_eval: usize, + /// The current fractional offset of the cached picture + fract_offset: PictureVector2D, } impl TileCacheInstance { @@ -1379,6 +1532,7 @@ impl TileCacheInstance { shared_clip_chain, current_tile_size: DeviceIntSize::zero(), frames_until_size_eval: 0, + fract_offset: PictureVector2D::zero(), } } @@ -1560,7 +1714,7 @@ impl TileCacheInstance { .origin; // Extract the fractional offset required in picture space to align in device space - let fract_offset = PictureVector2D::new( + self.fract_offset = PictureVector2D::new( ref_point.x.fract(), ref_point.y.fract(), ); @@ -1640,7 +1794,7 @@ impl TileCacheInstance { local_rect: self.local_rect, local_clip_rect: self.local_clip_rect, pic_to_world_mapper, - fract_offset, + fract_offset: self.fract_offset, background_color: self.background_color, global_screen_world_rect: frame_context.global_screen_world_rect, }; @@ -1661,8 +1815,8 @@ impl TileCacheInstance { // the snapping will be consistent. let rect = PictureRect::new( PicturePoint::new( - x as f32 * self.tile_size.width + fract_offset.x, - y as f32 * self.tile_size.height + fract_offset.y, + x as f32 * self.tile_size.width + self.fract_offset.x, + y as f32 * self.tile_size.height + self.fract_offset.y, ), self.tile_size, ); @@ -1969,6 +2123,31 @@ impl TileCacheInstance { true } + /// Print debug information about this picture cache to a tree printer. + fn print(&self) { + // TODO(gw): This initial implementation is very basic - just printing + // the picture cache state to stdout. In future, we can + // make this dump each frame to a file, and produce a report + // stating which frames had invalidations. This will allow + // diff'ing the invalidation states in a visual tool. + let mut pt = PrintTree::new("Picture Cache"); + + pt.new_level(format!("Slice {}", self.slice)); + + pt.add_item(format!("fract_offset: {:?}", self.fract_offset)); + pt.add_item(format!("background_color: {:?}", self.background_color)); + + for y in self.tile_bounds_p0.y .. self.tile_bounds_p1.y { + for x in self.tile_bounds_p0.x .. self.tile_bounds_p1.x { + let key = TileOffset::new(x, y); + let tile = &self.tiles[&key]; + tile.print(&mut pt); + } + } + + pt.end_level(); + } + /// Apply any updates after prim dependency updates. This applies /// any late tile invalidations, and sets up the dirty rect and /// set of tile blits. @@ -3328,8 +3507,6 @@ impl PicturePrimitive { } }; - let surface = tile.surface.as_mut().expect("no tile surface set!"); - // If that draw rect is occluded by some set of tiles in front of it, // then mark it as not visible and skip drawing. When it's not occluded // it will fail this test, and get rasterized by the render task setup @@ -3339,6 +3516,8 @@ impl PicturePrimitive { // occluded. We will need to re-allocate this surface if it becomes visible, // but that's likely to be rare (e.g. when there is no content display list // for a frame or two during a tab switch). + let surface = tile.surface.as_mut().expect("no tile surface set!"); + if let TileSurface::Texture { descriptor: SurfaceTextureDescriptor::NativeSurface { id, .. }, .. } = surface { if let Some(id) = id.take() { frame_state.composite_state.destroy_surface(id); @@ -3370,6 +3549,8 @@ impl PicturePrimitive { 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!"); + scratch.push_debug_string( tile_device_rect.origin + label_offset, debug_colors::RED, @@ -3383,7 +3564,7 @@ impl PicturePrimitive { } } - if let TileSurface::Texture { descriptor, .. } = surface { + if let TileSurface::Texture { descriptor, .. } = tile.surface.as_mut().unwrap() { match descriptor { SurfaceTextureDescriptor::TextureCache { ref handle, .. } => { // Invalidate if the backing texture was evicted. @@ -3400,15 +3581,13 @@ impl PicturePrimitive { } else { // If the texture was evicted on a previous frame, we need to assume // that the entire tile rect is dirty. - tile.is_valid = false; - tile.dirty_rect = tile.rect; + tile.invalidate(None, InvalidationReason::NoTexture); } } SurfaceTextureDescriptor::NativeSurface { id, .. } => { if id.is_none() { // There is no current surface allocation, so ensure the entire tile is invalidated - tile.is_valid = false; - tile.dirty_rect = tile.rect; + tile.invalidate(None, InvalidationReason::NoSurface); } } } @@ -3422,7 +3601,7 @@ impl PicturePrimitive { } // Ensure that this texture is allocated. - if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = surface { + if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = tile.surface.as_mut().unwrap() { match descriptor { SurfaceTextureDescriptor::TextureCache { ref mut handle } => { if !frame_state.resource_cache.texture_cache.is_allocated(handle) { @@ -3525,6 +3704,11 @@ impl PicturePrimitive { tile.is_valid = true; } + // If invalidation debugging is enabled, dump the picture cache state to a tree printer. + if frame_context.debug_flags.contains(DebugFlags::INVALIDATION_DBG) { + tile_cache.print(); + } + None } PictureCompositeMode::MixBlend(..) | @@ -4434,18 +4618,18 @@ impl<'a> PrimitiveComparer<'a> { } /// Check if two primitive descriptors are the same. - fn is_prim_same( + fn compare_prim( &mut self, prev: &PrimitiveDescriptor, curr: &PrimitiveDescriptor, - ) -> bool { + ) -> PrimitiveCompareResult { let resource_cache = self.resource_cache; let spatial_nodes = self.spatial_nodes; let opacity_bindings = self.opacity_bindings; // Check equality of the PrimitiveDescriptor if prev != curr { - return false; + return PrimitiveCompareResult::Descriptor; } // Check if any of the clips this prim has are different. @@ -4456,7 +4640,7 @@ impl<'a> PrimitiveComparer<'a> { false } ) { - return false; + return PrimitiveCompareResult::Clip; } // Check if any of the transforms this prim has are different. @@ -4467,7 +4651,7 @@ impl<'a> PrimitiveComparer<'a> { spatial_nodes[curr].changed } ) { - return false; + return PrimitiveCompareResult::Transform; } // Check if any of the images this prim has are different. @@ -4478,7 +4662,7 @@ impl<'a> PrimitiveComparer<'a> { resource_cache.is_image_dirty(*curr) } ) { - return false; + return PrimitiveCompareResult::Image; } // Check if any of the opacity bindings this prim has are different. @@ -4497,10 +4681,10 @@ impl<'a> PrimitiveComparer<'a> { false } ) { - return false; + return PrimitiveCompareResult::OpacityBinding; } - true + PrimitiveCompareResult::Equal } } @@ -4817,7 +5001,8 @@ impl TileNode { curr_prims: &[PrimitiveDescriptor], prim_comparer: &mut PrimitiveComparer, dirty_rect: &mut PictureRect, - compare_cache: &mut FastHashMap, + compare_cache: &mut FastHashMap, + invalidation_reason: &mut Option, ) { match self.kind { TileNodeKind::Node { ref mut children, .. } => { @@ -4828,6 +5013,7 @@ impl TileNode { prim_comparer, dirty_rect, compare_cache, + invalidation_reason, ); } } @@ -4859,16 +5045,21 @@ impl TileNode { curr_index: *curr_index, }; - let is_prim_same = *compare_cache + let prim_compare_result = *compare_cache .entry(key) .or_insert_with(|| { let prev = &prev_prims[i0]; let curr = &curr_prims[i1]; - prim_comparer.is_prim_same(prev, curr) + prim_comparer.compare_prim(prev, curr) }); // If not the same, mark this node as dirty and update the dirty rect - if !is_prim_same { + if prim_compare_result != PrimitiveCompareResult::Equal { + if invalidation_reason.is_none() { + *invalidation_reason = Some(InvalidationReason::Content { + prim_compare_result, + }); + } *dirty_rect = self.rect.union(dirty_rect); *dirty_tracker = *dirty_tracker | 1; break; @@ -4878,6 +5069,9 @@ impl TileNode { prev_i1 = i1; } } else { + if invalidation_reason.is_none() { + *invalidation_reason = Some(InvalidationReason::PrimCount); + } *dirty_rect = self.rect.union(dirty_rect); *dirty_tracker = *dirty_tracker | 1; } diff --git a/webrender_api/src/api.rs b/webrender_api/src/api.rs index 0e60b47415..434b768de3 100644 --- a/webrender_api/src/api.rs +++ b/webrender_api/src/api.rs @@ -1147,6 +1147,8 @@ bitflags! { const SMART_PROFILER = 1 << 26; /// Dynamically control whether picture caching is enabled. const DISABLE_PICTURE_CACHING = 1 << 27; + /// If set, dump picture cache invalidation debug to console. + const INVALIDATION_DBG = 1 << 28; } } diff --git a/wrench/src/main.rs b/wrench/src/main.rs index 390d53011f..ec66fcd2cd 100644 --- a/wrench/src/main.rs +++ b/wrench/src/main.rs @@ -710,6 +710,11 @@ fn render<'a>( wrench.api.send_debug_cmd(DebugCommand::SetFlags(debug_flags)); do_render = true; } + VirtualKeyCode::B => { + debug_flags.toggle(DebugFlags::INVALIDATION_DBG); + wrench.api.send_debug_cmd(DebugCommand::SetFlags(debug_flags)); + do_render = true; + } VirtualKeyCode::P => { debug_flags.toggle(DebugFlags::PROFILER_DBG); wrench.api.send_debug_cmd(DebugCommand::SetFlags(debug_flags)); From 462772f402ea4867f818ff1e5491fdf2dd5e26b0 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Mon, 11 Nov 2019 21:59:50 +0000 Subject: [PATCH 04/12] Bug 1593845 - Ensure picture cache dirty rect is clamped to tile rect. r=sotaro Differential Revision: https://phabricator.services.mozilla.com/D52493 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/e05f10f48f524d429d9eade6ee8d7001ab32061f --- webrender/src/picture.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index be9e11dc47..d062c5e8b1 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -924,6 +924,11 @@ impl Tile { self.dirty_rect = self.rect; } + // Ensure that the dirty rect doesn't extend outside the local tile rect. + self.dirty_rect = self.dirty_rect + .intersection(&self.rect) + .unwrap_or(PictureRect::zero()); + // 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. // TODO(gw): Initial native compositor interface doesn't support simple From 3e0e33ae5fe48530d3972d7a58f7351ad7f6e8f7 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Mon, 11 Nov 2019 21:59:58 +0000 Subject: [PATCH 05/12] Bug 1594934 - Handle clip rects in determining picture cache primitive dependencies. r=nical This reduces the amount of invalidation on some pages very significantly. It also fixes a number of cases where picture caching "fails", resulting in everything invalidating every frame. Differential Revision: https://phabricator.services.mozilla.com/D52275 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/afa3fdcab69155f37dc35daa0cdb22dbb849b907 --- webrender/src/picture.rs | 106 ++++++++++++++++++++------------ webrender/src/prim_store/mod.rs | 1 + 2 files changed, 67 insertions(+), 40 deletions(-) diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index d062c5e8b1..ca14d01910 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -380,8 +380,8 @@ struct PrimitiveDependencyInfo { /// Unique content identifier of the primitive. prim_uid: ItemUid, - /// The (conservative) area in picture space this primitive occupies. - prim_rect: PictureRect, + /// The picture space origin of this primitive. + prim_origin: PicturePoint, /// The (conservative) clipped area in picture space this primitive occupies. prim_clip_rect: PictureRect, @@ -403,17 +403,18 @@ impl PrimitiveDependencyInfo { /// Construct dependency info for a new primitive. fn new( prim_uid: ItemUid, - prim_rect: PictureRect, + prim_origin: PicturePoint, + prim_clip_rect: PictureRect, is_cacheable: bool, ) -> Self { PrimitiveDependencyInfo { prim_uid, - prim_rect, + prim_origin, is_cacheable, image_keys: SmallVec::new(), opacity_bindings: SmallVec::new(), clip_by_tile: false, - prim_clip_rect: PictureRect::zero(), + prim_clip_rect, clips: SmallVec::new(), spatial_nodes: SmallVec::new(), } @@ -789,7 +790,7 @@ impl Tile { // Mark if the tile is cacheable at all. if !info.is_cacheable { - self.invalidate(Some(info.prim_rect), InvalidationReason::NonCacheable); + self.invalidate(Some(info.prim_clip_rect), InvalidationReason::NonCacheable); } // Include any image keys this tile depends on. @@ -823,8 +824,8 @@ impl Tile { ( PicturePoint::new( - clampf(info.prim_rect.origin.x, tile_p0.x, tile_p1.x), - clampf(info.prim_rect.origin.y, tile_p0.y, tile_p1.y), + clampf(info.prim_origin.x, tile_p0.x, tile_p1.x), + clampf(info.prim_origin.y, tile_p0.y, tile_p1.y), ), PictureRect::new( clip_p0, @@ -835,7 +836,7 @@ impl Tile { ), ) } else { - (info.prim_rect.origin, info.prim_clip_rect) + (info.prim_origin, info.prim_clip_rect) }; // Update the tile descriptor, used for tile comparison during scene swaps. @@ -859,7 +860,7 @@ impl Tile { }); // Add this primitive to the dirty rect quadtree. - self.root.add_prim(prim_index, &info.prim_rect); + self.root.add_prim(prim_index, &info.prim_clip_rect); } /// Called during tile cache instance post_update. Allows invalidation and dirty @@ -1445,6 +1446,8 @@ pub struct TileCacheInstance { pub tiles: FastHashMap, /// A helper struct to map local rects into surface coords. map_local_to_surface: SpaceMapper, + /// A helper struct to map child picture rects into picture cache surface coords. + map_child_pic_to_surface: SpaceMapper, /// List of opacity bindings, with some extra information /// about whether they changed since last frame. opacity_bindings: FastHashMap, @@ -1517,6 +1520,10 @@ impl TileCacheInstance { ROOT_SPATIAL_NODE_INDEX, PictureRect::zero(), ), + map_child_pic_to_surface: SpaceMapper::new( + ROOT_SPATIAL_NODE_INDEX, + PictureRect::zero(), + ), opacity_bindings: FastHashMap::default(), spatial_nodes: FastHashMap::default(), used_spatial_nodes: FastHashSet::default(), @@ -1597,6 +1604,10 @@ impl TileCacheInstance { self.spatial_node_index, PictureRect::from_untyped(&pic_rect.to_untyped()), ); + self.map_child_pic_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, @@ -1877,7 +1888,15 @@ impl TileCacheInstance { opacity_binding_store: &OpacityBindingStorage, image_instances: &ImageInstanceStorage, surface_index: SurfaceIndex, + surface_spatial_node_index: SpatialNodeIndex, ) -> bool { + // If the primitive is completely clipped out by the clip chain, there + // is no need to add it to any primitive dependencies. + let prim_clip_chain = match prim_clip_chain { + Some(prim_clip_chain) => prim_clip_chain, + None => return false, + }; + self.map_local_to_surface.set_target_spatial_node( prim_spatial_node_index, clip_scroll_tree, @@ -1894,8 +1913,24 @@ impl TileCacheInstance { return false; } + // If the primitive is directly drawn onto this picture cache surface, then + // the pic_clip_rect is in the same space. If not, we need to map it from + // the surface space into the picture cache space. + let on_picture_surface = surface_index == self.surface_index; + let pic_clip_rect = if on_picture_surface { + prim_clip_chain.pic_clip_rect + } else { + self.map_child_pic_to_surface.set_target_spatial_node( + surface_spatial_node_index, + clip_scroll_tree, + ); + self.map_child_pic_to_surface + .map(&prim_clip_chain.pic_clip_rect) + .expect("bug: unable to map clip rect to picture cache space") + }; + // Get the tile coordinates in the picture space. - let (p0, p1) = self.get_tile_coords_for_rect(&prim_rect); + let (p0, p1) = self.get_tile_coords_for_rect(&pic_clip_rect); // If the primitive is outside the tiling rects, it's known to not // be visible. @@ -1912,7 +1947,8 @@ impl TileCacheInstance { // Build the list of resources that this primitive has dependencies on. let mut prim_info = PrimitiveDependencyInfo::new( prim_instance.uid(), - prim_rect, + prim_rect.origin, + pic_clip_rect, is_cacheable, ); @@ -1922,21 +1958,17 @@ impl TileCacheInstance { } // 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_info.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 { - prim_info.clips.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. - let clip_node = &data_stores.clip[clip_instance.handle]; - if clip_node.item.spatial_node_index != self.spatial_node_index { - if !prim_info.spatial_nodes.contains(&clip_node.item.spatial_node_index) { - prim_info.spatial_nodes.push(clip_node.item.spatial_node_index); - } + let clip_instances = &clip_store + .clip_node_instances[prim_clip_chain.clips_range.to_range()]; + for clip_instance in clip_instances { + prim_info.clips.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. + let clip_node = &data_stores.clip[clip_instance.handle]; + if clip_node.item.spatial_node_index != self.spatial_node_index { + if !prim_info.spatial_nodes.contains(&clip_node.item.spatial_node_index) { + prim_info.spatial_nodes.push(clip_node.item.spatial_node_index); } } } @@ -2025,10 +2057,6 @@ impl TileCacheInstance { 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). @@ -2037,8 +2065,10 @@ impl TileCacheInstance { FontRenderMode::Alpha | FontRenderMode::Mono => false, }; + // If a text run is on a child surface, the subpx mode will be + // correctly determined as we recurse through pictures in take_context. if on_picture_surface && subpx_requested { - if !self.backdrop.rect.contains_rect(&prim_info.prim_clip_rect) { + if !self.backdrop.rect.contains_rect(&pic_clip_rect) { self.subpixel_mode = SubpixelMode::Deny; } } @@ -2076,8 +2106,6 @@ impl TileCacheInstance { // - The primitive is on the main picture cache surface. // - Same coord system as picture cache (ensures rects are axis-aligned). // - No clip masks exist. - let on_picture_surface = surface_index == self.surface_index; - let same_coord_system = { let prim_spatial_node = &clip_scroll_tree .spatial_nodes[prim_spatial_node_index.0 as usize]; @@ -2092,12 +2120,10 @@ impl TileCacheInstance { }; if is_suitable_backdrop { - if let Some(ref clip_chain) = prim_clip_chain { - if !clip_chain.needs_mask && clip_chain.pic_clip_rect.contains_rect(&self.backdrop.rect) { - self.backdrop = BackdropInfo { - rect: clip_chain.pic_clip_rect, - kind: backdrop_candidate, - } + if !prim_clip_chain.needs_mask && pic_clip_rect.contains_rect(&self.backdrop.rect) { + self.backdrop = BackdropInfo { + rect: pic_clip_rect, + kind: backdrop_candidate, } } } diff --git a/webrender/src/prim_store/mod.rs b/webrender/src/prim_store/mod.rs index 439aa7bcb4..32cb1e1a3a 100644 --- a/webrender/src/prim_store/mod.rs +++ b/webrender/src/prim_store/mod.rs @@ -2145,6 +2145,7 @@ impl PrimitiveStore { &self.opacity_bindings, &self.images, surface_index, + surface.surface_spatial_node_index, ) { prim_instance.visibility_info = PrimitiveVisibilityIndex::INVALID; // Ensure the primitive clip is popped - perhaps we can use From e61578384f4e0e8c3e118841e0add41774614c6d Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 13 Nov 2019 10:04:28 +0000 Subject: [PATCH 06/12] Bug 1594593 - Add wrench regression test for picture cache occlusion culling. r=kvark This adds a regression test for bug #1594567. Differential Revision: https://phabricator.services.mozilla.com/D52619 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/31cc5ca631c7a7618b7e5027dd56d6906b1bbd1b --- .../reftests/clip/clipped-occlusion-ref.yaml | 7 ++++++ wrench/reftests/clip/clipped-occlusion.yaml | 23 +++++++++++++++++++ wrench/reftests/clip/reftest.list | 1 + 3 files changed, 31 insertions(+) create mode 100644 wrench/reftests/clip/clipped-occlusion-ref.yaml create mode 100644 wrench/reftests/clip/clipped-occlusion.yaml diff --git a/wrench/reftests/clip/clipped-occlusion-ref.yaml b/wrench/reftests/clip/clipped-occlusion-ref.yaml new file mode 100644 index 0000000000..69a5fb0624 --- /dev/null +++ b/wrench/reftests/clip/clipped-occlusion-ref.yaml @@ -0,0 +1,7 @@ +--- +root: + items: + - + type: rect + bounds: [0, 0, 500, 500] + color: red diff --git a/wrench/reftests/clip/clipped-occlusion.yaml b/wrench/reftests/clip/clipped-occlusion.yaml new file mode 100644 index 0000000000..05a055a5a3 --- /dev/null +++ b/wrench/reftests/clip/clipped-occlusion.yaml @@ -0,0 +1,23 @@ +# This is a regression test for https://bugzilla.mozilla.org/show_bug.cgi?id=1594567 +# The single clip node from the primitive inside the scroll frame will be promoted +# to a 'shared clip' in the picture cache for the scroll frame. Ensure that this clip +# (zero sized in this test) is included in the tile occlusion culling. +--- +root: + items: + - + type: rect + bounds: [0, 0, 500, 500] + color: red + - + type: scroll-frame + content-size: [1000, 10000] + bounds: [0, -5000, 1000, 10000] + items: + - type: clip + bounds: [0, 0, 0, 0] + items: + - + bounds: [0, -5000, 1000, 10000] + type: rect + color: green diff --git a/wrench/reftests/clip/reftest.list b/wrench/reftests/clip/reftest.list index bb473ae875..e8907cac46 100644 --- a/wrench/reftests/clip/reftest.list +++ b/wrench/reftests/clip/reftest.list @@ -13,3 +13,4 @@ skip_on(android,device) == color_targets(3) alpha_targets(1) stacking-context-cl == snapping.yaml snapping-ref.yaml fuzzy(70,2400) == clip-and-filter-with-rotation.yaml clip-and-filter-with-rotation-ref.yaml color_targets(1) alpha_targets(0) == clip-out-rotation.yaml blank.yaml # Unexpected color targets, see bug 1580795 +== clipped-occlusion.yaml clipped-occlusion-ref.yaml From f1699947e59fc833ede0a977ed8f595225e9e48b Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 13 Nov 2019 10:04:36 +0000 Subject: [PATCH 07/12] Bug 1594364 - Support subpx AA with picture caching in more cases. r=kvark Instead of creating a picture caching slice for any content that is fixed position, also check if the clip(s) for the cluster are fixed position or anchored to the scroll root. This prevents WR creating slices for parallax style effects. There's not much point in doing this anyway, since those slices will invalidate due to the spatial node of the clip changing during scrolling. This also allows subpixel AA to be enabled in more situations. Differential Revision: https://phabricator.services.mozilla.com/D52621 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/03ab38bbbd2e4be75170cdca08fcc7bd519e47c2 --- webrender/src/scene_building.rs | 56 ++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index e7cc18f46b..03f84080ee 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -1888,7 +1888,11 @@ impl<'a> SceneBuilder<'a> { // are scrollbars. Once this lands, we can simplify this logic considerably // (and add a separate picture cache slice / OS layer for scroll bars). if parent_sc.pipeline_id != stacking_context.pipeline_id && self.iframe_depth == 1 { - self.content_slice_count = stacking_context.init_picture_caching(&self.clip_scroll_tree); + self.content_slice_count = stacking_context.init_picture_caching( + &self.clip_scroll_tree, + &self.clip_store, + &self.interners, + ); // Mark that a user supplied tile cache was specified. self.picture_caching_initialized = true; @@ -1916,7 +1920,11 @@ impl<'a> SceneBuilder<'a> { // on the root stacking context. This can happen in Gecko when the parent process // provides the content display list (e.g. about:support, about:config etc). if !self.picture_caching_initialized { - self.content_slice_count = stacking_context.init_picture_caching(&self.clip_scroll_tree); + self.content_slice_count = stacking_context.init_picture_caching( + &self.clip_scroll_tree, + &self.clip_store, + &self.interners, + ); self.picture_caching_initialized = true; } @@ -3573,6 +3581,8 @@ impl FlattenedStackingContext { fn init_picture_caching( &mut self, clip_scroll_tree: &ClipScrollTree, + clip_store: &ClipStore, + interners: &Interners, ) -> usize { struct SliceInfo { cluster_index: usize, @@ -3590,12 +3600,50 @@ impl FlattenedStackingContext { // We want to create a slice in the following conditions: // (1) This cluster is a scrollbar - // (2) This cluster begins a scroll root different from the current + // (2) Certain conditions when the scroll root changes (see below) // (3) No slice exists yet let create_new_slice = cluster.flags.contains(ClusterFlags::SCROLLBAR_CONTAINER) || slices.last().map(|slice| { - scroll_root != slice.scroll_root + match (slice.scroll_root, scroll_root) { + (ROOT_SPATIAL_NODE_INDEX, ROOT_SPATIAL_NODE_INDEX) => { + // Both current slice and this cluster are fixed position, no need to cut + false + } + (ROOT_SPATIAL_NODE_INDEX, _) => { + // A real scroll root is being established, so create a cache slice + true + } + (_, ROOT_SPATIAL_NODE_INDEX) => { + // A fixed position slice is encountered within a scroll root. Only create + // a slice in this case if all the clips referenced by this cluster are also + // fixed position. There's no real point in creating slices for these cases, + // since we'll have to rasterize them as the scrolling clip moves anyway. It + // also allows us to retain subpixel AA in these cases. For these types of + // slices, the intra-slice dirty rect handling typically works quite well + // (a common case is parallax scrolling effects). + for prim_instance in &cluster.prim_instances { + let mut current_clip_chain_id = prim_instance.clip_chain_id; + + while current_clip_chain_id != ClipChainId::NONE { + let clip_chain_node = &clip_store + .clip_chain_nodes[current_clip_chain_id.0 as usize]; + let clip_node_data = &interners.clip[clip_chain_node.handle]; + let clip_scroll_root = clip_scroll_tree.find_scroll_root(clip_node_data.spatial_node_index); + if clip_scroll_root != ROOT_SPATIAL_NODE_INDEX { + return false; + } + current_clip_chain_id = clip_chain_node.parent_clip_chain_id; + } + } + + true + } + (curr_scroll_root, scroll_root) => { + // Two scrolling roots - only need a new slice if they differ + curr_scroll_root != scroll_root + } + } }).unwrap_or(true); // Create a new slice if required From 17fdc31c4725e8358b667ec0d225f1fe692ffc2f Mon Sep 17 00:00:00 2001 From: Bert Peers Date: Wed, 13 Nov 2019 10:04:44 +0000 Subject: [PATCH 08/12] Bug 1571971 - Remove unused background_color field in Frame struct r=gw Differential Revision: https://phabricator.services.mozilla.com/D52303 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/7019a4a1fe39964f90b14158e84d05d727d888bd --- webrender/src/frame_builder.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/webrender/src/frame_builder.rs b/webrender/src/frame_builder.rs index 0186362533..bc6273fd28 100644 --- a/webrender/src/frame_builder.rs +++ b/webrender/src/frame_builder.rs @@ -615,7 +615,6 @@ impl FrameBuilder { device_origin, scene.output_rect.size, ), - background_color: scene.background_color, layer, profile_counters, passes, @@ -953,7 +952,6 @@ pub struct Frame { pub content_origin: DeviceIntPoint, /// The rectangle to show the frame in, on screen. pub device_rect: DeviceIntRect, - pub background_color: Option, pub layer: DocumentLayer, pub passes: Vec, #[cfg_attr(any(feature = "capture", feature = "replay"), serde(default = "FrameProfileCounters::new", skip))] From 83e3429fb76b8ff251e53d5ab2eb06bfbc562d10 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 13 Nov 2019 17:10:05 +0000 Subject: [PATCH 09/12] Bug 1594492 - Count the number of color/alpha passes instead of targets in the profiler HUD. r=gw It's a more useful number since we also report the number of existing and rasterized picture cache tiles. Also bump the expected number of alpha passes up to 2 since it is rather common. Differential Revision: https://phabricator.services.mozilla.com/D52134 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/7ce4cf2b6d61a4241708fec5e22f634192a1fe90 --- webrender/src/profiler.rs | 36 ++++++++++++++++++------------------ webrender/src/renderer.rs | 9 ++++++--- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/webrender/src/profiler.rs b/webrender/src/profiler.rs index 0af49cdd65..cc271ce98a 100644 --- a/webrender/src/profiler.rs +++ b/webrender/src/profiler.rs @@ -30,8 +30,8 @@ pub mod expected { pub const TOTAL_PRIMITIVES: Range = 1..5000; pub const VISIBLE_PRIMITIVES: Range = 1..5000; pub const USED_TARGETS: Range = 1..4; - pub const COLOR_TARGETS: Range = 1..4; - pub const ALPHA_TARGETS: Range = 0..2; + pub const COLOR_PASSES: Range = 1..4; + pub const ALPHA_PASSES: Range = 0..3; pub const RENDERED_PICTURE_CACHE_TILES: Range = 0..5; pub const TOTAL_PICTURE_CACHE_TILES: Range = 0..15; pub const CREATED_TARGETS: Range = 0..3; @@ -832,8 +832,8 @@ pub struct RendererProfileCounters { pub draw_calls: AverageIntProfileCounter, pub vertices: AverageIntProfileCounter, pub vao_count_and_size: ResourceProfileCounter, - pub color_targets: AverageIntProfileCounter, - pub alpha_targets: AverageIntProfileCounter, + pub color_passes: AverageIntProfileCounter, + pub alpha_passes: AverageIntProfileCounter, pub texture_data_uploaded: AverageIntProfileCounter, pub rendered_picture_cache_tiles: AverageIntProfileCounter, pub total_picture_cache_tiles: AverageIntProfileCounter, @@ -861,13 +861,13 @@ impl RendererProfileCounters { None, Some(expected::VERTICES), ), vao_count_and_size: ResourceProfileCounter::new("VAO", None, None), - color_targets: AverageIntProfileCounter::new( - "Color Targets", - None, Some(expected::COLOR_TARGETS), + color_passes: AverageIntProfileCounter::new( + "Color passes", + None, Some(expected::COLOR_PASSES), ), - alpha_targets: AverageIntProfileCounter::new( - "Alpha Targets", - None, Some(expected::ALPHA_TARGETS), + alpha_passes: AverageIntProfileCounter::new( + "Alpha passes", + None, Some(expected::ALPHA_PASSES), ), texture_data_uploaded: AverageIntProfileCounter::new( "Texture data, kb", @@ -887,8 +887,8 @@ impl RendererProfileCounters { pub fn reset(&mut self) { self.draw_calls.reset(); self.vertices.reset(); - self.color_targets.reset(); - self.alpha_targets.reset(); + self.color_passes.reset(); + self.alpha_passes.reset(); self.texture_data_uploaded.reset(); self.rendered_picture_cache_tiles.reset(); self.total_picture_cache_tiles.reset(); @@ -1480,8 +1480,8 @@ impl Profiler { Profiler::draw_counters( &[ &renderer_profile.frame_time as &dyn ProfileCounter, - &renderer_profile.color_targets, - &renderer_profile.alpha_targets, + &renderer_profile.color_passes, + &renderer_profile.alpha_passes, &renderer_profile.draw_calls, &renderer_profile.vertices, &renderer_profile.rendered_picture_cache_tiles, @@ -1513,8 +1513,8 @@ impl Profiler { &[ &renderer_profile.frame_time as &dyn ProfileCounter, &renderer_profile.frame_counter, - &renderer_profile.color_targets, - &renderer_profile.alpha_targets, + &renderer_profile.color_passes, + &renderer_profile.alpha_passes, &renderer_profile.rendered_picture_cache_tiles, &renderer_profile.total_picture_cache_tiles, &renderer_profile.texture_data_uploaded, @@ -1685,8 +1685,8 @@ impl Profiler { &self.gpu_time, ], &[ - &renderer_profile.color_targets, - &renderer_profile.alpha_targets, + &renderer_profile.color_passes, + &renderer_profile.alpha_passes, &renderer_profile.draw_calls, &renderer_profile.vertices, &renderer_profile.rendered_picture_cache_tiles, diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 9a609580e5..b55696647f 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -3790,7 +3790,6 @@ impl Renderer { stats: &mut RendererStats, ) { self.profile_counters.rendered_picture_cache_tiles.inc(); - self.profile_counters.color_targets.inc(); let _gm = self.gpu_profile.start_marker("picture cache target"); let framebuffer_kind = FramebufferKind::Other; @@ -4260,7 +4259,7 @@ impl Renderer { frame_id: GpuFrameId, stats: &mut RendererStats, ) { - self.profile_counters.color_targets.inc(); + self.profile_counters.color_passes.inc(); let _gm = self.gpu_profile.start_marker("color target"); // sanity check for the depth buffer @@ -4521,7 +4520,7 @@ impl Renderer { render_tasks: &RenderTaskGraph, stats: &mut RendererStats, ) { - self.profile_counters.alpha_targets.inc(); + self.profile_counters.alpha_passes.inc(); let _gm = self.gpu_profile.start_marker("alpha target"); let alpha_sampler = self.gpu_profile.start_sampler(GPU_SAMPLER_TAG_ALPHA); @@ -5185,6 +5184,10 @@ impl Renderer { ); } + if !picture_cache.is_empty() { + self.profile_counters.color_passes.inc(); + } + // Draw picture caching tiles for this pass. for picture_target in picture_cache { results.stats.color_target_count += 1; From 4ad16451f0b4cdc5133fce1253c0fab54fa83c54 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 14 Nov 2019 17:23:25 +0000 Subject: [PATCH 10/12] Bug 1595965 - Fix tile draw order with native compositor mode. r=sotaro In Draw compositor mode, tiles are placed into a bucket based on the tile kind and/or opacity. This simplifies the renderer code for this mode, which uses the z-buffer to reject opaque tiles. In native compositor mode, the tiles need to be submitted in the correct draw order. Opacity is a property of the native surface, and should not affect the order the tiles are added to the visual tree of the native compositor. Differential Revision: https://phabricator.services.mozilla.com/D52933 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/e7837d7a78c67fb122f6f4685eb0e2f06841b621 --- webrender/src/batch.rs | 49 +++++++++++++---------------------- webrender/src/composite.rs | 53 ++++++++++++++++++++++++++++++++++++++ webrender/src/renderer.rs | 2 +- 3 files changed, 72 insertions(+), 32 deletions(-) diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index e0e7d63eba..5bc78356a0 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -1206,45 +1206,32 @@ impl BatchBuilder { let device_rect = (tile.world_rect * ctx.global_device_pixel_scale).round(); let dirty_rect = (tile.world_dirty_rect * ctx.global_device_pixel_scale).round(); let surface = tile.surface.as_ref().expect("no tile surface set!"); - match surface { + + let (surface, is_opaque) = match surface { TileSurface::Color { color } => { - composite_state.opaque_tiles.push(CompositeTile { - surface: CompositeTileSurface::Color { color: *color }, - rect: device_rect, - dirty_rect, - clip_rect: device_clip_rect, - z_id, - }); + (CompositeTileSurface::Color { color: *color }, true) } TileSurface::Clear => { - composite_state.clear_tiles.push(CompositeTile { - surface: CompositeTileSurface::Clear, - rect: device_rect, - dirty_rect, - clip_rect: device_clip_rect, - z_id, - }); + (CompositeTileSurface::Clear, false) } TileSurface::Texture { descriptor, .. } => { let surface = descriptor.resolve(ctx.resource_cache); + ( + CompositeTileSurface::Texture { surface }, + tile.is_opaque || tile_cache.is_opaque(), + ) + } + }; - let composite_tile = CompositeTile { - surface: CompositeTileSurface::Texture { - surface, - }, - rect: device_rect, - dirty_rect, - clip_rect: device_clip_rect, - z_id, - }; + let tile = CompositeTile { + surface, + rect: device_rect, + dirty_rect, + clip_rect: device_clip_rect, + z_id, + }; - if tile.is_opaque || tile_cache.is_opaque() { - composite_state.opaque_tiles.push(composite_tile); - } else { - composite_state.alpha_tiles.push(composite_tile); - } - } - } + composite_state.push_tile(tile, is_opaque); } } PictureCompositeMode::Filter(ref filter) => { diff --git a/webrender/src/composite.rs b/webrender/src/composite.rs index ef7bcf4d6a..612f87e441 100644 --- a/webrender/src/composite.rs +++ b/webrender/src/composite.rs @@ -137,9 +137,19 @@ struct Occluder { #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct CompositeState { + // TODO(gw): Consider splitting up CompositeState into separate struct types depending + // on the selected compositing mode. Many of the fields in this state struct + // are only applicable to either Native or Draw compositing mode. + /// List of tiles to be drawn by the native compositor. These are added in draw order + /// and not separated by kind (opacity is a property of the native surface). + pub native_tiles: Vec, + /// List of opaque tiles to be drawn by the Draw compositor. pub opaque_tiles: Vec, + /// List of alpha tiles to be drawn by the Draw compositor. pub alpha_tiles: Vec, + /// List of clear tiles to be drawn by the Draw compositor. pub clear_tiles: Vec, + /// Used to generate z-id values for tiles in the Draw compositor mode. pub z_generator: ZBufferIdGenerator, // If false, we can't rely on the dirty rects in the CompositeTile // instances. This currently occurs during a scroll event, as a @@ -179,6 +189,7 @@ impl CompositeState { } CompositeState { + native_tiles: Vec::new(), opaque_tiles: Vec::new(), alpha_tiles: Vec::new(), clear_tiles: Vec::new(), @@ -273,6 +284,48 @@ impl CompositeState { // Check if the tile area is completely covered ref_area == cover_area } + + /// Add a tile to the appropriate array, depending on tile properties and compositor mode. + pub fn push_tile( + &mut self, + tile: CompositeTile, + is_opaque: bool, + ) { + match (self.compositor_kind, &tile.surface) { + (CompositorKind::Draw { .. }, CompositeTileSurface::Color { .. }) => { + // Color tiles are, by definition, opaque. We might support non-opaque color + // tiles if we ever find pages that have a lot of these. + self.opaque_tiles.push(tile); + } + (CompositorKind::Draw { .. }, CompositeTileSurface::Clear) => { + // Clear tiles have a special bucket + self.clear_tiles.push(tile); + } + (CompositorKind::Draw { .. }, CompositeTileSurface::Texture { .. }) => { + // Texture surfaces get bucketed by opaque/alpha, for z-rejection + // on the Draw compositor mode. + if is_opaque { + self.opaque_tiles.push(tile); + } else { + self.alpha_tiles.push(tile); + } + } + (CompositorKind::Native { .. }, CompositeTileSurface::Color { .. }) => { + // Native compositor doesn't (yet) support color surfaces. + unreachable!(); + } + (CompositorKind::Native { .. }, CompositeTileSurface::Clear) => { + // Native compositor doesn't support color surfaces. + unreachable!(); + } + (CompositorKind::Native { .. }, CompositeTileSurface::Texture { .. }) => { + // Native tiles are supplied to the OS compositor in draw order, + // since there is no z-buffer involved (opacity is supplied as part + // of the surface properties). + self.native_tiles.push(tile); + } + } + } } /// An arbitrary identifier for a native (OS compositor) surface diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index b55696647f..3eed705788 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -6762,7 +6762,7 @@ impl CompositeState { // For each tile, update the properties with the native OS compositor, // such as position and clip rect. z-order of the tiles are implicit based // on the order they are added in this loop. - for tile in self.opaque_tiles.iter().chain(self.alpha_tiles.iter()) { + for tile in &self.native_tiles { // Extract the native surface id. We should only ever encounter native surfaces here! let id = match tile.surface { CompositeTileSurface::Texture { surface: ResolvedSurfaceTexture::NativeSurface { id, .. }, .. } => id, From 9d03d3dd9ae05efa7f27def676a61ebf286ac435 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 15 Nov 2019 10:00:35 +0000 Subject: [PATCH 11/12] Bug 1595787: Move ExternalImageHandler and OutputImageHandler traits to webrender_api. r=kvark Differential Revision: https://phabricator.services.mozilla.com/D52686 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/24b134b55c5f70eab758d74682fc1f6ca829e0ff --- examples/common/boilerplate.rs | 4 +-- examples/frame_output.rs | 14 ++++---- examples/texture_cache_stress.rs | 12 +++---- examples/yuv.rs | 12 +++---- webrender/src/lib.rs | 8 ++--- webrender/src/renderer.rs | 61 ++++++-------------------------- webrender_api/src/image.rs | 51 +++++++++++++++++++++++++- wrench/src/yaml_frame_reader.rs | 8 ++--- 8 files changed, 89 insertions(+), 81 deletions(-) diff --git a/examples/common/boilerplate.rs b/examples/common/boilerplate.rs index 572477ccb1..b6bf25e68e 100644 --- a/examples/common/boilerplate.rs +++ b/examples/common/boilerplate.rs @@ -91,8 +91,8 @@ pub trait Example { fn get_image_handlers( &mut self, _gl: &dyn gl::Gl, - ) -> (Option>, - Option>) { + ) -> (Option>, + Option>) { (None, None) } fn draw_custom(&mut self, _gl: &dyn gl::Gl) { diff --git a/examples/frame_output.rs b/examples/frame_output.rs index 7531388f75..6abbc970c6 100644 --- a/examples/frame_output.rs +++ b/examples/frame_output.rs @@ -43,7 +43,7 @@ struct ExternalHandler { texture_id: gl::GLuint } -impl webrender::OutputImageHandler for OutputHandler { +impl OutputImageHandler for OutputHandler { fn lock(&mut self, _id: PipelineId) -> Option<(u32, FramebufferIntSize)> { Some((self.texture_id, FramebufferIntSize::new(500, 500))) } @@ -51,16 +51,16 @@ impl webrender::OutputImageHandler for OutputHandler { fn unlock(&mut self, _id: PipelineId) {} } -impl webrender::ExternalImageHandler for ExternalHandler { +impl ExternalImageHandler for ExternalHandler { fn lock( &mut self, _key: ExternalImageId, _channel_index: u8, _rendering: ImageRendering - ) -> webrender::ExternalImage { - webrender::ExternalImage { + ) -> ExternalImage { + ExternalImage { uv: TexelRect::new(0.0, 0.0, 1.0, 1.0), - source: webrender::ExternalImageSource::NativeTexture(self.texture_id), + source: ExternalImageSource::NativeTexture(self.texture_id), } } fn unlock(&mut self, _key: ExternalImageId, _channel_index: u8) {} @@ -182,8 +182,8 @@ impl Example for App { fn get_image_handlers( &mut self, gl: &dyn gl::Gl, - ) -> (Option>, - Option>) { + ) -> (Option>, + Option>) { let texture_id = gl.gen_textures(1)[0]; gl.bind_texture(gl::TEXTURE_2D, texture_id); diff --git a/examples/texture_cache_stress.rs b/examples/texture_cache_stress.rs index f99553fcb1..d7decc749c 100644 --- a/examples/texture_cache_stress.rs +++ b/examples/texture_cache_stress.rs @@ -62,17 +62,17 @@ impl ImageGenerator { } } -impl webrender::ExternalImageHandler for ImageGenerator { +impl ExternalImageHandler for ImageGenerator { fn lock( &mut self, _key: ExternalImageId, channel_index: u8, _rendering: ImageRendering - ) -> webrender::ExternalImage { + ) -> ExternalImage { self.generate_image(channel_index as i32); - webrender::ExternalImage { + ExternalImage { uv: TexelRect::new(0.0, 0.0, 1.0, 1.0), - source: webrender::ExternalImageSource::RawData(&self.current_image), + source: ExternalImageSource::RawData(&self.current_image), } } fn unlock(&mut self, _key: ExternalImageId, _channel_index: u8) {} @@ -299,8 +299,8 @@ impl Example for App { fn get_image_handlers( &mut self, _gl: &dyn gl::Gl, - ) -> (Option>, - Option>) { + ) -> (Option>, + Option>) { (Some(Box::new(ImageGenerator::new())), None) } } diff --git a/examples/yuv.rs b/examples/yuv.rs index dd9b2059aa..9e19e1ab4e 100644 --- a/examples/yuv.rs +++ b/examples/yuv.rs @@ -61,17 +61,17 @@ impl YuvImageProvider { } } -impl webrender::ExternalImageHandler for YuvImageProvider { +impl ExternalImageHandler for YuvImageProvider { fn lock( &mut self, key: ExternalImageId, _channel_index: u8, _rendering: ImageRendering - ) -> webrender::ExternalImage { + ) -> ExternalImage { let id = self.texture_ids[key.0 as usize]; - webrender::ExternalImage { + ExternalImage { uv: TexelRect::new(0.0, 0.0, 1.0, 1.0), - source: webrender::ExternalImageSource::NativeTexture(id), + source: ExternalImageSource::NativeTexture(id), } } fn unlock(&mut self, _key: ExternalImageId, _channel_index: u8) { @@ -198,8 +198,8 @@ impl Example for App { fn get_image_handlers( &mut self, gl: &dyn gl::Gl, - ) -> (Option>, - Option>) { + ) -> (Option>, + Option>) { let provider = YuvImageProvider::new(gl); self.texture_id = provider.texture_ids[0]; (Some(Box::new(provider)), None) diff --git a/webrender/src/lib.rs b/webrender/src/lib.rs index f7c27f6837..3c02f58f1a 100644 --- a/webrender/src/lib.rs +++ b/webrender/src/lib.rs @@ -209,10 +209,10 @@ pub use crate::frame_builder::ChasePrimitive; pub use crate::prim_store::PrimitiveDebugId; pub use crate::profiler::{ProfilerHooks, set_profiler_hooks}; pub use crate::renderer::{ - AsyncPropertySampler, CpuProfile, DebugFlags, OutputImageHandler, RendererKind, ExternalImage, - ExternalImageHandler, ExternalImageSource, GpuProfile, GraphicsApi, GraphicsApiInfo, - PipelineInfo, Renderer, RendererError, RendererOptions, RenderResults, RendererStats, SceneBuilderHooks, - ThreadListener, ShaderPrecacheFlags, MAX_VERTEX_TEXTURE_WIDTH, + AsyncPropertySampler, CpuProfile, DebugFlags, RendererKind, GpuProfile, GraphicsApi, + GraphicsApiInfo, PipelineInfo, Renderer, RendererError, RendererOptions, RenderResults, + RendererStats, SceneBuilderHooks, ThreadListener, ShaderPrecacheFlags, + MAX_VERTEX_TEXTURE_WIDTH, }; pub use crate::screen_capture::{AsyncScreenshotHandle, RecordedFrameHandle}; pub use crate::shade::{Shaders, WrShaders}; diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 3eed705788..4932663196 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -35,11 +35,13 @@ //! calling `DrawTarget::to_framebuffer_rect` use api::{ApiMsg, BlobImageHandler, ColorF, ColorU, MixBlendMode}; -use api::{DocumentId, Epoch, ExternalImageId}; -use api::{ExternalImageType, FontRenderMode, FrameMsg, ImageFormat, PipelineId}; -use api::{ImageRendering, Checkpoint, NotificationRequest}; +use api::{DocumentId, Epoch, ExternalImageHandler, ExternalImageId}; +use api::{ExternalImageSource, ExternalImageType, FontRenderMode, FrameMsg, ImageFormat}; +use api::{PipelineId, ImageRendering, Checkpoint, NotificationRequest, OutputImageHandler}; use api::{DebugCommand, MemoryReport, VoidPtrToSizeFn}; use api::{RenderApiSender, RenderNotifier, TextureTarget}; +#[cfg(feature = "replay")] +use api::ExternalImage; use api::channel; use api::units::*; pub use api::DebugFlags; @@ -5992,52 +5994,6 @@ impl Renderer { } } -pub enum ExternalImageSource<'a> { - RawData(&'a [u8]), // raw buffers. - NativeTexture(u32), // It's a gl::GLuint texture handle - Invalid, -} - -/// The data that an external client should provide about -/// an external image. The timestamp is used to test if -/// the renderer should upload new texture data this -/// frame. For instance, if providing video frames, the -/// application could call wr.render() whenever a new -/// video frame is ready. If the callback increments -/// the returned timestamp for a given image, the renderer -/// will know to re-upload the image data to the GPU. -/// Note that the UV coords are supplied in texel-space! -pub struct ExternalImage<'a> { - pub uv: TexelRect, - pub source: ExternalImageSource<'a>, -} - -/// The interfaces that an application can implement to support providing -/// external image buffers. -/// When the application passes an external image to WR, it should keep that -/// external image life time. People could check the epoch id in RenderNotifier -/// at the client side to make sure that the external image is not used by WR. -/// Then, do the clean up for that external image. -pub trait ExternalImageHandler { - /// Lock the external image. Then, WR could start to read the image content. - /// The WR client should not change the image content until the unlock() - /// call. Provide ImageRendering for NativeTexture external images. - fn lock(&mut self, key: ExternalImageId, channel_index: u8, rendering: ImageRendering) -> ExternalImage; - /// Unlock the external image. The WR should not read the image content - /// after this call. - fn unlock(&mut self, key: ExternalImageId, channel_index: u8); -} - -/// Allows callers to receive a texture with the contents of a specific -/// pipeline copied to it. Lock should return the native texture handle -/// and the size of the texture. Unlock will only be called if the lock() -/// call succeeds, when WR has issued the GL commands to copy the output -/// to the texture handle. -pub trait OutputImageHandler { - fn lock(&mut self, pipeline_id: PipelineId) -> Option<(u32, FramebufferIntSize)>; - fn unlock(&mut self, pipeline_id: PipelineId); -} - pub trait ThreadListener { fn thread_started(&self, thread_name: &str); fn thread_stopped(&self, thread_name: &str); @@ -6347,7 +6303,10 @@ impl ExternalImageHandler for DummyExternalImageHandler { } #[cfg(feature = "replay")] -impl OutputImageHandler for () { +struct VoidHandler; + +#[cfg(feature = "replay")] +impl OutputImageHandler for VoidHandler { fn lock(&mut self, _: PipelineId) -> Option<(u32, FramebufferIntSize)> { None } @@ -6710,7 +6669,7 @@ impl Renderer { self.device.end_frame(); } - self.output_image_handler = Some(Box::new(()) as Box<_>); + self.output_image_handler = Some(Box::new(VoidHandler) as Box<_>); self.external_image_handler = Some(Box::new(image_handler) as Box<_>); info!("done."); } diff --git a/webrender_api/src/image.rs b/webrender_api/src/image.rs index 86d4f1d7ba..741f28a2a0 100644 --- a/webrender_api/src/image.rs +++ b/webrender_api/src/image.rs @@ -9,7 +9,8 @@ use peek_poke::PeekPoke; use std::ops::{Add, Sub}; use std::sync::Arc; // local imports -use crate::api::{IdNamespace, TileSize}; +use crate::api::{IdNamespace, PipelineId, TileSize}; +use crate::display_item::ImageRendering; use crate::font::{FontInstanceKey, FontInstanceData, FontKey, FontTemplate}; use crate::units::*; @@ -57,6 +58,54 @@ impl BlobImageKey { #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Serialize, Deserialize)] pub struct ExternalImageId(pub u64); +/// The source for an external image. +pub enum ExternalImageSource<'a> { + /// A raw pixel buffer. + RawData(&'a [u8]), + /// A gl::GLuint texture handle. + NativeTexture(u32), + /// An invalid source. + Invalid, +} + +/// The data that an external client should provide about +/// an external image. For instance, if providing video frames, +/// the application could call wr.render() whenever a new +/// video frame is ready. Note that the UV coords are supplied +/// in texel-space! +pub struct ExternalImage<'a> { + /// UV coordinates for the image. + pub uv: TexelRect, + /// The source for this image's contents. + pub source: ExternalImageSource<'a>, +} + +/// The interfaces that an application can implement to support providing +/// external image buffers. +/// When the application passes an external image to WR, it should keep that +/// external image life time. People could check the epoch id in RenderNotifier +/// at the client side to make sure that the external image is not used by WR. +/// Then, do the clean up for that external image. +pub trait ExternalImageHandler { + /// Lock the external image. Then, WR could start to read the image content. + /// The WR client should not change the image content until the unlock() + /// call. Provide ImageRendering for NativeTexture external images. + fn lock(&mut self, key: ExternalImageId, channel_index: u8, rendering: ImageRendering) -> ExternalImage; + /// Unlock the external image. WR should not read the image content + /// after this call. + fn unlock(&mut self, key: ExternalImageId, channel_index: u8); +} + +/// Allows callers to receive a texture with the contents of a specific +/// pipeline copied to it. +pub trait OutputImageHandler { + /// Return the native texture handle and the size of the texture. + fn lock(&mut self, pipeline_id: PipelineId) -> Option<(u32, FramebufferIntSize)>; + /// Unlock will only be called if the lock() call succeeds, when WR has issued + /// the GL commands to copy the output to the texture handle. + fn unlock(&mut self, pipeline_id: PipelineId); +} + /// Specifies the type of texture target in driver terms. #[repr(u8)] #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] diff --git a/wrench/src/yaml_frame_reader.rs b/wrench/src/yaml_frame_reader.rs index a2483d68ee..eac790bba3 100644 --- a/wrench/src/yaml_frame_reader.rs +++ b/wrench/src/yaml_frame_reader.rs @@ -138,17 +138,17 @@ impl LocalExternalImageHandler { } } -impl webrender::ExternalImageHandler for LocalExternalImageHandler { +impl ExternalImageHandler for LocalExternalImageHandler { fn lock( &mut self, key: ExternalImageId, _channel_index: u8, _rendering: ImageRendering, - ) -> webrender::ExternalImage { + ) -> ExternalImage { let (id, desc) = self.texture_ids[key.0 as usize]; - webrender::ExternalImage { + ExternalImage { uv: TexelRect::new(0.0, 0.0, desc.size.width as f32, desc.size.height as f32), - source: webrender::ExternalImageSource::NativeTexture(id), + source: ExternalImageSource::NativeTexture(id), } } fn unlock(&mut self, _key: ExternalImageId, _channel_index: u8) {} From 352a122f7b3e313e7211237df6658a07c082058d Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 15 Nov 2019 10:00:44 +0000 Subject: [PATCH 12/12] Bug 1596503 - Remove unnecessary trailing semicolon with rust nightly (breaks the build) r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D53056 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/fb4b9875d44d472f27ea1df1066e1f359b28a053 --- webrender/src/resource_cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrender/src/resource_cache.rs b/webrender/src/resource_cache.rs index d25665a1f8..fb3d24289d 100644 --- a/webrender/src/resource_cache.rs +++ b/webrender/src/resource_cache.rs @@ -1028,7 +1028,7 @@ impl ResourceCache { image.dirty_rect = image.dirty_rect.union(dirty_rect); } - image.tiling = get_blob_tiling(image.tiling, blob_size, max_texture_size);; + image.tiling = get_blob_tiling(image.tiling, blob_size, max_texture_size); image.valid_tiles_after_bounds_change = valid_tiles_after_bounds_change; image.visible_rect = *visible_rect; }