From 44849e69c1427366c577121a769e370420333e49 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Fri, 3 May 2019 20:26:32 +0000 Subject: [PATCH] Bug 1548712 - Restructure and tidy up some code in prepare primitive pass. r=kvark,nical Differential Revision: https://phabricator.services.mozilla.com/D29777 --HG-- extra : moz-landing-system : lando --- gfx/wr/webrender/src/batch.rs | 123 +++++++--------------- gfx/wr/webrender/src/frame_builder.rs | 44 ++++---- gfx/wr/webrender/src/picture.rs | 36 +++---- gfx/wr/webrender/src/prim_store/mod.rs | 135 +++++++++++-------------- gfx/wr/webrender/src/render_task.rs | 11 ++ 5 files changed, 143 insertions(+), 206 deletions(-) diff --git a/gfx/wr/webrender/src/batch.rs b/gfx/wr/webrender/src/batch.rs index b1e899cd0e46..cda428787493 100644 --- a/gfx/wr/webrender/src/batch.rs +++ b/gfx/wr/webrender/src/batch.rs @@ -14,7 +14,7 @@ use gpu_types::{ClipMaskInstance, SplitCompositeInstance, SnapOffsets}; 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 picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive}; use prim_store::{DeferredResolve, EdgeAaSegmentMask, PrimitiveInstanceKind, PrimitiveVisibilityIndex}; use prim_store::{VisibleGradientTile, PrimitiveInstance, PrimitiveOpacity, SegmentInstanceIndex}; use prim_store::{BrushSegment, ClipMaskKind, ClipTaskIndex, VECS_PER_SEGMENT}; @@ -1032,16 +1032,12 @@ impl AlphaBatchBuilder { .raster_config .as_ref() .expect("BUG: 3d primitive was not assigned a surface"); - let (uv_rect_address, _) = ctx - .surfaces[raster_config.surface_index.0] - .surface - .as_ref() - .expect("BUG: no surface") - .resolve( - render_tasks, - ctx.resource_cache, - gpu_cache, - ); + let (uv_rect_address, _) = render_tasks.resolve_surface( + ctx.surfaces[raster_config.surface_index.0] + .surface + .expect("BUG: no surface"), + gpu_cache, + ); let prim_header_index = prim_headers.push(&prim_header, z_id, [ uv_rect_address.as_int(), @@ -1099,9 +1095,7 @@ impl AlphaBatchBuilder { render_tasks, ).unwrap_or(OPAQUE_TASK_ADDRESS); - let surface = ctx.surfaces[raster_config.surface_index.0] - .surface - .as_ref(); + let surface = ctx.surfaces[raster_config.surface_index.0].surface; match raster_config.composite_mode { PictureCompositeMode::TileCache { .. } => { @@ -1263,13 +1257,10 @@ impl AlphaBatchBuilder { let kind = BatchKind::Brush( BrushBatchKind::Image(ImageBufferKind::Texture2DArray) ); - let (uv_rect_address, textures) = surface - .expect("bug: surface must be allocated by now") - .resolve( - render_tasks, - ctx.resource_cache, - gpu_cache, - ); + let (uv_rect_address, textures) = render_tasks.resolve_surface( + surface.expect("bug: surface must be allocated by now"), + gpu_cache, + ); let key = BatchKey::new( kind, non_segmented_blend_mode, @@ -1327,9 +1318,7 @@ impl AlphaBatchBuilder { let content_key = BatchKey::new(kind, non_segmented_blend_mode, content_textures); // Retrieve the UV rect addresses for shadow/content. - let cache_task_id = surface - .expect("bug: surface must be allocated by now") - .resolve_render_task_id(); + let cache_task_id = surface.expect("bug: surface must be allocated by now"); let shadow_uv_rect_address = render_tasks[cache_task_id] .get_texture_address(gpu_cache) .as_int(); @@ -1442,13 +1431,10 @@ impl AlphaBatchBuilder { FilterOp::ComponentTransfer => unreachable!(), }; - let (uv_rect_address, textures) = surface - .expect("bug: surface must be allocated by now") - .resolve( - render_tasks, - ctx.resource_cache, - gpu_cache, - ); + let (uv_rect_address, textures) = render_tasks.resolve_surface( + surface.expect("bug: surface must be allocated by now"), + gpu_cache, + ); let key = BatchKey::new( BatchKind::Brush(BrushBatchKind::Blend), @@ -1494,13 +1480,10 @@ impl AlphaBatchBuilder { let user_data = filter_data.gpu_cache_handle.as_int(gpu_cache); - let (uv_rect_address, textures) = surface - .expect("bug: surface must be allocated by now") - .resolve( - render_tasks, - ctx.resource_cache, - gpu_cache, - ); + let (uv_rect_address, textures) = render_tasks.resolve_surface( + surface.expect("bug: surface must be allocated by now"), + gpu_cache, + ); let key = BatchKey::new( BatchKind::Brush(BrushBatchKind::Blend), @@ -1532,13 +1515,10 @@ impl AlphaBatchBuilder { ); } PictureCompositeMode::MixBlend(mode) if ctx.use_advanced_blending => { - let (uv_rect_address, textures) = surface - .expect("bug: surface must be allocated by now") - .resolve( - render_tasks, - ctx.resource_cache, - gpu_cache, - ); + let (uv_rect_address, textures) = render_tasks.resolve_surface( + surface.expect("bug: surface must be allocated by now"), + gpu_cache, + ); let key = BatchKey::new( BatchKind::Brush( BrushBatchKind::Image(ImageBufferKind::Texture2DArray), @@ -1570,9 +1550,7 @@ impl AlphaBatchBuilder { ); } PictureCompositeMode::MixBlend(mode) => { - let cache_task_id = surface - .expect("bug: surface must be allocated by now") - .resolve_render_task_id(); + let cache_task_id = surface.expect("bug: surface must be allocated by now"); let backdrop_id = picture.secondary_render_task_id.expect("no backdrop!?"); let key = BatchKey::new( @@ -1612,9 +1590,7 @@ impl AlphaBatchBuilder { ); } PictureCompositeMode::Blit(_) => { - let cache_task_id = surface - .expect("bug: surface must be allocated by now") - .resolve_render_task_id(); + let cache_task_id = surface.expect("bug: surface must be allocated by now"); let uv_rect_address = render_tasks[cache_task_id] .get_texture_address(gpu_cache) .as_int(); @@ -2653,47 +2629,16 @@ impl PrimitiveInstance { } } -impl PictureSurface { - // Retrieve the uv rect handle, and texture for a picture surface. - fn resolve( +impl RenderTaskTree { + fn resolve_surface( &self, - render_tasks: &RenderTaskTree, - resource_cache: &ResourceCache, + task_id: RenderTaskId, gpu_cache: &GpuCache, ) -> (GpuCacheAddress, BatchTextures) { - match *self { - PictureSurface::TextureCache(ref handle) => { - let rt_cache_entry = resource_cache - .get_cached_render_task(handle); - let cache_item = resource_cache - .get_texture_cache_item(&rt_cache_entry.handle); - - ( - gpu_cache.get_address(&cache_item.uv_rect_handle), - BatchTextures::color(cache_item.texture_id), - ) - } - PictureSurface::RenderTask(task_id) => { - ( - render_tasks[task_id].get_texture_address(gpu_cache), - BatchTextures::render_target_cache(), - ) - } - } - } - - // Retrieve the render task id for a picture surface. Should only - // be used where it's known that this picture surface will never - // be persisted in the texture cache. - fn resolve_render_task_id(&self) -> RenderTaskId { - match *self { - PictureSurface::TextureCache(..) => { - panic!("BUG: unexpectedly cached render task"); - } - PictureSurface::RenderTask(task_id) => { - task_id - } - } + ( + self[task_id].get_texture_address(gpu_cache), + BatchTextures::render_target_cache(), + ) } } diff --git a/gfx/wr/webrender/src/frame_builder.rs b/gfx/wr/webrender/src/frame_builder.rs index b07532480c54..023fdd3c5803 100644 --- a/gfx/wr/webrender/src/frame_builder.rs +++ b/gfx/wr/webrender/src/frame_builder.rs @@ -14,7 +14,7 @@ use hit_test::{HitTester, HitTestingScene}; #[cfg(feature = "replay")] use hit_test::HitTestingSceneStats; use internal_types::{FastHashMap, PlaneSplitter}; -use picture::{PictureSurface, PictureUpdateState, SurfaceInfo, ROOT_SURFACE_INDEX, SurfaceIndex}; +use picture::{PictureUpdateState, SurfaceInfo, ROOT_SURFACE_INDEX, SurfaceIndex}; use picture::{RetainedTiles, TileCache, DirtyRegion}; use prim_store::{PrimitiveStore, SpaceMapper, PictureIndex, PrimitiveDebugId, PrimitiveScratchBuffer}; #[cfg(feature = "replay")] @@ -428,6 +428,24 @@ impl FrameBuilder { clip_chain_stack: ClipChainStack::new(), }; + let root_render_task = RenderTask::new_picture( + RenderTaskLocation::Fixed(self.output_rect), + self.output_rect.size.to_f32(), + self.root_pic_index, + DeviceIntPoint::zero(), + Vec::new(), + UvRectKind::Rect, + root_spatial_node_index, + global_device_pixel_scale, + ); + + let root_render_task_id = frame_state.render_tasks.add(root_render_task); + frame_state + .surfaces + .first_mut() + .unwrap() + .surface = Some(root_render_task_id); + // Push a default dirty region which culls primitives // against the screen world rect, in absence of any // other dirty regions. @@ -478,24 +496,14 @@ impl FrameBuilder { .surfaces[ROOT_SURFACE_INDEX.0] .take_render_tasks(); - let root_render_task = RenderTask::new_picture( - RenderTaskLocation::Fixed(self.output_rect), - self.output_rect.size.to_f32(), - self.root_pic_index, - DeviceIntPoint::zero(), - child_tasks, - UvRectKind::Rect, - root_spatial_node_index, - global_device_pixel_scale, - ); + for child_task_id in child_tasks { + frame_state.render_tasks.add_dependency( + root_render_task_id, + child_task_id, + ); + } - let render_task_id = frame_state.render_tasks.add(root_render_task); - frame_state - .surfaces - .first_mut() - .unwrap() - .surface = Some(PictureSurface::RenderTask(render_task_id)); - Some(render_task_id) + Some(root_render_task_id) } pub fn build( diff --git a/gfx/wr/webrender/src/picture.rs b/gfx/wr/webrender/src/picture.rs index 0c57fcc6c0bc..43dbe719065d 100644 --- a/gfx/wr/webrender/src/picture.rs +++ b/gfx/wr/webrender/src/picture.rs @@ -27,7 +27,7 @@ use prim_store::{get_raster_rects, PrimitiveScratchBuffer, VectorKey, PointKey}; use prim_store::{OpacityBindingStorage, ImageInstanceStorage, OpacityBindingIndex, RectangleKey}; use print_tree::PrintTreePrinter; use render_backend::DataStores; -use render_task::{ClearMode, RenderTask, RenderTaskCacheEntryHandle, TileBlit}; +use render_task::{ClearMode, RenderTask, TileBlit}; use render_task::{RenderTaskId, RenderTaskLocation}; use resource_cache::ResourceCache; use scene::{FilterOpHelpers, SceneProperties}; @@ -1794,7 +1794,7 @@ pub struct SurfaceInfo { pub raster_spatial_node_index: SpatialNodeIndex, pub surface_spatial_node_index: SpatialNodeIndex, /// This is set when the render task is created. - pub surface: Option, + pub surface: Option, /// A list of render tasks that are dependencies of this surface. pub tasks: Vec, /// How much the local surface rect should be inflated (for blur radii). @@ -1898,17 +1898,6 @@ pub enum PictureCompositeMode { }, } -// Stores the location of the picture if it is drawn to -// an intermediate surface. This can be a render task if -// it is not persisted, or a texture cache item if the -// picture is cached in the texture cache. -#[derive(Debug)] -pub enum PictureSurface { - RenderTask(RenderTaskId), - #[allow(dead_code)] - TextureCache(RenderTaskCacheEntryHandle), -} - /// Enum value describing the place of a picture in a 3D context. #[derive(Clone, Debug)] #[cfg_attr(feature = "capture", derive(Serialize))] @@ -2497,14 +2486,14 @@ impl PicturePrimitive { pub fn add_split_plane( splitter: &mut PlaneSplitter, transforms: &TransformPalette, - prim_instance: &PrimitiveInstance, + prim_spatial_node_index: SpatialNodeIndex, original_local_rect: LayoutRect, combined_local_clip_rect: &LayoutRect, world_rect: WorldRect, plane_split_anchor: usize, ) -> bool { let transform = transforms - .get_world_transform(prim_instance.spatial_node_index); + .get_world_transform(prim_spatial_node_index); let matrix = transform.cast(); // Apply the local clip rect here, before splitting. This is @@ -2523,7 +2512,7 @@ impl PicturePrimitive { if transform.is_simple_translation() { let inv_transform = transforms - .get_world_inv_transform(prim_instance.spatial_node_index); + .get_world_inv_transform(prim_spatial_node_index); let polygon = Polygon::from_transformed_rect_with_inverse( local_rect, &matrix, @@ -2886,7 +2875,6 @@ impl PicturePrimitive { pub fn prepare_for_render( &mut self, pic_index: PictureIndex, - prim_instance: &PrimitiveInstance, clipped_prim_bounding_rect: WorldRect, surface_index: SurfaceIndex, frame_context: &FrameBuildingContext, @@ -2916,7 +2904,7 @@ impl PicturePrimitive { }; let (map_raster_to_world, map_pic_to_raster) = create_raster_mappers( - prim_instance.spatial_node_index, + self.spatial_node_index, raster_spatial_node_index, frame_context.screen_world_rect, frame_context.clip_scroll_tree, @@ -3015,7 +3003,7 @@ impl PicturePrimitive { frame_state.surfaces[surface_index.0].tasks.push(render_task_id); - PictureSurface::RenderTask(render_task_id) + render_task_id } PictureCompositeMode::Filter(FilterOp::DropShadow(offset, blur_radius, color)) => { let blur_std_deviation = blur_radius * device_pixel_scale.0; @@ -3101,7 +3089,7 @@ impl PicturePrimitive { request.push([0.0, 0.0, 0.0, 0.0]); } - PictureSurface::RenderTask(render_task_id) + render_task_id } PictureCompositeMode::MixBlend(..) if !frame_context.fb_config.gpu_supports_advanced_blend => { let uv_rect_kind = calculate_uv_rect_kind( @@ -3132,7 +3120,7 @@ impl PicturePrimitive { let render_task_id = frame_state.render_tasks.add(picture_task); frame_state.surfaces[surface_index.0].tasks.push(render_task_id); - PictureSurface::RenderTask(render_task_id) + render_task_id } PictureCompositeMode::Filter(ref filter) => { if let FilterOp::ColorMatrix(m) = *filter { @@ -3164,7 +3152,7 @@ impl PicturePrimitive { let render_task_id = frame_state.render_tasks.add(picture_task); frame_state.surfaces[surface_index.0].tasks.push(render_task_id); - PictureSurface::RenderTask(render_task_id) + render_task_id } PictureCompositeMode::ComponentTransferFilter(handle) => { let filter_data = &mut data_stores.filter_data[handle]; @@ -3191,7 +3179,7 @@ impl PicturePrimitive { let render_task_id = frame_state.render_tasks.add(picture_task); frame_state.surfaces[surface_index.0].tasks.push(render_task_id); - PictureSurface::RenderTask(render_task_id) + render_task_id } PictureCompositeMode::MixBlend(..) | PictureCompositeMode::Blit(_) => { @@ -3223,7 +3211,7 @@ impl PicturePrimitive { let render_task_id = frame_state.render_tasks.add(picture_task); frame_state.surfaces[surface_index.0].tasks.push(render_task_id); - PictureSurface::RenderTask(render_task_id) + render_task_id } }; diff --git a/gfx/wr/webrender/src/prim_store/mod.rs b/gfx/wr/webrender/src/prim_store/mod.rs index b3734a536770..2a63c60a6dc8 100644 --- a/gfx/wr/webrender/src/prim_store/mod.rs +++ b/gfx/wr/webrender/src/prim_store/mod.rs @@ -2442,78 +2442,16 @@ impl PrimitiveStore { prim_instance.prepared_frame_id = frame_state.render_tasks.frame_id(); } - match prim_instance.kind { - PrimitiveInstanceKind::Picture { pic_index, segment_instance_index, .. } => { - let pic = &mut self.pictures[pic_index.0]; - let prim_info = &scratch.prim_info[prim_instance.visibility_info.0 as usize]; - if pic.prepare_for_render( - pic_index, - prim_instance, - prim_info.clipped_world_rect, - pic_context.surface_index, - frame_context, - frame_state, - data_stores, - ) { - if let Some(ref mut splitter) = pic_state.plane_splitter { - PicturePrimitive::add_split_plane( - splitter, - frame_state.transforms, - prim_instance, - pic.local_rect, - &prim_info.combined_local_clip_rect, - frame_context.screen_world_rect, - plane_split_anchor, - ); - } - - // If this picture uses segments, ensure the GPU cache is - // up to date with segment local rects. - // TODO(gw): This entire match statement above can now be - // refactored into prepare_interned_prim_for_render. - if pic.can_use_segments() { - write_segment( - segment_instance_index, - frame_state, - &mut scratch.segments, - &mut scratch.segment_instances, - |request| { - request.push(PremultipliedColorF::WHITE); - request.push(PremultipliedColorF::WHITE); - request.push([ - -1.0, // -ve means use prim rect for stretch size - 0.0, - 0.0, - 0.0, - ]); - } - ); - } - } else { - prim_instance.visibility_info = PrimitiveVisibilityIndex::INVALID; - } - } - PrimitiveInstanceKind::TextRun { .. } | - PrimitiveInstanceKind::Clear { .. } | - PrimitiveInstanceKind::Rectangle { .. } | - PrimitiveInstanceKind::NormalBorder { .. } | - PrimitiveInstanceKind::ImageBorder { .. } | - PrimitiveInstanceKind::YuvImage { .. } | - PrimitiveInstanceKind::Image { .. } | - PrimitiveInstanceKind::LinearGradient { .. } | - PrimitiveInstanceKind::RadialGradient { .. } | - PrimitiveInstanceKind::LineDecoration { .. } => { - self.prepare_interned_prim_for_render( - prim_instance, - pic_context, - pic_state, - frame_context, - frame_state, - data_stores, - scratch, - ); - } - } + self.prepare_interned_prim_for_render( + prim_instance, + plane_split_anchor, + pic_context, + pic_state, + frame_context, + frame_state, + data_stores, + scratch, + ); true } @@ -2602,8 +2540,9 @@ impl PrimitiveStore { fn prepare_interned_prim_for_render( &mut self, prim_instance: &mut PrimitiveInstance, + plane_split_anchor: usize, pic_context: &PictureContext, - pic_state: &PictureState, + pic_state: &mut PictureState, frame_context: &FrameBuildingContext, frame_state: &mut FrameBuildingState, data_stores: &mut DataStores, @@ -2993,8 +2932,54 @@ impl PrimitiveStore { // TODO(gw): Consider whether it's worth doing segment building // for gradient primitives. } - _ => { - unreachable!(); + PrimitiveInstanceKind::Picture { pic_index, segment_instance_index, .. } => { + let pic = &mut self.pictures[pic_index.0]; + let prim_info = &scratch.prim_info[prim_instance.visibility_info.0 as usize]; + if pic.prepare_for_render( + *pic_index, + prim_info.clipped_world_rect, + pic_context.surface_index, + frame_context, + frame_state, + data_stores, + ) { + if let Some(ref mut splitter) = pic_state.plane_splitter { + PicturePrimitive::add_split_plane( + splitter, + frame_state.transforms, + prim_instance.spatial_node_index, + pic.local_rect, + &prim_info.combined_local_clip_rect, + frame_context.screen_world_rect, + plane_split_anchor, + ); + } + + // If this picture uses segments, ensure the GPU cache is + // up to date with segment local rects. + // TODO(gw): This entire match statement above can now be + // refactored into prepare_interned_prim_for_render. + if pic.can_use_segments() { + write_segment( + *segment_instance_index, + frame_state, + &mut scratch.segments, + &mut scratch.segment_instances, + |request| { + request.push(PremultipliedColorF::WHITE); + request.push(PremultipliedColorF::WHITE); + request.push([ + -1.0, // -ve means use prim rect for stretch size + 0.0, + 0.0, + 0.0, + ]); + } + ); + } + } else { + prim_instance.visibility_info = PrimitiveVisibilityIndex::INVALID; + } } }; } diff --git a/gfx/wr/webrender/src/render_task.rs b/gfx/wr/webrender/src/render_task.rs index 830ceddda133..dd68a6b55461 100644 --- a/gfx/wr/webrender/src/render_task.rs +++ b/gfx/wr/webrender/src/render_task.rs @@ -139,6 +139,17 @@ impl RenderTaskTree { } } + /// Express a render task dependency between a parent and child task. + /// This is used to assign tasks to render passes. + pub fn add_dependency( + &mut self, + parent_id: RenderTaskId, + child_id: RenderTaskId, + ) { + let parent = &mut self[parent_id]; + parent.children.push(child_id); + } + /// Assign this frame's render tasks to render passes ordered so that passes appear /// earlier than the ones that depend on them. pub fn generate_passes(