From 0e04c9fc99e10004d38f22909b5c820ede9060ce Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Fri, 9 Nov 2018 11:46:46 +1000 Subject: [PATCH 1/2] Separate brush segment descriptors from clip mask instances. As part of interning primitives, we can move the segmenting process to occur when a primitive is newly interned (which is a significant optimization opportunity). We can do this since segmentation only occurs on clip nodes with the same spatial node as the primitive, thus the segmentation cannot be changed due to scrolling or transform animation. However, the presence of a clip mask per segment may still change each frame, due to clips from other positioning nodes. To handle this, we need to split the per-frame clip mask instance information from the brush segment descriptors. This is also a necessary step to allow borders to be interned, which rely on pre-generated segment descriptors. At the same time, make the size of a RenderTaskId smaller in release builds, and tidy up FrameId::invalid(). --- webrender/src/batch.rs | 127 ++++++++++++++++++-------- webrender/src/frame_builder.rs | 2 + webrender/src/glyph_rasterizer/mod.rs | 2 +- webrender/src/gpu_cache.rs | 2 +- webrender/src/prim_store.rs | 103 ++++++++++++++------- webrender/src/render_backend.rs | 12 +-- webrender/src/render_task.rs | 50 +++++++--- webrender/src/renderer.rs | 6 +- webrender/src/resource_cache.rs | 4 +- webrender/src/texture_cache.rs | 6 +- 10 files changed, 209 insertions(+), 105 deletions(-) diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 9ffd590ce7..0e869d3eb5 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -15,10 +15,10 @@ use gpu_types::{PrimitiveInstanceData, RasterizationSpace, GlyphInstance}; use gpu_types::{PrimitiveHeader, PrimitiveHeaderIndex, TransformPaletteId, TransformPalette}; use internal_types::{FastHashMap, SavedTargetIndex, TextureSource}; use picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive, PictureSurface}; -use prim_store::{BrushKind, BrushPrimitive, BrushSegmentTaskId, DeferredResolve}; -use prim_store::{EdgeAaSegmentMask, ImageSource, PrimitiveInstanceKind}; +use prim_store::{BrushKind, BrushPrimitive, DeferredResolve}; +use prim_store::{EdgeAaSegmentMask, ImageSource, PrimitiveInstanceKind, PrimitiveStore}; use prim_store::{VisibleGradientTile, PrimitiveInstance, PrimitiveOpacity}; -use prim_store::{BrushSegment, BorderSource, PrimitiveDetails}; +use prim_store::{BrushSegment, BorderSource, ClipMaskKind, ClipTaskIndex, PrimitiveDetails}; use render_task::{RenderTaskAddress, RenderTaskId, RenderTaskTree}; use renderer::{BlendMode, ImageBufferKind, ShaderColorMode}; use renderer::BLOCKS_PER_UV_RECT; @@ -537,9 +537,11 @@ impl AlphaBatchBuilder { .expect("bug"); let z_id = z_generator.next(); - let clip_task_address = prim_instance - .clip_task_id - .map_or(OPAQUE_TASK_ADDRESS, |id| render_tasks.get_task_address(id)); + // Get the clip task address for the global primitive, if one was set. + let clip_task_address = ctx + .prim_store + .get_clip_task_address(prim_instance.clip_task_index, 0, render_tasks) + .unwrap_or(OPAQUE_TASK_ADDRESS); match prim_instance.kind { PrimitiveInstanceKind::Clear => { @@ -745,7 +747,7 @@ impl AlphaBatchBuilder { // helper methods, as we port more primitives to make // use of interning. let blend_mode = if !prim_data.opacity.is_opaque || - prim_instance.clip_task_id.is_some() || + prim_instance.clip_task_index != ClipTaskIndex::INVALID || transform_kind == TransformedRectKind::Complex { BlendMode::PremultipliedAlpha @@ -820,9 +822,11 @@ impl AlphaBatchBuilder { }; let pic = &ctx.prim_store.pictures[pic_index.0]; - let clip_task_address = prim_instance - .clip_task_id - .map_or(OPAQUE_TASK_ADDRESS, |id| render_tasks.get_task_address(id)); + // Get clip task, if set, for the picture primitive. + let clip_task_address = ctx + .prim_store + .get_clip_task_address(prim_instance.clip_task_index, 0, render_tasks) + .unwrap_or(OPAQUE_TASK_ADDRESS); let prim_header = PrimitiveHeader { local_rect: pic.local_rect, @@ -1242,7 +1246,7 @@ impl AlphaBatchBuilder { match prim.details { PrimitiveDetails::Brush(ref brush) => { let non_segmented_blend_mode = if !brush.opacity.is_opaque || - prim_instance.clip_task_id.is_some() || + prim_instance.clip_task_index != ClipTaskIndex::INVALID || transform_kind == TransformedRectKind::Complex { specified_blend_mode @@ -1338,6 +1342,8 @@ impl AlphaBatchBuilder { transform_kind, render_tasks, z_id, + prim_instance.clip_task_index, + ctx, ); } } @@ -1396,40 +1402,48 @@ impl AlphaBatchBuilder { render_tasks: &RenderTaskTree, z_id: ZBufferId, prim_opacity: PrimitiveOpacity, + clip_task_index: ClipTaskIndex, + ctx: &RenderTargetContext, ) { - let clip_task_address = match segment.clip_task_id { - BrushSegmentTaskId::RenderTaskId(id) => - render_tasks.get_task_address(id), - BrushSegmentTaskId::Opaque => OPAQUE_TASK_ADDRESS, - BrushSegmentTaskId::Empty => return, - }; - - let is_inner = segment.edge_flags.is_empty(); - let needs_blending = !prim_opacity.is_opaque || - segment.clip_task_id.needs_blending() || - (!is_inner && transform_kind == TransformedRectKind::Complex); + debug_assert!(clip_task_index != ClipTaskIndex::INVALID); - let instance = PrimitiveInstanceData::from(BrushInstance { + // Get GPU address of clip task for this segment, or None if + // the entire segment is clipped out. + let clip_task_address = ctx.prim_store.get_clip_task_address( + clip_task_index, segment_index, - edge_flags: segment.edge_flags, - clip_task_address, - brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION | segment.brush_flags, - prim_header_index, - user_data: segment_data.user_data, - }); + render_tasks, + ); - let batch_key = BatchKey { - blend_mode: if needs_blending { alpha_blend_mode } else { BlendMode::None }, - kind: BatchKind::Brush(batch_kind), - textures: segment_data.textures, - }; + // If a got a valid (or OPAQUE) clip task address, add the segment. + if let Some(clip_task_address) = clip_task_address { + let is_inner = segment.edge_flags.is_empty(); + let needs_blending = !prim_opacity.is_opaque || + clip_task_address != OPAQUE_TASK_ADDRESS || + (!is_inner && transform_kind == TransformedRectKind::Complex); - self.batch_list.push_single_instance( - batch_key, - bounding_rect, - z_id, - instance, - ); + let instance = PrimitiveInstanceData::from(BrushInstance { + segment_index, + edge_flags: segment.edge_flags, + clip_task_address, + brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION | segment.brush_flags, + prim_header_index, + user_data: segment_data.user_data, + }); + + let batch_key = BatchKey { + blend_mode: if needs_blending { alpha_blend_mode } else { BlendMode::None }, + kind: BatchKind::Brush(batch_kind), + textures: segment_data.textures, + }; + + self.batch_list.push_single_instance( + batch_key, + bounding_rect, + z_id, + instance, + ); + } } /// Add any segment(s) from a brush to batches. @@ -1445,6 +1459,8 @@ impl AlphaBatchBuilder { transform_kind: TransformedRectKind, render_tasks: &RenderTaskTree, z_id: ZBufferId, + clip_task_index: ClipTaskIndex, + ctx: &RenderTargetContext, ) { match (&brush.segment_desc, ¶ms.segment_data) { (Some(ref segment_desc), SegmentDataKind::Instanced(ref segment_data)) => { @@ -1468,6 +1484,8 @@ impl AlphaBatchBuilder { render_tasks, z_id, brush.opacity, + clip_task_index, + ctx, ); } } @@ -1490,6 +1508,8 @@ impl AlphaBatchBuilder { render_tasks, z_id, brush.opacity, + clip_task_index, + ctx, ); } } @@ -2185,3 +2205,30 @@ fn get_buffer_kind(texture: TextureSource) -> ImageBufferKind { fn get_shader_opacity(opacity: f32) -> i32 { (opacity * 65535.0).round() as i32 } + +impl PrimitiveStore { + /// Retrieve the GPU task address for a given clip task instance. + /// Returns None if the segment was completely clipped out. + /// Returns Some(OPAQUE_TASK_ADDRESS) if no clip mask is needed. + /// Returns Some(task_address) if there was a valid clip mask. + fn get_clip_task_address( + &self, + clip_task_index: ClipTaskIndex, + offset: i32, + render_tasks: &RenderTaskTree, + ) -> Option { + let address = match self.clip_mask_instances[clip_task_index.0 as usize + offset as usize] { + ClipMaskKind::Mask(task_id) => { + render_tasks.get_task_address(task_id) + } + ClipMaskKind::None => { + OPAQUE_TASK_ADDRESS + } + ClipMaskKind::Clipped => { + return None; + } + }; + + Some(address) + } +} diff --git a/webrender/src/frame_builder.rs b/webrender/src/frame_builder.rs index 8f2298b606..58edd16608 100644 --- a/webrender/src/frame_builder.rs +++ b/webrender/src/frame_builder.rs @@ -194,6 +194,8 @@ impl FrameBuilder { return None } + self.prim_store.begin_frame(); + let root_spatial_node_index = clip_scroll_tree.root_reference_frame_index(); const MAX_CLIP_COORD: f32 = 1.0e9; diff --git a/webrender/src/glyph_rasterizer/mod.rs b/webrender/src/glyph_rasterizer/mod.rs index 837bc40e83..feb64181e5 100644 --- a/webrender/src/glyph_rasterizer/mod.rs +++ b/webrender/src/glyph_rasterizer/mod.rs @@ -734,7 +734,7 @@ mod test_glyph_rasterizer { let mut gpu_cache = GpuCache::new(); let mut texture_cache = TextureCache::new(2048, 1024); let mut render_task_cache = RenderTaskCache::new(); - let mut render_task_tree = RenderTaskTree::new(FrameId::invalid()); + let mut render_task_tree = RenderTaskTree::new(FrameId::INVALID); let mut special_render_passes = SpecialRenderPasses::new(&DeviceIntSize::new(1366, 768)); let mut font_file = diff --git a/webrender/src/gpu_cache.rs b/webrender/src/gpu_cache.rs index 0258f70c2a..98f2bcbfca 100644 --- a/webrender/src/gpu_cache.rs +++ b/webrender/src/gpu_cache.rs @@ -549,7 +549,7 @@ pub struct GpuCache { impl GpuCache { pub fn new() -> Self { GpuCache { - frame_id: FrameId::invalid(), + frame_id: FrameId::INVALID, texture: Texture::new(), saved_block_count: 0, in_debug: false, diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index 4e3930f863..c963a2518b 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -30,7 +30,7 @@ use render_task::{RenderTaskCacheKeyKind, RenderTaskId, RenderTaskCacheEntryHand use renderer::{MAX_VERTEX_TEXTURE_WIDTH}; use resource_cache::{ImageProperties, ImageRequest, ResourceCache}; use scene::SceneProperties; -use std::{cmp, fmt, mem, ops, usize}; +use std::{cmp, fmt, mem, ops, u32, usize}; #[cfg(debug_assertions)] use std::sync::atomic::{AtomicUsize, Ordering}; use tiling::SpecialRenderPasses; @@ -290,6 +290,13 @@ pub struct DeferredResolve { #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct PrimitiveIndex(pub usize); +#[derive(Debug, Copy, Clone, PartialEq)] +pub struct ClipTaskIndex(pub u32); + +impl ClipTaskIndex { + pub const INVALID: ClipTaskIndex = ClipTaskIndex(0); +} + #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] @@ -823,26 +830,20 @@ bitflags! { } } +/// Represents the visibility state of a segment (wrt clip masks). #[derive(Debug, Clone)] -pub enum BrushSegmentTaskId { - RenderTaskId(RenderTaskId), - Opaque, - Empty, -} - -impl BrushSegmentTaskId { - pub fn needs_blending(&self) -> bool { - match *self { - BrushSegmentTaskId::RenderTaskId(..) => true, - BrushSegmentTaskId::Opaque | BrushSegmentTaskId::Empty => false, - } - } +pub enum ClipMaskKind { + /// The segment has a clip mask, specified by the render task. + Mask(RenderTaskId), + /// The segment has no clip mask. + None, + /// The segment is made invisible / clipped completely. + Clipped, } #[derive(Debug, Clone)] pub struct BrushSegment { pub local_rect: LayoutRect, - pub clip_task_id: BrushSegmentTaskId, pub may_need_clip_mask: bool, pub edge_flags: EdgeAaSegmentMask, pub extra_data: [f32; 4], @@ -859,7 +860,6 @@ impl BrushSegment { ) -> Self { Self { local_rect, - clip_task_id: BrushSegmentTaskId::Opaque, may_need_clip_mask, edge_flags, extra_data, @@ -867,6 +867,8 @@ impl BrushSegment { } } + /// Write out to the clip mask instances array the correct clip mask + /// config for this segment. pub fn update_clip_task( &mut self, clip_chain: Option<&ClipChainInstance>, @@ -876,12 +878,13 @@ impl BrushSegment { pic_state: &mut PictureState, frame_context: &FrameBuildingContext, frame_state: &mut FrameBuildingState, + clip_mask_instances: &mut Vec, ) { match clip_chain { Some(clip_chain) => { if !clip_chain.needs_mask || (!self.may_need_clip_mask && !clip_chain.has_non_local_clips) { - self.clip_task_id = BrushSegmentTaskId::Opaque; + clip_mask_instances.push(ClipMaskKind::None); return; } @@ -894,7 +897,7 @@ impl BrushSegment { ) { Some(info) => info, None => { - self.clip_task_id = BrushSegmentTaskId::Empty; + clip_mask_instances.push(ClipMaskKind::Clipped); return; } }; @@ -912,10 +915,10 @@ impl BrushSegment { let clip_task_id = frame_state.render_tasks.add(clip_task); frame_state.surfaces[surface_index.0].tasks.push(clip_task_id); - self.clip_task_id = BrushSegmentTaskId::RenderTaskId(clip_task_id); + clip_mask_instances.push(ClipMaskKind::Mask(clip_task_id)); } None => { - self.clip_task_id = BrushSegmentTaskId::Empty; + clip_mask_instances.push(ClipMaskKind::Clipped); } } } @@ -1847,9 +1850,12 @@ pub struct PrimitiveInstance { /// completely off-screen. pub clipped_world_rect: Option, - /// If this primitive has a global clip mask, this identifies - /// the render task for it. - pub clip_task_id: Option, + /// An index into the clip task instances array in the primitive + /// store. If this is ClipTaskIndex::INVALID, then the primitive + /// has no clip mask. Otherwise, it may store the offset of the + /// global clip mask task for this primitive, or the first of + /// a list of clip task ids (one per segment). + pub clip_task_index: ClipTaskIndex, /// The main GPU cache handle that this primitive uses to /// store data accessible to shaders. This should be moved @@ -1881,11 +1887,11 @@ impl PrimitiveInstance { combined_local_clip_rect: LayoutRect::zero(), clipped_world_rect: None, #[cfg(debug_assertions)] - prepared_frame_id: FrameId::invalid(), + prepared_frame_id: FrameId::INVALID, #[cfg(debug_assertions)] id: PrimitiveDebugId(NEXT_PRIM_ID.fetch_add(1, Ordering::Relaxed)), - clip_task_id: None, gpu_location: GpuCacheHandle::new(), + clip_task_index: ClipTaskIndex::INVALID, clip_chain_id, spatial_node_index, cluster_range: ClusterRange { start: 0, end: 0 }, @@ -1907,6 +1913,13 @@ pub struct PrimitiveStore { pub primitives: Vec, pub pictures: Vec, pub text_runs: Vec, + + /// Written during primitive preparation, and read during + /// batching. Contains a list of clip mask instance parameters + /// per segment generated. + /// TODO(gw): We should be able to completely remove this once + /// the batching and prepare_prim passes are unified. + pub clip_mask_instances: Vec, } impl PrimitiveStore { @@ -1915,9 +1928,18 @@ impl PrimitiveStore { primitives: Vec::new(), pictures: Vec::new(), text_runs: Vec::new(), + clip_mask_instances: Vec::new(), } } + pub fn begin_frame(&mut self) { + // Clear the clip mask tasks for the beginning of the frame. Append + // a single kind representing no clip mask, at the ClipTaskIndex::INVALID + // location. + self.clip_mask_instances.clear(); + self.clip_mask_instances.push(ClipMaskKind::None) + } + pub fn create_picture( &mut self, prim: PicturePrimitive, @@ -2313,6 +2335,7 @@ impl PrimitiveStore { frame_state, &clip_node_collector, &mut self.primitives, + &mut self.clip_mask_instances, ); if prim_instance.is_chased() { @@ -2791,6 +2814,7 @@ impl PrimitiveInstance { frame_state: &mut FrameBuildingState, clip_node_collector: &Option, primitives: &mut [Primitive], + clip_mask_instances: &mut Vec, ) -> bool { let brush = match self.kind { PrimitiveInstanceKind::Picture { .. } | @@ -2807,13 +2831,6 @@ impl PrimitiveInstance { } }; - // Reset clip tasks from previous frame - if let Some(ref mut desc) = brush.segment_desc { - for segment in &mut desc.segments { - segment.clip_task_id = BrushSegmentTaskId::Opaque; - } - } - brush.write_brush_segment_description( prim_local_rect, prim_local_clip_rect, @@ -2826,6 +2843,17 @@ impl PrimitiveInstance { None => return false, }; + // If there are no segments, early out to avoid setting a valid + // clip task instance location below. + if segment_desc.segments.is_empty() { + return true; + } + + // Set where in the clip mask instances array the clip mask info + // can be found for this primitive. Each segment will push the + // clip mask information for itself in update_clip_task below. + self.clip_task_index = ClipTaskIndex(clip_mask_instances.len() as _); + // If we only built 1 segment, there is no point in re-running // the clip chain builder. Instead, just use the clip chain // instance that was built for the main primitive. This is a @@ -2839,6 +2867,7 @@ impl PrimitiveInstance { pic_state, frame_context, frame_state, + clip_mask_instances, ); } else { for segment in &mut segment_desc.segments { @@ -2871,6 +2900,7 @@ impl PrimitiveInstance { pic_state, frame_context, frame_state, + clip_mask_instances, ); } } @@ -3477,13 +3507,14 @@ impl PrimitiveInstance { frame_state: &mut FrameBuildingState, clip_node_collector: &Option, primitives: &mut [Primitive], + clip_mask_instances: &mut Vec, ) { if self.is_chased() { println!("\tupdating clip task with pic rect {:?}", clip_chain.pic_clip_rect); } // Reset clips from previous frames since we may clip differently each frame. - self.clip_task_id = None; + self.clip_task_index = ClipTaskIndex::INVALID; // First try to render this primitive's mask using optimized brush rendering. if self.update_clip_task_for_brush( @@ -3499,6 +3530,7 @@ impl PrimitiveInstance { frame_state, clip_node_collector, primitives, + clip_mask_instances, ) { if self.is_chased() { println!("\tsegment tasks have been created for clipping"); @@ -3530,7 +3562,10 @@ impl PrimitiveInstance { println!("\tcreated task {:?} with device rect {:?}", clip_task_id, device_rect); } - self.clip_task_id = Some(clip_task_id); + // Set the global clip mask instance for this primitive. + let clip_task_index = ClipTaskIndex(clip_mask_instances.len() as _); + clip_mask_instances.push(ClipMaskKind::Mask(clip_task_id)); + self.clip_task_index = clip_task_index; frame_state.surfaces[surface_index.0].tasks.push(clip_task_id); } } diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index 81b9903a1a..1f1e71dae6 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -86,12 +86,6 @@ impl DocumentView { pub struct FrameId(usize); impl FrameId { - /// Returns an invalid sentinel FrameId, which will always compare less than - /// any valid FrameId. - pub fn invalid() -> Self { - FrameId(0) - } - /// Returns a FrameId corresponding to the first frame. /// /// Note that we use 0 as the internal id here because the current code @@ -112,6 +106,10 @@ impl FrameId { fn advance(&mut self) { self.0 += 1; } + + /// An invalid sentinel FrameId, which will always compare less than + /// any valid FrameId. + pub const INVALID: FrameId = FrameId(0); } impl ::std::ops::Add for FrameId { @@ -345,7 +343,7 @@ impl Document { let accumulated_scale_factor = self.view.accumulated_scale_factor(); let pan = self.view.pan.to_f32() / accumulated_scale_factor; - assert!(self.frame_id != FrameId::invalid(), + assert!(self.frame_id != FrameId::INVALID, "First frame increment must happen before build_frame()"); let frame = { diff --git a/webrender/src/render_task.rs b/webrender/src/render_task.rs index cd8d1430b4..dbe5ec144d 100644 --- a/webrender/src/render_task.rs +++ b/webrender/src/render_task.rs @@ -27,7 +27,7 @@ use print_tree::{PrintTreePrinter}; use render_backend::FrameId; use resource_cache::{CacheItem, ResourceCache}; use surface::SurfaceCacheKey; -use std::{cmp, ops, mem, usize, f32, i32}; +use std::{cmp, ops, mem, usize, f32, i32, u32}; use texture_cache::{TextureCache, TextureCacheHandle, Eviction}; use tiling::{RenderPass, RenderTargetIndex}; use tiling::{RenderTargetKind}; @@ -50,7 +50,20 @@ fn render_task_sanity_check(size: &DeviceIntSize) { #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] -pub struct RenderTaskId(pub u32, FrameId); // TODO(gw): Make private when using GPU cache! +pub struct RenderTaskId { + pub index: u32, + + #[cfg(debug_assertions)] + pub frame_id: FrameId, +} + +impl RenderTaskId { + pub const INVALID: RenderTaskId = RenderTaskId { + index: u32::MAX, + #[cfg(debug_assertions)] + frame_id: FrameId::INVALID, + }; +} #[derive(Debug, Copy, Clone, PartialEq)] #[repr(C)] @@ -79,16 +92,21 @@ impl RenderTaskTree { } pub fn add(&mut self, task: RenderTask) -> RenderTaskId { - let id = self.tasks.len(); + let index = self.tasks.len() as _; self.tasks.push(task); - RenderTaskId(id as _, self.frame_id) + RenderTaskId { + index, + #[cfg(debug_assertions)] + frame_id: self.frame_id, + } } pub fn max_depth(&self, id: RenderTaskId, depth: usize, max_depth: &mut usize) { - debug_assert_eq!(self.frame_id, id.1); + #[cfg(debug_assertions)] + debug_assert_eq!(self.frame_id, id.frame_id); let depth = depth + 1; *max_depth = cmp::max(*max_depth, depth); - let task = &self.tasks[id.0 as usize]; + let task = &self.tasks[id.index as usize]; for child in &task.children { self.max_depth(*child, depth, max_depth); } @@ -100,8 +118,9 @@ impl RenderTaskTree { pass_index: usize, passes: &mut [RenderPass], ) { - debug_assert_eq!(self.frame_id, id.1); - let task = &self.tasks[id.0 as usize]; + #[cfg(debug_assertions)] + debug_assert_eq!(self.frame_id, id.frame_id); + let task = &self.tasks[id.index as usize]; for child in &task.children { self.assign_to_passes(*child, pass_index - 1, passes); @@ -135,8 +154,9 @@ impl RenderTaskTree { } pub fn get_task_address(&self, id: RenderTaskId) -> RenderTaskAddress { - debug_assert_eq!(self.frame_id, id.1); - RenderTaskAddress(id.0) + #[cfg(debug_assertions)] + debug_assert_eq!(self.frame_id, id.frame_id); + RenderTaskAddress(id.index) } pub fn write_task_data(&mut self, device_pixel_scale: DevicePixelScale) { @@ -160,15 +180,17 @@ impl RenderTaskTree { impl ops::Index for RenderTaskTree { type Output = RenderTask; fn index(&self, id: RenderTaskId) -> &RenderTask { - debug_assert_eq!(self.frame_id, id.1); - &self.tasks[id.0 as usize] + #[cfg(debug_assertions)] + debug_assert_eq!(self.frame_id, id.frame_id); + &self.tasks[id.index as usize] } } impl ops::IndexMut for RenderTaskTree { fn index_mut(&mut self, id: RenderTaskId) -> &mut RenderTask { - debug_assert_eq!(self.frame_id, id.1); - &mut self.tasks[id.0 as usize] + #[cfg(debug_assertions)] + debug_assert_eq!(self.frame_id, id.frame_id); + &mut self.tasks[id.index as usize] } } diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index c731f07959..4761ac1c08 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2022,7 +2022,7 @@ impl Renderer { gpu_cache_texture, #[cfg(feature = "debug_renderer")] gpu_cache_debug_chunks: Vec::new(), - gpu_cache_frame_id: FrameId::invalid(), + gpu_cache_frame_id: FrameId::INVALID, gpu_cache_overflow: false, texture_cache_upload_pbo, texture_resolver, @@ -2736,7 +2736,7 @@ impl Renderer { let gpu_cache_height = self.gpu_cache_texture.get_height(); if gpu_cache_height != 0 && GPU_CACHE_RESIZE_TEST { self.pending_gpu_cache_updates.push(GpuCacheUpdateList { - frame_id: FrameId::invalid(), + frame_id: FrameId::INVALID, height: gpu_cache_height, blocks: vec![[1f32; 4].into()], updates: Vec::new(), @@ -3747,7 +3747,7 @@ impl Renderer { .expect("Found external image, but no handler set!"); let mut list = GpuCacheUpdateList { - frame_id: FrameId::invalid(), + frame_id: FrameId::INVALID, height: self.gpu_cache_texture.get_height(), blocks: Vec::new(), updates: Vec::new(), diff --git a/webrender/src/resource_cache.rs b/webrender/src/resource_cache.rs index a7724f5c10..8c1b14a6bb 100644 --- a/webrender/src/resource_cache.rs +++ b/webrender/src/resource_cache.rs @@ -444,7 +444,7 @@ impl ResourceCache { cached_glyph_dimensions: FastHashMap::default(), texture_cache, state: State::Idle, - current_frame_id: FrameId::invalid(), + current_frame_id: FrameId::INVALID, pending_image_requests: FastHashSet::default(), glyph_rasterizer, blob_image_handler, @@ -2032,7 +2032,7 @@ impl ResourceCache { self.texture_cache = cached.textures; } None => { - self.current_frame_id = FrameId::invalid(); + self.current_frame_id = FrameId::INVALID; self.cached_glyphs.clear(); self.cached_glyph_dimensions.clear(); self.cached_images.clear(); diff --git a/webrender/src/texture_cache.rs b/webrender/src/texture_cache.rs index 23ee9fdfb1..ae3681cd09 100644 --- a/webrender/src/texture_cache.rs +++ b/webrender/src/texture_cache.rs @@ -373,8 +373,8 @@ impl TextureCache { max_texture_layers, next_id: CacheTextureId(1), pending_updates: TextureUpdateList::new(), - frame_id: FrameId::invalid(), - last_shared_cache_expiration: FrameId::invalid(), + frame_id: FrameId::INVALID, + last_shared_cache_expiration: FrameId::INVALID, entries: FreeList::new(), handles: EntryHandles::default(), } @@ -621,7 +621,7 @@ impl TextureCache { if let Some(entry) = self.entries.get_opt_mut(handle) { // Set last accessed frame to invalid to ensure it gets cleaned up // next time we expire entries. - entry.last_access = FrameId::invalid(); + entry.last_access = FrameId::INVALID; entry.eviction = Eviction::Auto; } } From 47f5cf1d7d3aeb056e64eb00f2a47ad363c99ed1 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Mon, 12 Nov 2018 06:47:40 +1000 Subject: [PATCH 2/2] Address review comments --- webrender/src/batch.rs | 126 +++++++++++++++++---------------- webrender/src/frame_builder.rs | 2 +- webrender/src/prim_store.rs | 23 +++--- webrender/src/render_task.rs | 2 +- 4 files changed, 77 insertions(+), 76 deletions(-) diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 0e869d3eb5..c4a87bd560 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -16,7 +16,7 @@ use gpu_types::{PrimitiveHeader, PrimitiveHeaderIndex, TransformPaletteId, Trans use internal_types::{FastHashMap, SavedTargetIndex, TextureSource}; use picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive, PictureSurface}; use prim_store::{BrushKind, BrushPrimitive, DeferredResolve}; -use prim_store::{EdgeAaSegmentMask, ImageSource, PrimitiveInstanceKind, PrimitiveStore}; +use prim_store::{EdgeAaSegmentMask, ImageSource, PrimitiveInstanceKind}; use prim_store::{VisibleGradientTile, PrimitiveInstance, PrimitiveOpacity}; use prim_store::{BrushSegment, BorderSource, ClipMaskKind, ClipTaskIndex, PrimitiveDetails}; use render_task::{RenderTaskAddress, RenderTaskId, RenderTaskTree}; @@ -538,10 +538,12 @@ impl AlphaBatchBuilder { let z_id = z_generator.next(); // Get the clip task address for the global primitive, if one was set. - let clip_task_address = ctx - .prim_store - .get_clip_task_address(prim_instance.clip_task_index, 0, render_tasks) - .unwrap_or(OPAQUE_TASK_ADDRESS); + let clip_task_address = get_clip_task_address( + &ctx.prim_store.clip_mask_instances, + prim_instance.clip_task_index, + 0, + render_tasks, + ).unwrap_or(OPAQUE_TASK_ADDRESS); match prim_instance.kind { PrimitiveInstanceKind::Clear => { @@ -823,10 +825,12 @@ impl AlphaBatchBuilder { let pic = &ctx.prim_store.pictures[pic_index.0]; // Get clip task, if set, for the picture primitive. - let clip_task_address = ctx - .prim_store - .get_clip_task_address(prim_instance.clip_task_index, 0, render_tasks) - .unwrap_or(OPAQUE_TASK_ADDRESS); + let clip_task_address = get_clip_task_address( + &ctx.prim_store.clip_mask_instances, + prim_instance.clip_task_index, + 0, + render_tasks, + ).unwrap_or(OPAQUE_TASK_ADDRESS); let prim_header = PrimitiveHeader { local_rect: pic.local_rect, @@ -1409,41 +1413,43 @@ impl AlphaBatchBuilder { // Get GPU address of clip task for this segment, or None if // the entire segment is clipped out. - let clip_task_address = ctx.prim_store.get_clip_task_address( + let clip_task_address = match get_clip_task_address( + &ctx.prim_store.clip_mask_instances, clip_task_index, segment_index, render_tasks, - ); + ) { + Some(clip_task_address) => clip_task_address, + None => return, + }; // If a got a valid (or OPAQUE) clip task address, add the segment. - if let Some(clip_task_address) = clip_task_address { - let is_inner = segment.edge_flags.is_empty(); - let needs_blending = !prim_opacity.is_opaque || - clip_task_address != OPAQUE_TASK_ADDRESS || - (!is_inner && transform_kind == TransformedRectKind::Complex); - - let instance = PrimitiveInstanceData::from(BrushInstance { - segment_index, - edge_flags: segment.edge_flags, - clip_task_address, - brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION | segment.brush_flags, - prim_header_index, - user_data: segment_data.user_data, - }); + let is_inner = segment.edge_flags.is_empty(); + let needs_blending = !prim_opacity.is_opaque || + clip_task_address != OPAQUE_TASK_ADDRESS || + (!is_inner && transform_kind == TransformedRectKind::Complex); - let batch_key = BatchKey { - blend_mode: if needs_blending { alpha_blend_mode } else { BlendMode::None }, - kind: BatchKind::Brush(batch_kind), - textures: segment_data.textures, - }; + let instance = PrimitiveInstanceData::from(BrushInstance { + segment_index, + edge_flags: segment.edge_flags, + clip_task_address, + brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION | segment.brush_flags, + prim_header_index, + user_data: segment_data.user_data, + }); - self.batch_list.push_single_instance( - batch_key, - bounding_rect, - z_id, - instance, - ); - } + let batch_key = BatchKey { + blend_mode: if needs_blending { alpha_blend_mode } else { BlendMode::None }, + kind: BatchKind::Brush(batch_kind), + textures: segment_data.textures, + }; + + self.batch_list.push_single_instance( + batch_key, + bounding_rect, + z_id, + instance, + ); } /// Add any segment(s) from a brush to batches. @@ -2206,29 +2212,27 @@ fn get_shader_opacity(opacity: f32) -> i32 { (opacity * 65535.0).round() as i32 } -impl PrimitiveStore { - /// Retrieve the GPU task address for a given clip task instance. - /// Returns None if the segment was completely clipped out. - /// Returns Some(OPAQUE_TASK_ADDRESS) if no clip mask is needed. - /// Returns Some(task_address) if there was a valid clip mask. - fn get_clip_task_address( - &self, - clip_task_index: ClipTaskIndex, - offset: i32, - render_tasks: &RenderTaskTree, - ) -> Option { - let address = match self.clip_mask_instances[clip_task_index.0 as usize + offset as usize] { - ClipMaskKind::Mask(task_id) => { - render_tasks.get_task_address(task_id) - } - ClipMaskKind::None => { - OPAQUE_TASK_ADDRESS - } - ClipMaskKind::Clipped => { - return None; - } - }; +/// Retrieve the GPU task address for a given clip task instance. +/// Returns None if the segment was completely clipped out. +/// Returns Some(OPAQUE_TASK_ADDRESS) if no clip mask is needed. +/// Returns Some(task_address) if there was a valid clip mask. +fn get_clip_task_address( + clip_mask_instances: &[ClipMaskKind], + clip_task_index: ClipTaskIndex, + offset: i32, + render_tasks: &RenderTaskTree, +) -> Option { + let address = match clip_mask_instances[clip_task_index.0 as usize + offset as usize] { + ClipMaskKind::Mask(task_id) => { + render_tasks.get_task_address(task_id) + } + ClipMaskKind::None => { + OPAQUE_TASK_ADDRESS + } + ClipMaskKind::Clipped => { + return None; + } + }; - Some(address) - } + Some(address) } diff --git a/webrender/src/frame_builder.rs b/webrender/src/frame_builder.rs index 58edd16608..4cd4263a14 100644 --- a/webrender/src/frame_builder.rs +++ b/webrender/src/frame_builder.rs @@ -194,7 +194,7 @@ impl FrameBuilder { return None } - self.prim_store.begin_frame(); + self.prim_store.reset_clip_instances(); let root_spatial_node_index = clip_scroll_tree.root_reference_frame_index(); diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index c963a2518b..8a0ec381a0 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -878,14 +878,12 @@ impl BrushSegment { pic_state: &mut PictureState, frame_context: &FrameBuildingContext, frame_state: &mut FrameBuildingState, - clip_mask_instances: &mut Vec, - ) { + ) -> ClipMaskKind { match clip_chain { Some(clip_chain) => { if !clip_chain.needs_mask || (!self.may_need_clip_mask && !clip_chain.has_non_local_clips) { - clip_mask_instances.push(ClipMaskKind::None); - return; + return ClipMaskKind::None; } let (device_rect, _, _) = match get_raster_rects( @@ -897,8 +895,7 @@ impl BrushSegment { ) { Some(info) => info, None => { - clip_mask_instances.push(ClipMaskKind::Clipped); - return; + return ClipMaskKind::Clipped; } }; @@ -915,10 +912,10 @@ impl BrushSegment { let clip_task_id = frame_state.render_tasks.add(clip_task); frame_state.surfaces[surface_index.0].tasks.push(clip_task_id); - clip_mask_instances.push(ClipMaskKind::Mask(clip_task_id)); + ClipMaskKind::Mask(clip_task_id) } None => { - clip_mask_instances.push(ClipMaskKind::Clipped); + ClipMaskKind::Clipped } } } @@ -1932,7 +1929,7 @@ impl PrimitiveStore { } } - pub fn begin_frame(&mut self) { + pub fn reset_clip_instances(&mut self) { // Clear the clip mask tasks for the beginning of the frame. Append // a single kind representing no clip mask, at the ClipTaskIndex::INVALID // location. @@ -2859,7 +2856,7 @@ impl PrimitiveInstance { // instance that was built for the main primitive. This is a // significant optimization for the common case. if segment_desc.segments.len() == 1 { - segment_desc.segments[0].update_clip_task( + let clip_mask_kind = segment_desc.segments[0].update_clip_task( Some(prim_clip_chain), prim_bounding_rect, root_spatial_node_index, @@ -2867,8 +2864,8 @@ impl PrimitiveInstance { pic_state, frame_context, frame_state, - clip_mask_instances, ); + clip_mask_instances.push(clip_mask_kind); } else { for segment in &mut segment_desc.segments { // Build a clip chain for the smaller segment rect. This will @@ -2892,7 +2889,7 @@ impl PrimitiveInstance { &mut frame_state.resources.clip_data_store, ); - segment.update_clip_task( + let clip_mask_kind = segment.update_clip_task( segment_clip_chain.as_ref(), prim_bounding_rect, root_spatial_node_index, @@ -2900,8 +2897,8 @@ impl PrimitiveInstance { pic_state, frame_context, frame_state, - clip_mask_instances, ); + clip_mask_instances.push(clip_mask_kind); } } diff --git a/webrender/src/render_task.rs b/webrender/src/render_task.rs index dbe5ec144d..3854e45917 100644 --- a/webrender/src/render_task.rs +++ b/webrender/src/render_task.rs @@ -54,7 +54,7 @@ pub struct RenderTaskId { pub index: u32, #[cfg(debug_assertions)] - pub frame_id: FrameId, + frame_id: FrameId, } impl RenderTaskId {