From cf9d1473410901ef689b67085876c7a40f1e005c Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Fri, 13 Apr 2018 13:10:07 +1000 Subject: [PATCH] Change drop-shadows to be drawn via a single primitive. Previously, drop-shadows were a Picture, with two brush image primitives, each referencing the picture (one for the content, one for the shadow with an offset). Although conceptually reasonable, this exposes some problems with the way the current prim_store visibility pass works. Specifically, we can end up processing the picture more than once. In general this is an inefficiency but doesn't affect correctness, but it breaks some assumptions once the content of the drop-shadow element is an item in the render task cache. In the future, this problem should disappear as we refactor how the visibility pass operates, but for now the simplest solution is to draw the drop-shadow with a single primitive. To make this work, there are a couple of hacks in this patch, which push an extra brush image primitive into the GPU cache data. The drop shadows still have an issue with the shadow disappearing if the content of the drop shadow is off-screen, however this is no worse than the existing code. Doing it this way actually makes it easier to fix this is the future too. --- webrender/src/batch.rs | 85 ++++++++++++++++--------- webrender/src/display_list_flattener.rs | 61 ++---------------- webrender/src/gpu_cache.rs | 7 ++ webrender/src/picture.rs | 50 +++++++++++---- webrender/src/prim_store.rs | 67 ++++--------------- 5 files changed, 116 insertions(+), 154 deletions(-) diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index eadb75f288..cfe3a649c7 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -638,7 +638,7 @@ impl AlphaBatchBuilder { let brush = &ctx.prim_store.cpu_brushes[prim_metadata.cpu_prim_index.0]; match brush.kind { - BrushKind::Picture { pic_index, source_kind, .. } => { + BrushKind::Picture { pic_index, .. } => { let picture = &ctx.prim_store.pictures[pic_index.0]; @@ -711,42 +711,49 @@ impl AlphaBatchBuilder { } FilterOp::DropShadow(..) => { if let Some(cache_task_id) = picture.surface { + // Draw an instance of the shadow first, following by the content. + + // Both the shadow and the content get drawn as a brush image. let kind = BatchKind::Brush( BrushBatchKind::Image(ImageBufferKind::Texture2DArray), ); - let (textures, task_id) = match source_kind { - BrushImageSourceKind::Color => { - let secondary_id = picture.secondary_render_task_id.expect("no secondary!?"); - let saved_index = render_tasks[secondary_id].saved_index.expect("no saved index!?"); - debug_assert_ne!(saved_index, SavedTargetIndex::PENDING); - let textures = BatchTextures { - colors: [ - SourceTexture::RenderTaskCache(saved_index), - SourceTexture::Invalid, - SourceTexture::Invalid, - ], - }; - (textures, secondary_id) - } - BrushImageSourceKind::ColorAlphaMask => { - (BatchTextures::render_target_cache(), cache_task_id) - } + // Gets the saved render task ID of the content, which is + // deeper in the render task tree than the direct child. + let secondary_id = picture.secondary_render_task_id.expect("no secondary!?"); + let saved_index = render_tasks[secondary_id].saved_index.expect("no saved index!?"); + debug_assert_ne!(saved_index, SavedTargetIndex::PENDING); + + // Build BatchTextures for shadow/content + let shadow_textures = BatchTextures::render_target_cache(); + let content_textures = BatchTextures { + colors: [ + SourceTexture::RenderTaskCache(saved_index), + SourceTexture::Invalid, + SourceTexture::Invalid, + ], }; - let key = BatchKey::new( - kind, - non_segmented_blend_mode, - textures, - ); + // Build batch keys for shadow/content + let shadow_key = BatchKey::new(kind, non_segmented_blend_mode, shadow_textures); + let content_key = BatchKey::new(kind, non_segmented_blend_mode, content_textures); - let uv_rect_address = render_tasks[task_id] + // Retrieve the UV rect addresses for shadow/content. + let shadow_uv_rect_address = render_tasks[cache_task_id] .get_texture_handle() .as_int(gpu_cache); + let content_uv_rect_address = render_tasks[secondary_id] + .get_texture_handle() + .as_int(gpu_cache); + + // Get the GPU cache address of the extra data handle. + let extra_data_address = gpu_cache.get_address(&picture.extra_gpu_data_handle); + let shadow_prim_address = extra_data_address.offset(3); + let shadow_data_address = extra_data_address.offset(6); - let instance = BrushInstance { + let shadow_instance = BrushInstance { picture_address: task_address, - prim_address: prim_cache_address, + prim_address: shadow_prim_address, clip_chain_rect_index, scroll_id, clip_task_address, @@ -755,15 +762,31 @@ impl AlphaBatchBuilder { edge_flags: EdgeAaSegmentMask::empty(), brush_flags: BrushFlags::empty(), user_data: [ - uv_rect_address, - (source_kind as i32) << 16 | + shadow_uv_rect_address, + (BrushImageSourceKind::ColorAlphaMask as i32) << 16 | RasterizationSpace::Screen as i32, - picture.extra_gpu_data_handle.as_int(gpu_cache), + shadow_data_address.as_int(), ], }; - let batch = self.batch_list.get_suitable_batch(key, &task_relative_bounding_rect); - batch.push(PrimitiveInstance::from(instance)); + let content_instance = BrushInstance { + prim_address: prim_cache_address, + user_data: [ + content_uv_rect_address, + (BrushImageSourceKind::Color as i32) << 16 | + RasterizationSpace::Screen as i32, + extra_data_address.as_int(), + ], + ..shadow_instance + }; + + self.batch_list + .get_suitable_batch(shadow_key, &task_relative_bounding_rect) + .push(PrimitiveInstance::from(shadow_instance)); + + self.batch_list + .get_suitable_batch(content_key, &task_relative_bounding_rect) + .push(PrimitiveInstance::from(content_instance)); } false diff --git a/webrender/src/display_list_flattener.rs b/webrender/src/display_list_flattener.rs index 3687580ab1..f71064927a 100644 --- a/webrender/src/display_list_flattener.rs +++ b/webrender/src/display_list_flattener.rs @@ -14,7 +14,6 @@ use api::{RepeatMode, ScrollFrameDisplayItem, ScrollPolicy, ScrollSensitivity, S use api::{SpecificDisplayItem, StackingContext, StickyFrameDisplayItem, TexelRect, TileOffset}; use api::{TransformStyle, YuvColorSpace, YuvData}; use app_units::Au; -use batch::BrushImageSourceKind; use border::ImageBorderSegment; use clip::{ClipRegion, ClipSource, ClipSources, ClipStore}; use clip_scroll_node::{ClipScrollNode, NodeType, StickyFrameInfo}; @@ -1070,11 +1069,7 @@ impl<'a> DisplayListFlattener<'a> { true, ); - let prim = BrushPrimitive::new_picture( - container_index, - BrushImageSourceKind::Color, - LayerVector2D::zero(), - ); + let prim = BrushPrimitive::new_picture(container_index); let prim_index = self.prim_store.add_primitive( &LayerRect::zero(), @@ -1124,34 +1119,7 @@ impl<'a> DisplayListFlattener<'a> { true, ); - // For drop shadows, add an extra brush::picture primitive - // that will draw the picture as an alpha mask. - let shadow_prim_index = match *filter { - FilterOp::DropShadow(offset, ..) => { - let shadow_prim = BrushPrimitive::new_picture( - src_pic_index, - BrushImageSourceKind::ColorAlphaMask, - offset, - ); - Some(self.prim_store.add_primitive( - &LayerRect::zero(), - &max_clip, - is_backface_visible, - None, - None, - PrimitiveContainer::Brush(shadow_prim), - )) - } - _ => { - None - } - }; - - let src_prim = BrushPrimitive::new_picture( - src_pic_index, - BrushImageSourceKind::Color, - LayoutVector2D::zero(), - ); + let src_prim = BrushPrimitive::new_picture(src_pic_index); let src_prim_index = self.prim_store.add_primitive( &LayerRect::zero(), &max_clip, @@ -1164,13 +1132,6 @@ impl<'a> DisplayListFlattener<'a> { let parent_pic = &mut self.prim_store.pictures[parent_pic_index.0]; parent_pic_index = src_pic_index; - if let Some(shadow_prim_index) = shadow_prim_index { - parent_pic.add_primitive( - shadow_prim_index, - clip_and_scroll, - ); - } - parent_pic.add_primitive(src_prim_index, clip_and_scroll); self.picture_stack.push(src_pic_index); @@ -1187,11 +1148,7 @@ impl<'a> DisplayListFlattener<'a> { true, ); - let src_prim = BrushPrimitive::new_picture( - src_pic_index, - BrushImageSourceKind::Color, - LayoutVector2D::zero(), - ); + let src_prim = BrushPrimitive::new_picture(src_pic_index); let src_prim_index = self.prim_store.add_primitive( &LayerRect::zero(), @@ -1247,11 +1204,7 @@ impl<'a> DisplayListFlattener<'a> { ); // Create a brush primitive that draws this picture. - let sc_prim = BrushPrimitive::new_picture( - pic_index, - BrushImageSourceKind::Color, - LayoutVector2D::zero(), - ); + let sc_prim = BrushPrimitive::new_picture(pic_index); // Add the brush to the parent picture. let sc_prim_index = self.prim_store.add_primitive( @@ -1486,11 +1439,7 @@ impl<'a> DisplayListFlattener<'a> { ); // Create the primitive to draw the shadow picture into the scene. - let shadow_prim = BrushPrimitive::new_picture( - shadow_pic_index, - BrushImageSourceKind::Color, - LayoutVector2D::zero(), - ); + let shadow_prim = BrushPrimitive::new_picture(shadow_pic_index); let shadow_prim_index = self.prim_store.add_primitive( &LayerRect::zero(), &max_clip, diff --git a/webrender/src/gpu_cache.rs b/webrender/src/gpu_cache.rs index 3f02703a2c..2e145e8a5c 100644 --- a/webrender/src/gpu_cache.rs +++ b/webrender/src/gpu_cache.rs @@ -151,6 +151,13 @@ impl GpuCacheAddress { v: u16::MAX, } } + + pub fn offset(&self, offset: usize) -> Self { + GpuCacheAddress { + u: self.u + offset as u16, + v: self.v + } + } } impl Add for GpuCacheAddress { diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index b47c15dfcb..7bb03a7108 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use api::{FilterOp, LayerVector2D, MixBlendMode, PipelineId, PremultipliedColorF}; +use api::{FilterOp, MixBlendMode, PipelineId, PremultipliedColorF}; use api::{DeviceIntRect, LayerRect}; use box_shadow::{BLUR_SAMPLE_SCALE}; use clip_scroll_tree::ClipScrollNodeIndex; @@ -162,6 +162,16 @@ impl PicturePrimitive { Some(PictureCompositeMode::Filter(FilterOp::DropShadow(_, blur_radius, _))) => { let inflate_size = (blur_radius * BLUR_SAMPLE_SCALE).ceil(); local_content_rect.inflate(inflate_size, inflate_size) + + // TODO(gw): When we support culling rect being separate from + // the task/screen rect, we should include both the + // content and shadow rect here, which will prevent + // drop-shadows from disappearing if the main content + // rect is not visible. Something like: + // let shadow_rect = local_content_rect + // .inflate(inflate_size, inflate_size) + // .translate(&offset); + // shadow_rect.union(&local_content_rect) } _ => { local_content_rect @@ -392,22 +402,38 @@ impl PicturePrimitive { } if let Some(mut request) = frame_state.gpu_cache.request(&mut self.extra_gpu_data_handle) { + // [GLSL ImageBrush: task_rect, offset, color] request.push(self.task_rect.to_f32()); + request.push([0.0; 4]); + request.push(PremultipliedColorF::WHITE); // TODO(gw): It would make the shaders a bit simpler if the offset // was provided as part of the brush::picture instance, // rather than in the Picture data itself. - let (offset, color) = match self.composite_mode { - Some(PictureCompositeMode::Filter(FilterOp::DropShadow(offset, _, color))) => { - (offset, color.premultiplied()) - } - _ => { - (LayerVector2D::zero(), PremultipliedColorF::WHITE) - } - }; - - request.push([offset.x, offset.y, 0.0, 0.0]); - request.push(color); + if let Some(PictureCompositeMode::Filter(FilterOp::DropShadow(offset, _, color))) = self.composite_mode { + // TODO(gw): This is very hacky code below! It stores an extra + // brush primitive below for the special case of a + // drop-shadow where we need a different local + // rect for the shadow. To tidy this up in future, + // we could consider abstracting the code in prim_store.rs + // that writes a brush primitive header. + + // Basic brush primitive header is (see end of prepare_prim_for_render_inner in prim_store.rs) + // local_rect + // clip_rect + // [segment_rects] + let shadow_rect = prim_metadata.local_rect.translate(&offset); + let shadow_clip_rect = prim_metadata.local_clip_rect.translate(&offset); + + request.push(shadow_rect); + request.push(shadow_clip_rect); + request.push(shadow_rect); + + // Now write another GLSL ImageBrush struct, for the shadow to reference. + request.push(self.task_rect.to_f32()); + request.push([offset.x, offset.y, 0.0, 0.0]); + request.push(color.premultiplied()); + } } } } diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index 021b19913f..d409eef90d 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -7,7 +7,6 @@ use api::{DeviceIntRect, DeviceIntSize, DevicePixelScale, Epoch, ExtendMode, Fon use api::{FilterOp, GlyphInstance, GlyphKey, GradientStop, ImageKey, ImageRendering, ItemRange, ItemTag}; use api::{LayerPoint, LayerRect, LayerSize, LayerToWorldTransform, LayerVector2D}; use api::{PipelineId, PremultipliedColorF, Shadow, YuvColorSpace, YuvFormat}; -use batch::BrushImageSourceKind; use border::{BorderCornerInstance, BorderEdgeKind}; use box_shadow::BLUR_SAMPLE_SCALE; use clip_scroll_tree::{ClipChainIndex, ClipScrollNodeIndex, CoordinateSystemId}; @@ -202,12 +201,6 @@ pub enum BrushKind { Clear, Picture { pic_index: PictureIndex, - // What kind of texels to sample from the - // picture (e.g color or alpha mask). - source_kind: BrushImageSourceKind, - // A local space offset to apply when drawing - // this picture. - local_offset: LayerVector2D, }, Image { request: ImageRequest, @@ -331,16 +324,10 @@ impl BrushPrimitive { } } - pub fn new_picture( - pic_index: PictureIndex, - source_kind: BrushImageSourceKind, - local_offset: LayerVector2D, - ) -> BrushPrimitive { + pub fn new_picture(pic_index: PictureIndex) -> BrushPrimitive { BrushPrimitive { kind: BrushKind::Picture { pic_index, - source_kind, - local_offset, }, segment_desc: None, } @@ -1438,42 +1425,16 @@ impl PrimitiveStore { ); } } - BrushKind::Picture { pic_index, source_kind, .. } => { + BrushKind::Picture { pic_index, .. } => { let pic = &mut self.pictures[pic_index.0]; - // If this picture is referenced by multiple brushes, - // we only want to prepare it once per frame. It - // should be prepared for the main color pass. - // TODO(gw): Make this a bit more explicit - perhaps - // we could mark which brush::picture is - // the owner of the picture, vs the shadow - // which is just referencing it. - match source_kind { - BrushImageSourceKind::Color => { - pic.prepare_for_render( - prim_index, - metadata, - pic_state_for_children, - pic_state, - frame_context, - frame_state, - ); - } - BrushImageSourceKind::ColorAlphaMask => { - // Since we will always visit the shadow - // brush first, use this to clear out the - // render tasks from the previous frame. - // This ensures that if the primary brush - // is found to be non-visible, then we - // won't try to draw the drop-shadow either. - // This isn't quite correct - it can result - // in clipping artifacts if the image is - // off-screen, but the drop-shadow is - // partially visible - we can fix this edge - // case as a follow up. - pic.surface = None; - pic.secondary_render_task_id = None; - } - } + pic.prepare_for_render( + prim_index, + metadata, + pic_state_for_children, + pic_state, + frame_context, + frame_state, + ); } BrushKind::Solid { .. } | BrushKind::Clear => {} @@ -1932,7 +1893,7 @@ impl PrimitiveStore { // local space, which may force us to render this item on a larger // picture target, if being composited. if let PrimitiveKind::Brush = prim_kind { - if let BrushKind::Picture { pic_index, local_offset, .. } = self.cpu_brushes[cpu_prim_index.0].kind { + if let BrushKind::Picture { pic_index, .. } = self.cpu_brushes[cpu_prim_index.0].kind { let pic_context_for_children = { let pic = &mut self.pictures[pic_index.0]; @@ -1987,11 +1948,7 @@ impl PrimitiveStore { pic.runs = pic_context_for_children.prim_runs; let metadata = &mut self.cpu_metadata[prim_index.0]; - // Store local rect of the picture for this brush, - // also applying any local offset for the instance. - metadata.local_rect = pic - .update_local_rect(result) - .translate(&local_offset); + metadata.local_rect = pic.update_local_rect(result); } }