From 41083ad982b820d77fa70b9bc9993d866b969677 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 13 Apr 2018 15:32:52 +0200 Subject: [PATCH 1/5] Simplify repeated primitive parameters when possible. --- webrender/src/display_list_flattener.rs | 41 ++++++++++++++++--------- webrender/src/image.rs | 22 +++++++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/webrender/src/display_list_flattener.rs b/webrender/src/display_list_flattener.rs index e669cd75e6..bcbafec92e 100644 --- a/webrender/src/display_list_flattener.rs +++ b/webrender/src/display_list_flattener.rs @@ -22,7 +22,7 @@ use euclid::{SideOffsets2D, vec2}; use frame_builder::{FrameBuilder, FrameBuilderConfig}; use glyph_rasterizer::FontInstance; use hit_test::{HitTestingItem, HitTestingRun}; -use image::{decompose_image, TiledImageInfo}; +use image::{decompose_image, TiledImageInfo, simplify_repeated_primitive}; use internal_types::{FastHashMap, FastHashSet}; use picture::PictureCompositeMode; use prim_store::{BrushKind, BrushPrimitive, BrushSegmentDescriptor, CachedGradient}; @@ -1892,11 +1892,18 @@ impl<'a> DisplayListFlattener<'a> { stops_count: usize, extend_mode: ExtendMode, stretch_size: LayoutSize, - tile_spacing: LayoutSize, + mut tile_spacing: LayoutSize, ) { let gradient_index = CachedGradientIndex(self.cached_gradients.len()); self.cached_gradients.push(CachedGradient::new()); + let mut prim_rect = info.rect; + simplify_repeated_primitive(&stretch_size, &mut tile_spacing, &mut prim_rect); + let info = LayoutPrimitiveInfo { + rect: prim_rect, + .. *info + }; + if tile_spacing != LayoutSize::zero() { let prim_infos = info.decompose( stretch_size, @@ -1925,7 +1932,7 @@ impl<'a> DisplayListFlattener<'a> { self.add_gradient_impl( clip_and_scroll, - info, + &info, start_point, end_point, stops, @@ -1982,11 +1989,18 @@ impl<'a> DisplayListFlattener<'a> { stops: ItemRange, extend_mode: ExtendMode, stretch_size: LayoutSize, - tile_spacing: LayoutSize, + mut tile_spacing: LayoutSize, ) { let gradient_index = CachedGradientIndex(self.cached_gradients.len()); self.cached_gradients.push(CachedGradient::new()); + let mut prim_rect = info.rect; + simplify_repeated_primitive(&stretch_size, &mut tile_spacing, &mut prim_rect); + let info = LayoutPrimitiveInfo { + rect: prim_rect, + .. *info + }; + if tile_spacing != LayoutSize::zero() { let prim_infos = info.decompose( stretch_size, @@ -2016,7 +2030,7 @@ impl<'a> DisplayListFlattener<'a> { self.add_radial_gradient_impl( clip_and_scroll, - info, + &info, center, start_radius, end_radius, @@ -2123,13 +2137,12 @@ impl<'a> DisplayListFlattener<'a> { alpha_type: AlphaType, tile_offset: Option, ) { - // If the tile spacing is the same as the rect size, - // then it is effectively zero. We use this later on - // in prim_store to detect if an image can be considered - // opaque. - if tile_spacing == info.rect.size { - tile_spacing = LayoutSize::zero(); - } + let mut prim_rect = info.rect; + simplify_repeated_primitive(&stretch_size, &mut tile_spacing, &mut prim_rect); + let info = LayoutPrimitiveInfo { + rect: prim_rect, + .. *info + }; let request = ImageRequest { key: image_key, @@ -2170,7 +2183,7 @@ impl<'a> DisplayListFlattener<'a> { self.add_primitive( clip_and_scroll, - info, + &info, Vec::new(), PrimitiveContainer::Brush(prim), ); @@ -2189,7 +2202,7 @@ impl<'a> DisplayListFlattener<'a> { self.add_primitive( clip_and_scroll, - info, + &info, Vec::new(), PrimitiveContainer::Image(prim_cpu), ); diff --git a/webrender/src/image.rs b/webrender/src/image.rs index 4ada110452..dcf54877da 100644 --- a/webrender/src/image.rs +++ b/webrender/src/image.rs @@ -5,6 +5,28 @@ use api::{TileOffset, LayoutRect, LayoutSize, LayoutVector2D, DeviceUintSize}; use euclid::rect; +/// If repetitions are far enough apart that only one is within +/// the primitive rect, then we can simplify the parameters and +/// treat the primitive as not repeated. +/// This can let us avoid unnecessary work later to handle some +/// of the parameters. +pub fn simplify_repeated_primitive( + stretch_size: &LayoutSize, + tile_spacing: &mut LayoutSize, + prim_rect: &mut LayoutRect, +) { + let stride = *stretch_size + *tile_spacing; + + if stride.width >= prim_rect.size.width { + tile_spacing.width = 0.0; + prim_rect.size.width = f32::min(prim_rect.size.width, stretch_size.width); + } + if stride.height >= prim_rect.size.height { + tile_spacing.height = 0.0; + prim_rect.size.height = f32::min(prim_rect.size.height, stretch_size.height); + } +} + pub struct DecomposedTile { pub rect: LayoutRect, pub stretch_size: LayoutSize, From bdce7955fe06a27890ec2b88172847a5b9a06f08 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 16 Apr 2018 14:49:24 +0200 Subject: [PATCH 2/5] Bake repeated image spacing using a render task. --- webrender/src/device.rs | 4 +- webrender/src/display_list_flattener.rs | 3 +- webrender/src/prim_store.rs | 53 +++++++++++++++++++++---- webrender/src/renderer.rs | 3 +- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/webrender/src/device.rs b/webrender/src/device.rs index 8bfcde406e..a49bf76ef7 100644 --- a/webrender/src/device.rs +++ b/webrender/src/device.rs @@ -1250,8 +1250,8 @@ impl Device { src_rect.origin.y + src_rect.size.height, dest_rect.origin.x, dest_rect.origin.y, - dest_rect.origin.x + dest_rect.size.width, - dest_rect.origin.y + dest_rect.size.height, + dest_rect.origin.x + src_rect.size.width, + dest_rect.origin.y + src_rect.size.height, gl::COLOR_BUFFER_BIT, gl::LINEAR, ); diff --git a/webrender/src/display_list_flattener.rs b/webrender/src/display_list_flattener.rs index bcbafec92e..da064b6eaa 100644 --- a/webrender/src/display_list_flattener.rs +++ b/webrender/src/display_list_flattener.rs @@ -2165,8 +2165,7 @@ impl<'a> DisplayListFlattener<'a> { // See if conditions are met to run through the new // image brush shader, which supports segments. - if tile_spacing == LayoutSize::zero() && - tile_offset.is_none() { + if tile_offset.is_none() { let prim = BrushPrimitive::new( BrushKind::Image { request, diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index 6213cc5184..3e275b2e29 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -430,10 +430,15 @@ impl BrushPrimitive { } // Images are drawn as a white color, modulated by the total // opacity coming from any collapsed property bindings. - BrushKind::Image { stretch_size, ref opacity_binding, .. } => { + BrushKind::Image { stretch_size, tile_spacing, ref opacity_binding, .. } => { request.push(ColorF::new(1.0, 1.0, 1.0, opacity_binding.current).premultiplied()); request.push(PremultipliedColorF::WHITE); - request.push([stretch_size.width, stretch_size.height, 0.0, 0.0]); + request.push([ + stretch_size.width + tile_spacing.width, + stretch_size.height + tile_spacing.height, + 0.0, + 0.0, + ]); } // Solid rects also support opacity collapsing. BrushKind::Solid { color, ref opacity_binding, .. } => { @@ -1519,7 +1524,16 @@ impl PrimitiveStore { let brush = &mut self.cpu_brushes[metadata.cpu_prim_index.0]; match brush.kind { - BrushKind::Image { request, sub_rect, ref mut current_epoch, ref mut source, ref mut opacity_binding, .. } => { + BrushKind::Image { + request, + sub_rect, + stretch_size, + ref mut tile_spacing, + ref mut current_epoch, + ref mut source, + ref mut opacity_binding, + .. + } => { let image_properties = frame_state .resource_cache .get_image_properties(request.key); @@ -1540,6 +1554,17 @@ impl PrimitiveStore { image_properties.descriptor.is_opaque && opacity_binding.current == 1.0; + if *tile_spacing != LayoutSize::zero() { + *source = ImageSource::Cache { + // Size in device-pixels we need to allocate in render task cache. + size: DeviceIntSize::new( + image_properties.descriptor.width as i32, + image_properties.descriptor.height as i32 + ), + handle: None, + }; + } + // Work out whether this image is a normal / simple type, or if // we need to pre-render it to the render task cache. if let Some(rect) = sub_rect { @@ -1557,7 +1582,21 @@ impl PrimitiveStore { // time through, and any time the render task output has been // evicted from the texture cache. match *source { - ImageSource::Cache { size, ref mut handle } => { + ImageSource::Cache { ref mut size, ref mut handle } => { + let padding_x = (tile_spacing.width * size.width as f32 / + stretch_size.width) as i32; + let padding_y = (tile_spacing.height * size.height as f32 / + stretch_size.height) as i32; + + if padding_x > 0 { + metadata.opacity.is_opaque = false; + size.width += padding_x; + } + if padding_y > 0 { + metadata.opacity.is_opaque = false; + size.height += padding_y; + } + let image_cache_key = ImageCacheKey { request, texel_rect: sub_rect, @@ -1566,7 +1605,7 @@ impl PrimitiveStore { // Request a pre-rendered image task. *handle = Some(frame_state.resource_cache.request_render_task( RenderTaskCacheKey { - size, + size: *size, kind: RenderTaskCacheKeyKind::Image(image_cache_key), }, frame_state.gpu_cache, @@ -1582,7 +1621,7 @@ impl PrimitiveStore { // a normal transient render task surface. This will // copy only the sub-rect, if specified. let cache_to_target_task = RenderTask::new_blit( - size, + *size, BlitSource::Image { key: image_cache_key }, ); let cache_to_target_task_id = render_tasks.add(cache_to_target_task); @@ -1591,7 +1630,7 @@ impl PrimitiveStore { // task above back into the right spot in the persistent // render target cache. let target_to_cache_task = RenderTask::new_blit( - size, + *size, BlitSource::RenderTask { task_id: cache_to_target_task_id, }, diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index c7066108dc..8940e0b5ae 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2696,7 +2696,8 @@ impl Renderer { source_rect } }; - debug_assert_eq!(source_rect.size, blit.target_rect.size); + debug_assert!(source_rect.size.width <= blit.target_rect.size.width); + debug_assert!(source_rect.size.height <= blit.target_rect.size.height); self.device.blit_render_target( source_rect, blit.target_rect, From 6fe8e93fc1045fcbd5f4246d53e2d80247826208 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 27 Apr 2018 17:26:37 +0200 Subject: [PATCH 3/5] Support padding in blit render tasks. --- webrender/src/device.rs | 4 ++-- webrender/src/prim_store.rs | 30 ++++++++++++++++-------------- webrender/src/render_task.rs | 15 ++++++++++++++- webrender/src/renderer.rs | 3 +-- webrender/src/tiling.rs | 4 ++-- webrender_api/src/units.rs | 3 ++- 6 files changed, 37 insertions(+), 22 deletions(-) diff --git a/webrender/src/device.rs b/webrender/src/device.rs index a49bf76ef7..8bfcde406e 100644 --- a/webrender/src/device.rs +++ b/webrender/src/device.rs @@ -1250,8 +1250,8 @@ impl Device { src_rect.origin.y + src_rect.size.height, dest_rect.origin.x, dest_rect.origin.y, - dest_rect.origin.x + src_rect.size.width, - dest_rect.origin.y + src_rect.size.height, + dest_rect.origin.x + dest_rect.size.width, + dest_rect.origin.y + dest_rect.size.height, gl::COLOR_BUFFER_BIT, gl::LINEAR, ); diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index 3e275b2e29..b4577ff399 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -6,7 +6,7 @@ use api::{AlphaType, BorderRadius, BoxShadowClipMode, BuiltDisplayList, ClipMode use api::{DeviceIntRect, DeviceIntSize, DevicePixelScale, Epoch, ExtendMode, FontRenderMode}; use api::{FilterOp, GlyphInstance, GlyphKey, GradientStop, ImageKey, ImageRendering, ItemRange, ItemTag}; use api::{GlyphRasterSpace, LayoutPoint, LayoutRect, LayoutSize, LayoutToWorldTransform, LayoutVector2D}; -use api::{PipelineId, PremultipliedColorF, PropertyBinding, Shadow, YuvColorSpace, YuvFormat}; +use api::{PipelineId, PremultipliedColorF, PropertyBinding, Shadow, YuvColorSpace, YuvFormat, DeviceIntSideOffsets}; use border::{BorderCornerInstance, BorderEdgeKind}; use box_shadow::BLUR_SAMPLE_SCALE; use clip_scroll_tree::{ClipChainIndex, ClipScrollNodeIndex, CoordinateSystemId}; @@ -1583,18 +1583,19 @@ impl PrimitiveStore { // evicted from the texture cache. match *source { ImageSource::Cache { ref mut size, ref mut handle } => { - let padding_x = (tile_spacing.width * size.width as f32 / - stretch_size.width) as i32; - let padding_y = (tile_spacing.height * size.height as f32 / - stretch_size.height) as i32; + let padding = DeviceIntSideOffsets::new( + 0, + (tile_spacing.width * size.width as f32 / stretch_size.width) as i32, + (tile_spacing.height * size.height as f32 / stretch_size.height) as i32, + 0, + ); - if padding_x > 0 { - metadata.opacity.is_opaque = false; - size.width += padding_x; - } - if padding_y > 0 { + let inner_size = *size; + size.width += padding.horizontal(); + size.height += padding.vertical(); + + if padding != DeviceIntSideOffsets::zero() { metadata.opacity.is_opaque = false; - size.height += padding_y; } let image_cache_key = ImageCacheKey { @@ -1621,7 +1622,7 @@ impl PrimitiveStore { // a normal transient render task surface. This will // copy only the sub-rect, if specified. let cache_to_target_task = RenderTask::new_blit( - *size, + inner_size, BlitSource::Image { key: image_cache_key }, ); let cache_to_target_task_id = render_tasks.add(cache_to_target_task); @@ -1629,8 +1630,9 @@ impl PrimitiveStore { // Create a task to blit the rect from the child render // task above back into the right spot in the persistent // render target cache. - let target_to_cache_task = RenderTask::new_blit( - *size, + let target_to_cache_task = RenderTask::new_blit_with_padding( + inner_size, + &padding, BlitSource::RenderTask { task_id: cache_to_target_task_id, }, diff --git a/webrender/src/render_task.rs b/webrender/src/render_task.rs index da8eb19bfa..58228c5a87 100644 --- a/webrender/src/render_task.rs +++ b/webrender/src/render_task.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::{DeviceIntPoint, DeviceIntRect, DeviceIntSize, DeviceSize, ImageDescriptor, ImageFormat}; +use api::{DeviceIntPoint, DeviceIntRect, DeviceIntSize, DeviceSize, DeviceIntSideOffsets, ImageDescriptor, ImageFormat}; #[cfg(feature = "pathfinder")] use api::FontRenderMode; use box_shadow::{BoxShadowCacheKey}; @@ -254,6 +254,7 @@ pub enum BlitSource { #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct BlitTask { pub source: BlitSource, + pub padding: DeviceIntSideOffsets, } #[derive(Debug)] @@ -337,6 +338,14 @@ impl RenderTask { pub fn new_blit( size: DeviceIntSize, source: BlitSource, + ) -> Self { + RenderTask::new_blit_with_padding(size, &DeviceIntSideOffsets::zero(), source) + } + + pub fn new_blit_with_padding( + mut size: DeviceIntSize, + padding: &DeviceIntSideOffsets, + source: BlitSource, ) -> Self { let mut children = Vec::new(); @@ -349,11 +358,15 @@ impl RenderTask { children.push(task_id); } + size.width += padding.horizontal(); + size.height += padding.vertical(); + RenderTask { children, location: RenderTaskLocation::Dynamic(None, Some(size)), kind: RenderTaskKind::Blit(BlitTask { source, + padding: *padding, }), clear_mode: ClearMode::Transparent, saved_index: None, diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 8940e0b5ae..c7066108dc 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2696,8 +2696,7 @@ impl Renderer { source_rect } }; - debug_assert!(source_rect.size.width <= blit.target_rect.size.width); - debug_assert!(source_rect.size.height <= blit.target_rect.size.height); + debug_assert_eq!(source_rect.size, blit.target_rect.size); self.device.blit_render_target( source_rect, blit.target_rect, diff --git a/webrender/src/tiling.rs b/webrender/src/tiling.rs index ebaf5721b8..a154961e33 100644 --- a/webrender/src/tiling.rs +++ b/webrender/src/tiling.rs @@ -484,7 +484,7 @@ impl RenderTarget for ColorRenderTarget { cache_item.texture_layer, source_rect, ), - target_rect, + target_rect: target_rect.inner_rect(task_info.padding) }); } BlitSource::RenderTask { .. } => { @@ -673,7 +673,7 @@ impl TextureCacheRenderTarget { // task to this target. self.blits.push(BlitJob { source: BlitJobSource::RenderTask(task_id), - target_rect: target_rect.0, + target_rect: target_rect.0.inner_rect(task_info.padding), }); } } diff --git a/webrender_api/src/units.rs b/webrender_api/src/units.rs index 7e4916a306..a1946f4a6c 100644 --- a/webrender_api/src/units.rs +++ b/webrender_api/src/units.rs @@ -14,7 +14,7 @@ use app_units::Au; use euclid::{Length, TypedRect, TypedScale, TypedSize2D, TypedTransform3D}; -use euclid::{TypedPoint2D, TypedPoint3D, TypedVector2D, TypedVector3D}; +use euclid::{TypedPoint2D, TypedPoint3D, TypedVector2D, TypedVector3D, TypedSideOffsets2D}; /// Geometry in the coordinate system of the render target (screen or intermediate /// surface) in physical pixels. @@ -25,6 +25,7 @@ pub type DeviceIntRect = TypedRect; pub type DeviceIntPoint = TypedPoint2D; pub type DeviceIntSize = TypedSize2D; pub type DeviceIntLength = Length; +pub type DeviceIntSideOffsets = TypedSideOffsets2D; pub type DeviceUintRect = TypedRect; pub type DeviceUintPoint = TypedPoint2D; From 421fd07a8ad7c9bab56c4e316e7b3c75171701a0 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 30 Apr 2018 17:48:53 +0200 Subject: [PATCH 4/5] Bake tile spacing in the first blit to avoid artifacts. It would seem more efficient to apply tile spacing in the second blit and avoid the memory overhead of baking the tile spacing in the first render task, but clearing seems to be handled differently on the two blits (absent in the second). --- webrender/src/prim_store.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/webrender/src/prim_store.rs b/webrender/src/prim_store.rs index b4577ff399..e868ff6b18 100644 --- a/webrender/src/prim_store.rs +++ b/webrender/src/prim_store.rs @@ -1621,8 +1621,9 @@ impl PrimitiveStore { // Create a task to blit from the texture cache to // a normal transient render task surface. This will // copy only the sub-rect, if specified. - let cache_to_target_task = RenderTask::new_blit( + let cache_to_target_task = RenderTask::new_blit_with_padding( inner_size, + &padding, BlitSource::Image { key: image_cache_key }, ); let cache_to_target_task_id = render_tasks.add(cache_to_target_task); @@ -1630,9 +1631,8 @@ impl PrimitiveStore { // Create a task to blit the rect from the child render // task above back into the right spot in the persistent // render target cache. - let target_to_cache_task = RenderTask::new_blit_with_padding( - inner_size, - &padding, + let target_to_cache_task = RenderTask::new_blit( + *size, BlitSource::RenderTask { task_id: cache_to_target_task_id, }, From c9e4f808a2e99703e59185a0f91365447e3d8116 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 2 May 2018 14:36:07 +0200 Subject: [PATCH 5/5] Update reftest reference.