From faaf772402e4e46b5c93a87a9a89b184caacbed4 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Thu, 19 Mar 2020 04:01:47 +0000 Subject: [PATCH 1/7] Bug 1623353 - implement SWGL scaled blit. r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D67336 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/c7b34aa0386b29d739b92d1ae40399c306b67f86 --- swgl/src/gl.cc | 80 ++++++++++++++++++++++++++++++++++++++++---- swgl/src/swgl_fns.rs | 2 +- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/swgl/src/gl.cc b/swgl/src/gl.cc index 07d9044f8b..d8e346c209 100644 --- a/swgl/src/gl.cc +++ b/swgl/src/gl.cc @@ -2009,6 +2009,74 @@ void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, xoffset, yoffset, 0, width, height, 1); } +} // extern "C" + +template +static void scale_row(P* dst, int dstWidth, const P* src, int srcWidth) { + int frac = 0; + for (int x = 0; x < dstWidth; x++) { + *dst = *src; + dst++; + frac += srcWidth; + while (frac >= dstWidth) { + frac -= dstWidth; + src++; + } + } +} + +static void scale_blit(GLuint srcName, GLint srcX, GLint srcY, GLint srcZ, + GLsizei srcWidth, GLsizei srcHeight, + GLuint dstName, GLint dstX, GLint dstY, GLint dstZ, + GLsizei dstWidth, GLsizei dstHeight) { + Texture& srctex = ctx->textures[srcName]; + if (!srctex.buf) return; + prepare_texture(srctex); + Texture& dsttex = ctx->textures[dstName]; + if (!dsttex.buf) return; + IntRect skip = {dstX, dstY, dstWidth, abs(dstHeight)}; + prepare_texture(dsttex, &skip); + assert(srctex.internal_format == dsttex.internal_format); + assert(srcX + srcWidth <= srctex.width); + assert(srcY + srcHeight <= srctex.height); + assert(srcZ < max(srctex.depth, 1)); + assert(dstX + dstWidth <= dsttex.width); + assert(max(dstY, dstY + dstHeight) <= dsttex.height); + assert(dstZ < max(dsttex.depth, 1)); + int bpp = srctex.bpp(); + int srcStride = srctex.stride(bpp); + int destStride = dsttex.stride(bpp); + char* dest = dsttex.buf + (dsttex.height * dstZ + dstY) * destStride + + dstX * bpp; + char* src = srctex.buf + (srctex.height * srcZ + srcY) * srcStride + + srcX * bpp; + if (dstHeight < 0) { + dest -= destStride; + destStride = -destStride; + dstHeight = -dstHeight; + } + int frac = 0; + for (int y = 0; y < dstHeight; y++) { + if (srcWidth == dstWidth) { + memcpy(dest, src, srcWidth * bpp); + } else { + switch (bpp) { + case 1: scale_row((uint8_t*)dest, dstWidth, (uint8_t*)src, srcWidth); break; + case 2: scale_row((uint16_t*)dest, dstWidth, (uint16_t*)src, srcWidth); break; + case 4: scale_row((uint32_t*)dest, dstWidth, (uint32_t*)src, srcWidth); break; + } + } + dest += destStride; + frac += srcHeight; + while (frac >= dstHeight) { + frac -= dstHeight; + src += srcStride; + } + } +} + +extern "C" { + void BlitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter) { @@ -2017,13 +2085,11 @@ void BlitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, if (!srcfb) return; Framebuffer* dstfb = get_framebuffer(GL_DRAW_FRAMEBUFFER); if (!dstfb) return; - int dstWidth = dstX1 - dstX0; - int dstHeight = dstY1 - dstY0; - assert(srcX1 - srcX0 == dstWidth && srcY1 - srcY0 == abs(dstHeight)); - CopyImageSubData(srcfb->color_attachment, GL_TEXTURE_2D_ARRAY, 0, srcX0, - srcY0, srcfb->layer, dstfb->color_attachment, - GL_TEXTURE_2D_ARRAY, 0, dstX0, dstY0, dstfb->layer, dstWidth, - dstHeight, 1); + // TODO: support linear filtering + scale_blit(srcfb->color_attachment, srcX0, srcY0, srcfb->layer, + srcX1 - srcX0, srcY1 - srcY0, + dstfb->color_attachment, dstX0, dstY0, dstfb->layer, + dstX1 - dstX0, dstY1 - dstY0); } } // extern "C" diff --git a/swgl/src/swgl_fns.rs b/swgl/src/swgl_fns.rs index 0d5dd80faa..5192c8d031 100644 --- a/swgl/src/swgl_fns.rs +++ b/swgl/src/swgl_fns.rs @@ -577,7 +577,7 @@ impl Gl for Context { format: GLenum, pixel_type: GLenum, ) { - panic!(); + ReadPixels(x, y, width, height, format, pixel_type, ptr::null_mut()); } fn sample_coverage(&self, value: GLclampf, invert: bool) { From 14df295b6e61ec6ea8a494b6ee97951103188d0a Mon Sep 17 00:00:00 2001 From: David Michael Date: Thu, 19 Mar 2020 04:01:56 +0000 Subject: [PATCH 2/7] Bug 1623407 - Replace AtomicU64 with AtomicUsize r=kats PowerPC and MIPS do not have AtomicU64, but AtomicUsize is a more portable type that can be used as a replacement. [import_pr] From https://github.com/servo/webrender/pull/3884 Differential Revision: https://phabricator.services.mozilla.com/D67365 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/ff701e82864569f328f9466ba5fdbfb0273354d6 --- webrender/src/resource_cache.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webrender/src/resource_cache.rs b/webrender/src/resource_cache.rs index c6a2da8a6d..20bf26ef93 100644 --- a/webrender/src/resource_cache.rs +++ b/webrender/src/resource_cache.rs @@ -44,7 +44,7 @@ use std::os::raw::c_void; #[cfg(any(feature = "capture", feature = "replay"))] use std::path::PathBuf; use std::sync::{Arc, RwLock}; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::SystemTime; use std::u32; use crate::texture_cache::{TextureCache, TextureCacheHandle, Eviction}; @@ -53,7 +53,7 @@ use crate::util::drain_filter; const DEFAULT_TILE_SIZE: TileSize = 512; // Counter for generating unique native surface ids -static NEXT_NATIVE_SURFACE_ID: AtomicU64 = AtomicU64::new(0); +static NEXT_NATIVE_SURFACE_ID: AtomicUsize = AtomicUsize::new(0); #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] @@ -1787,7 +1787,7 @@ impl ResourceCache { tile_size: DeviceIntSize, is_opaque: bool, ) -> NativeSurfaceId { - let id = NativeSurfaceId(NEXT_NATIVE_SURFACE_ID.fetch_add(1, Ordering::Relaxed)); + let id = NativeSurfaceId(NEXT_NATIVE_SURFACE_ID.fetch_add(1, Ordering::Relaxed) as u64); self.pending_native_surface_updates.push( NativeSurfaceOperation { From 8f2ae15e145328eda92b1b8998e46b2072062c8b Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Thu, 19 Mar 2020 04:02:05 +0000 Subject: [PATCH 3/7] Bug 1614655 - Part 1: Remove item_key from CommonItemProperties r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D66442 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/10897d6106a85383569ac03d8d14907a4c063d9b --- webrender_api/src/display_item.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs index c87249df6d..abc4dd22d2 100644 --- a/webrender_api/src/display_item.rs +++ b/webrender_api/src/display_item.rs @@ -78,8 +78,6 @@ pub struct CommonItemProperties { pub hit_info: Option, /// Various flags describing properties of this primitive. pub flags: PrimitiveFlags, - /// The unique id of this display item. - pub item_key: Option } impl CommonItemProperties { @@ -94,7 +92,6 @@ impl CommonItemProperties { clip_id: space_and_clip.clip_id, hit_info: None, flags: PrimitiveFlags::default(), - item_key: None, } } } From 9ba9b1edfdbc9abbf7fa899012f5bf54829b0f71 Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Thu, 19 Mar 2020 04:02:17 +0000 Subject: [PATCH 4/7] Bug 1614655 - Part 2: Allow 1:n mapping of Gecko - WR display items r=jrmuizel This patch changes the underlying storage for WR display items in DisplayItemCache from Vec to Vec>. This allows storing multiple WebRender display items for one Gecko display item. The storage is populated by traversing BuiltDisplayList extra data portion in display list format, which is roughly as follows: RetainedItems(key k1) Item1(..) RetainedItems(key k2) ItemN(..) ItemN+1(..) This would store Item1 under key k1, and ItemN and ItemN+1 under the key k2, where k1 and k2 are arbitrary unique identifiers (currently of type uint16_t). The entries in the storage are accessed by DisplayItemCache::get_iterator(key), which is called by BuiltDisplayListIter, whenever it encounters a display item DisplayItem::ReuseItems(key). Differential Revision: https://phabricator.services.mozilla.com/D66443 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/cc263e909c61d2d9c7d7cc1ccb8d24c8d09bf5b7 --- webrender/src/scene_building.rs | 5 +- webrender_api/src/display_item.rs | 8 +- webrender_api/src/display_item_cache.rs | 97 ++++------- webrender_api/src/display_list.rs | 215 +++++++++++++++++------- 4 files changed, 201 insertions(+), 124 deletions(-) diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index 257c8c02d8..b39dd8e86a 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -1510,8 +1510,9 @@ impl<'a> SceneBuilder<'a> { unreachable!("Should have returned in parent method.") } - DisplayItem::ReuseItem(..) => { - unreachable!("Iterator logic error") + DisplayItem::ReuseItems(key) | + DisplayItem::RetainedItems(key) => { + unreachable!("Iterator logic error: {:?}", key); } DisplayItem::PushShadow(info) => { diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs index abc4dd22d2..7863604ea4 100644 --- a/webrender_api/src/display_item.rs +++ b/webrender_api/src/display_item.rs @@ -162,7 +162,8 @@ pub enum DisplayItem { PopStackingContext, PopAllShadows, - ReuseItem(ItemKey), + ReuseItems(ItemKey), + RetainedItems(ItemKey), } /// This is a "complete" version of the DisplayItem, with all implicit trailing @@ -204,8 +205,6 @@ pub enum DebugDisplayItem { PopReferenceFrame, PopStackingContext, PopAllShadows, - - ReuseItem(ItemKey), } #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)] @@ -1502,7 +1501,8 @@ impl DisplayItem { DisplayItem::Rectangle(..) => "rectangle", DisplayItem::ScrollFrame(..) => "scroll_frame", DisplayItem::SetGradientStops => "set_gradient_stops", - DisplayItem::ReuseItem(..) => "reuse_item", + DisplayItem::ReuseItems(..) => "reuse_item", + DisplayItem::RetainedItems(..) => "retained_items", DisplayItem::StickyFrame(..) => "sticky_frame", DisplayItem::Text(..) => "text", DisplayItem::YuvImage(..) => "yuv_image", diff --git a/webrender_api/src/display_item_cache.rs b/webrender_api/src/display_item_cache.rs index 9118bd7c4a..3f85f0c68f 100644 --- a/webrender_api/src/display_item_cache.rs +++ b/webrender_api/src/display_item_cache.rs @@ -4,8 +4,6 @@ use crate::display_item::*; use crate::display_list::*; -#[cfg(debug_assertions)] -use std::collections::HashSet; #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct CachedDisplayItem { @@ -14,7 +12,7 @@ pub struct CachedDisplayItem { } impl CachedDisplayItem { - pub fn item(&self) -> &DisplayItem { + pub fn display_item(&self) -> &DisplayItem { &self.item } @@ -40,92 +38,71 @@ impl From> for CachedDisplayItem { } } -fn key_from_item(item: &DisplayItem) -> ItemKey { - let key = match item { - DisplayItem::Rectangle(ref info) => info.common.item_key, - DisplayItem::ClearRectangle(ref info) => info.common.item_key, - DisplayItem::HitTest(ref info) => info.common.item_key, - DisplayItem::Text(ref info) => info.common.item_key, - DisplayItem::Image(ref info) => info.common.item_key, - _ => unimplemented!("Unexpected item: {:?}", item) - }; - - key.expect("Cached item without a key") +#[derive(Clone, Deserialize, Serialize)] +struct CacheEntry { + items: Vec, + occupied: bool, } #[derive(Clone, Deserialize, Serialize)] pub struct DisplayItemCache { - items: Vec>, - - #[cfg(debug_assertions)] - keys: HashSet, + entries: Vec, } impl DisplayItemCache { - fn grow_if_needed( - &mut self, - capacity: usize - ) { - if capacity > self.items.len() { - self.items.resize_with(capacity, || None::); - // println!("Current cache size: {:?} elements, {:?} bytes", - // capacity, std::mem::size_of::() * capacity); - } + fn add_item(&mut self, key: ItemKey, item: CachedDisplayItem) { + let mut entry = &mut self.entries[key as usize]; + entry.items.push(item); + entry.occupied = true; } - fn add_item( - &mut self, - item: CachedDisplayItem, - key: ItemKey, - ) { - self.items[key as usize] = Some(item); + fn clear_entry(&mut self, key: ItemKey) { + let mut entry = &mut self.entries[key as usize]; + entry.items.clear(); + entry.occupied = false; } - pub fn get_item( - &self, - key: ItemKey - ) -> Option<&CachedDisplayItem> { - self.items[key as usize].as_ref() + fn grow_if_needed(&mut self, capacity: usize) { + if capacity > self.entries.len() { + self.entries.resize_with(capacity, || CacheEntry { + items: Vec::new(), + occupied: false, + }); + } + } + + pub fn get_items(&self, key: ItemKey) -> &[CachedDisplayItem] { + let entry = &self.entries[key as usize]; + debug_assert!(entry.occupied); + entry.items.as_slice() } pub fn new() -> Self { Self { - items: Vec::new(), - - #[cfg(debug_assertions)] - /// Used to check that there is only one item per key. - keys: HashSet::new(), + entries: Vec::new(), } } - pub fn update( - &mut self, - display_list: &BuiltDisplayList - ) { + pub fn update(&mut self, display_list: &BuiltDisplayList) { self.grow_if_needed(display_list.cache_size()); let mut iter = display_list.extra_data_iter(); - - #[cfg(debug_assertions)] - { - self.keys.clear(); - } - + let mut current_key: Option = None; loop { let item = match iter.next() { Some(item) => item, None => break, }; - let item_key = key_from_item(item.item()); - let cached_item = CachedDisplayItem::from(item); - - #[cfg(debug_assertions)] - { - debug_assert!(self.keys.insert(item_key)); + if let DisplayItem::RetainedItems(key) = item.item() { + current_key = Some(*key); + self.clear_entry(*key); + continue; } - self.add_item(cached_item, item_key); + let key = current_key.expect("Missing RetainedItems marker"); + let cached_item = CachedDisplayItem::from(item); + self.add_item(key, cached_item); } } } diff --git a/webrender_api/src/display_list.rs b/webrender_api/src/display_list.rs index 9cf4dd8933..eddd9b3e9a 100644 --- a/webrender_api/src/display_list.rs +++ b/webrender_api/src/display_list.rs @@ -204,6 +204,8 @@ pub struct BuiltDisplayListIter<'a> { list: &'a BuiltDisplayList, data: &'a [u8], cache: Option<&'a DisplayItemCache>, + pending_items: std::slice::Iter<'a, CachedDisplayItem>, + cur_cached_item: Option<&'a CachedDisplayItem>, cur_item: di::DisplayItem, cur_stops: ItemRange<'a, di::GradientStop>, cur_glyphs: ItemRange<'a, GlyphInstance>, @@ -284,29 +286,21 @@ pub struct ItemStats { pub struct DisplayItemRef<'a: 'b, 'b> { iter: &'b BuiltDisplayListIter<'a>, - cached_item: Option<&'a CachedDisplayItem>, } // Some of these might just become ItemRanges impl<'a, 'b> DisplayItemRef<'a, 'b> { - fn cached_or_iter_data( - &self, - data: ItemRange<'a, T> - ) -> ItemRange<'a, T> { - self.cached_item.map_or(data, |i| i.data_as_item_range()) - } - pub fn display_list(&self) -> &BuiltDisplayList { self.iter.display_list() } // Creates a new iterator where this element's iterator is, to hack around borrowck. pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> { - BuiltDisplayListIter::new(self.iter.list, self.iter.data, self.iter.cache) + self.iter.sub_iter() } pub fn item(&self) -> &di::DisplayItem { - self.cached_item.map_or(&self.iter.cur_item, |i| i.item()) + self.iter.current_item() } pub fn clip_chain_items(&self) -> ItemRange { @@ -318,11 +312,11 @@ impl<'a, 'b> DisplayItemRef<'a, 'b> { } pub fn glyphs(&self) -> ItemRange { - self.cached_or_iter_data(self.iter.cur_glyphs) + self.iter.glyphs() } pub fn gradient_stops(&self) -> ItemRange { - self.cached_or_iter_data(self.iter.cur_stops) + self.iter.gradient_stops() } pub fn filters(&self) -> ItemRange { @@ -493,7 +487,8 @@ impl BuiltDisplayList { Real::PopReferenceFrame => Debug::PopReferenceFrame, Real::PopStackingContext => Debug::PopStackingContext, Real::PopAllShadows => Debug::PopAllShadows, - Real::ReuseItem(_) => unreachable!("Unexpected item"), + Real::ReuseItems(_) | + Real::RetainedItems(_) => unreachable!("Unexpected item"), }; seq.serialize_element(&serial_di)? } @@ -526,6 +521,8 @@ impl<'a> BuiltDisplayListIter<'a> { list, data, cache, + pending_items: [].iter(), + cur_cached_item: None, cur_item: di::DisplayItem::PopStackingContext, cur_stops: ItemRange::default(), cur_glyphs: ItemRange::default(), @@ -538,14 +535,52 @@ impl<'a> BuiltDisplayListIter<'a> { debug_stats: DebugStats { last_addr: data.as_ptr() as usize, stats: HashMap::default(), - } + }, } } + pub fn sub_iter(&self) -> Self { + let mut iter = BuiltDisplayListIter::new( + self.list, self.data, self.cache + ); + iter.pending_items = self.pending_items.clone(); + iter + } + pub fn display_list(&self) -> &'a BuiltDisplayList { self.list } + pub fn current_item(&self) -> &di::DisplayItem { + match self.cur_cached_item { + Some(cached_item) => cached_item.display_item(), + None => &self.cur_item + } + } + + fn cached_item_range_or( + &self, + data: ItemRange<'a, T> + ) -> ItemRange<'a, T> { + match self.cur_cached_item { + Some(cached_item) => cached_item.data_as_item_range(), + None => data, + } + } + + pub fn glyphs(&self) -> ItemRange { + self.cached_item_range_or(self.cur_glyphs) + } + + pub fn gradient_stops(&self) -> ItemRange { + self.cached_item_range_or(self.cur_stops) + } + + fn advance_pending_items(&mut self) -> bool { + self.cur_cached_item = self.pending_items.next(); + self.cur_cached_item.is_some() + } + pub fn next<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; @@ -593,6 +628,10 @@ impl<'a> BuiltDisplayListIter<'a> { pub fn next_raw<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; + if self.advance_pending_items() { + return Some(self.as_ref()); + } + // A "red zone" of DisplayItem::max_size() bytes has been added to the // end of the serialized display list. If this amount, or less, is // remaining then we've reached the end of the display list. @@ -649,6 +688,17 @@ impl<'a> BuiltDisplayListIter<'a> { self.cur_glyphs = skip_slice::(&mut self.data); self.debug_stats.log_slice("text.glyphs", &self.cur_glyphs); } + ReuseItems(key) => { + match self.cache { + Some(cache) => { + self.pending_items = cache.get_items(key).iter(); + self.advance_pending_items(); + } + None => { + unreachable!("Cache marker without cache!"); + } + } + } _ => { /* do nothing */ } } @@ -656,17 +706,8 @@ impl<'a> BuiltDisplayListIter<'a> { } pub fn as_ref<'b>(&'b self) -> DisplayItemRef<'a, 'b> { - let cached_item = match self.cur_item { - di::DisplayItem::ReuseItem(key) => { - let cache = self.cache.expect("Cache marker without cache!"); - cache.get_item(key) - } - _ => None - }; - DisplayItemRef { iter: self, - cached_item } } @@ -869,7 +910,6 @@ impl<'de> Deserialize<'de> for BuiltDisplayList { Debug::PopStackingContext => Real::PopStackingContext, Debug::PopReferenceFrame => Real::PopReferenceFrame, Debug::PopAllShadows => Real::PopAllShadows, - Debug::ReuseItem(_) => unreachable!("Unexpected item"), }; poke_into_vec(&item, &mut data); // the aux data is serialized after the item, hence the temporary @@ -905,14 +945,25 @@ pub struct SaveState { next_clip_chain_id: u64, } +/// DisplayListSection determines the target buffer for the display items. +pub enum DisplayListSection { + /// The main/default buffer: contains item data and item group markers. + Data, + /// Auxiliary buffer: contains the item data for item groups. + ExtraData, + /// Temporary buffer: contains the data for pending item group. Flushed to + /// one of the buffers above, after item grouping finishes. + Chunk, +} + #[derive(Clone)] pub struct DisplayListBuilder { pub data: Vec, pub pipeline_id: PipelineId, extra_data: Vec, - extra_data_chunk_start: usize, - writing_extra_data_chunk: bool, + pending_chunk: Vec, + writing_to_chunk: bool, next_clip_index: usize, next_spatial_index: usize, @@ -945,8 +996,8 @@ impl DisplayListBuilder { pipeline_id, extra_data: Vec::new(), - extra_data_chunk_start: 0, - writing_extra_data_chunk: false, + pending_chunk: Vec::new(), + writing_to_chunk: false, next_clip_index: FIRST_CLIP_NODE_INDEX, next_spatial_index: FIRST_SPATIAL_NODE_INDEX, @@ -1035,17 +1086,47 @@ impl DisplayListBuilder { index } - fn active_buffer(&mut self) -> &mut Vec { - if self.writing_extra_data_chunk { - &mut self.extra_data + /// Print the display items in the list to stdout. + pub fn dump_serialized_display_list(&mut self) { + self.serialized_content_buffer = Some(String::new()); + } + + fn add_to_display_list_dump(&mut self, item: T) { + if let Some(ref mut content) = self.serialized_content_buffer { + use std::fmt::Write; + write!(content, "{:?}\n", item).expect("DL dump write failed."); + } + } + + /// Returns the default section that DisplayListBuilder will write to, + /// if no section is specified explicitly. + fn default_section(&self) -> DisplayListSection { + if self.writing_to_chunk { + DisplayListSection::Chunk } else { - &mut self.data + DisplayListSection::Data } } - /// Print the display items in the list to stdout. - pub fn dump_serialized_display_list(&mut self) { - self.serialized_content_buffer = Some(String::new()); + fn buffer_from_section( + &mut self, + section: DisplayListSection + ) -> &mut Vec { + match section { + DisplayListSection::Data => &mut self.data, + DisplayListSection::ExtraData => &mut self.extra_data, + DisplayListSection::Chunk => &mut self.pending_chunk, + } + } + + #[inline] + pub fn push_item_to_section( + &mut self, + item: &di::DisplayItem, + section: DisplayListSection, + ) { + poke_into_vec(item, self.buffer_from_section(section)); + self.add_to_display_list_dump(item); } /// Add an item to the display list. @@ -1055,12 +1136,7 @@ impl DisplayListBuilder { /// result in WebRender panicking or behaving in unexpected ways. #[inline] pub fn push_item(&mut self, item: &di::DisplayItem) { - poke_into_vec(item, self.active_buffer()); - - if let Some(ref mut content) = self.serialized_content_buffer { - use std::fmt::Write; - write!(content, "{:?}\n", item).expect("DL dump write failed."); - } + self.push_item_to_section(item, self.default_section()); } fn push_iter_impl(data: &mut Vec, iter_source: I) @@ -1106,7 +1182,8 @@ impl DisplayListBuilder { I::IntoIter: ExactSizeIterator, I::Item: Poke, { - Self::push_iter_impl(self.active_buffer(), iter); + let mut buffer = self.buffer_from_section(self.default_section()); + Self::push_iter_impl(&mut buffer, iter); } pub fn push_rect( @@ -1744,35 +1821,57 @@ impl DisplayListBuilder { self.push_item(&di::DisplayItem::PopAllShadows); } - fn truncate_extra_data_chunk(&mut self) { - self.extra_data.truncate(self.extra_data_chunk_start) + pub fn start_item_group(&mut self) { + debug_assert!(!self.writing_to_chunk); + debug_assert!(self.pending_chunk.is_empty()); + + self.writing_to_chunk = true; } - pub fn start_item_group(&mut self, _key: di::ItemKey) { - self.writing_extra_data_chunk = true; - self.extra_data_chunk_start = self.extra_data.len(); + fn flush_pending_item_group(&mut self, key: di::ItemKey) { + // Push RetainedItems-marker to extra_data section. + self.push_retained_items(key); + + // Push pending chunk to extra_data section. + self.extra_data.append(&mut self.pending_chunk); + + // Push ReuseItems-marker to data section. + self.push_reuse_items(key); } pub fn finish_item_group(&mut self, key: di::ItemKey) -> bool { - self.writing_extra_data_chunk = false; + debug_assert!(self.writing_to_chunk); + self.writing_to_chunk = false; - let chunk_size = self.extra_data.len() - self.extra_data_chunk_start; - if chunk_size > 0 { - self.push_reuse_items(key); - return true + if self.pending_chunk.len() > 0 { + self.flush_pending_item_group(key); + true + } else { + debug_assert!(self.pending_chunk.is_empty()); + false } - - false } pub fn cancel_item_group(&mut self) { - debug_assert!(self.writing_extra_data_chunk); - self.writing_extra_data_chunk = false; - self.truncate_extra_data_chunk(); + debug_assert!(self.writing_to_chunk); + self.writing_to_chunk = false; + + // Push pending chunk to data section. + self.data.append(&mut self.pending_chunk); } pub fn push_reuse_items(&mut self, key: di::ItemKey) { - self.push_item(&di::DisplayItem::ReuseItem(key)); + self.push_item_to_section( + &di::DisplayItem::ReuseItems(key), + DisplayListSection::Data + ); + } + + fn push_retained_items(&mut self, key: di::ItemKey) { + self.push_item_to_section( + &di::DisplayItem::RetainedItems(key), + DisplayListSection::ExtraData + ); } pub fn set_cache_size(&mut self, cache_size: usize) { From bf2cf543bd585724b03dceaa0173fdd5a7ed3c6c Mon Sep 17 00:00:00 2001 From: Razvan Maries Date: Thu, 19 Mar 2020 04:02:26 +0000 Subject: [PATCH 5/7] Backed out 3 changesets (bug 1614655) for WebRender bustages. CLOSED TREE Backed out changeset e79e84e8819c (bug 1614655) Backed out changeset cc263e909c61 (bug 1614655) Backed out changeset 10897d6106a8 (bug 1614655) [ghsync] From https://hg.mozilla.org/mozilla-central/rev/4eeedb4337c8b945764997ea8d959ee3ff8c4de9 --- webrender/src/scene_building.rs | 5 +- webrender_api/src/display_item.rs | 11 +- webrender_api/src/display_item_cache.rs | 97 +++++++---- webrender_api/src/display_list.rs | 215 +++++++----------------- 4 files changed, 127 insertions(+), 201 deletions(-) diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index b39dd8e86a..257c8c02d8 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -1510,9 +1510,8 @@ impl<'a> SceneBuilder<'a> { unreachable!("Should have returned in parent method.") } - DisplayItem::ReuseItems(key) | - DisplayItem::RetainedItems(key) => { - unreachable!("Iterator logic error: {:?}", key); + DisplayItem::ReuseItem(..) => { + unreachable!("Iterator logic error") } DisplayItem::PushShadow(info) => { diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs index 7863604ea4..c87249df6d 100644 --- a/webrender_api/src/display_item.rs +++ b/webrender_api/src/display_item.rs @@ -78,6 +78,8 @@ pub struct CommonItemProperties { pub hit_info: Option, /// Various flags describing properties of this primitive. pub flags: PrimitiveFlags, + /// The unique id of this display item. + pub item_key: Option } impl CommonItemProperties { @@ -92,6 +94,7 @@ impl CommonItemProperties { clip_id: space_and_clip.clip_id, hit_info: None, flags: PrimitiveFlags::default(), + item_key: None, } } } @@ -162,8 +165,7 @@ pub enum DisplayItem { PopStackingContext, PopAllShadows, - ReuseItems(ItemKey), - RetainedItems(ItemKey), + ReuseItem(ItemKey), } /// This is a "complete" version of the DisplayItem, with all implicit trailing @@ -205,6 +207,8 @@ pub enum DebugDisplayItem { PopReferenceFrame, PopStackingContext, PopAllShadows, + + ReuseItem(ItemKey), } #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)] @@ -1501,8 +1505,7 @@ impl DisplayItem { DisplayItem::Rectangle(..) => "rectangle", DisplayItem::ScrollFrame(..) => "scroll_frame", DisplayItem::SetGradientStops => "set_gradient_stops", - DisplayItem::ReuseItems(..) => "reuse_item", - DisplayItem::RetainedItems(..) => "retained_items", + DisplayItem::ReuseItem(..) => "reuse_item", DisplayItem::StickyFrame(..) => "sticky_frame", DisplayItem::Text(..) => "text", DisplayItem::YuvImage(..) => "yuv_image", diff --git a/webrender_api/src/display_item_cache.rs b/webrender_api/src/display_item_cache.rs index 3f85f0c68f..9118bd7c4a 100644 --- a/webrender_api/src/display_item_cache.rs +++ b/webrender_api/src/display_item_cache.rs @@ -4,6 +4,8 @@ use crate::display_item::*; use crate::display_list::*; +#[cfg(debug_assertions)] +use std::collections::HashSet; #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct CachedDisplayItem { @@ -12,7 +14,7 @@ pub struct CachedDisplayItem { } impl CachedDisplayItem { - pub fn display_item(&self) -> &DisplayItem { + pub fn item(&self) -> &DisplayItem { &self.item } @@ -38,71 +40,92 @@ impl From> for CachedDisplayItem { } } -#[derive(Clone, Deserialize, Serialize)] -struct CacheEntry { - items: Vec, - occupied: bool, +fn key_from_item(item: &DisplayItem) -> ItemKey { + let key = match item { + DisplayItem::Rectangle(ref info) => info.common.item_key, + DisplayItem::ClearRectangle(ref info) => info.common.item_key, + DisplayItem::HitTest(ref info) => info.common.item_key, + DisplayItem::Text(ref info) => info.common.item_key, + DisplayItem::Image(ref info) => info.common.item_key, + _ => unimplemented!("Unexpected item: {:?}", item) + }; + + key.expect("Cached item without a key") } #[derive(Clone, Deserialize, Serialize)] pub struct DisplayItemCache { - entries: Vec, + items: Vec>, + + #[cfg(debug_assertions)] + keys: HashSet, } impl DisplayItemCache { - fn add_item(&mut self, key: ItemKey, item: CachedDisplayItem) { - let mut entry = &mut self.entries[key as usize]; - entry.items.push(item); - entry.occupied = true; - } - - fn clear_entry(&mut self, key: ItemKey) { - let mut entry = &mut self.entries[key as usize]; - entry.items.clear(); - entry.occupied = false; + fn grow_if_needed( + &mut self, + capacity: usize + ) { + if capacity > self.items.len() { + self.items.resize_with(capacity, || None::); + // println!("Current cache size: {:?} elements, {:?} bytes", + // capacity, std::mem::size_of::() * capacity); + } } - fn grow_if_needed(&mut self, capacity: usize) { - if capacity > self.entries.len() { - self.entries.resize_with(capacity, || CacheEntry { - items: Vec::new(), - occupied: false, - }); - } + fn add_item( + &mut self, + item: CachedDisplayItem, + key: ItemKey, + ) { + self.items[key as usize] = Some(item); } - pub fn get_items(&self, key: ItemKey) -> &[CachedDisplayItem] { - let entry = &self.entries[key as usize]; - debug_assert!(entry.occupied); - entry.items.as_slice() + pub fn get_item( + &self, + key: ItemKey + ) -> Option<&CachedDisplayItem> { + self.items[key as usize].as_ref() } pub fn new() -> Self { Self { - entries: Vec::new(), + items: Vec::new(), + + #[cfg(debug_assertions)] + /// Used to check that there is only one item per key. + keys: HashSet::new(), } } - pub fn update(&mut self, display_list: &BuiltDisplayList) { + pub fn update( + &mut self, + display_list: &BuiltDisplayList + ) { self.grow_if_needed(display_list.cache_size()); let mut iter = display_list.extra_data_iter(); - let mut current_key: Option = None; + + #[cfg(debug_assertions)] + { + self.keys.clear(); + } + loop { let item = match iter.next() { Some(item) => item, None => break, }; - if let DisplayItem::RetainedItems(key) = item.item() { - current_key = Some(*key); - self.clear_entry(*key); - continue; + let item_key = key_from_item(item.item()); + let cached_item = CachedDisplayItem::from(item); + + #[cfg(debug_assertions)] + { + debug_assert!(self.keys.insert(item_key)); } - let key = current_key.expect("Missing RetainedItems marker"); - let cached_item = CachedDisplayItem::from(item); - self.add_item(key, cached_item); + self.add_item(cached_item, item_key); } } } diff --git a/webrender_api/src/display_list.rs b/webrender_api/src/display_list.rs index eddd9b3e9a..9cf4dd8933 100644 --- a/webrender_api/src/display_list.rs +++ b/webrender_api/src/display_list.rs @@ -204,8 +204,6 @@ pub struct BuiltDisplayListIter<'a> { list: &'a BuiltDisplayList, data: &'a [u8], cache: Option<&'a DisplayItemCache>, - pending_items: std::slice::Iter<'a, CachedDisplayItem>, - cur_cached_item: Option<&'a CachedDisplayItem>, cur_item: di::DisplayItem, cur_stops: ItemRange<'a, di::GradientStop>, cur_glyphs: ItemRange<'a, GlyphInstance>, @@ -286,21 +284,29 @@ pub struct ItemStats { pub struct DisplayItemRef<'a: 'b, 'b> { iter: &'b BuiltDisplayListIter<'a>, + cached_item: Option<&'a CachedDisplayItem>, } // Some of these might just become ItemRanges impl<'a, 'b> DisplayItemRef<'a, 'b> { + fn cached_or_iter_data( + &self, + data: ItemRange<'a, T> + ) -> ItemRange<'a, T> { + self.cached_item.map_or(data, |i| i.data_as_item_range()) + } + pub fn display_list(&self) -> &BuiltDisplayList { self.iter.display_list() } // Creates a new iterator where this element's iterator is, to hack around borrowck. pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> { - self.iter.sub_iter() + BuiltDisplayListIter::new(self.iter.list, self.iter.data, self.iter.cache) } pub fn item(&self) -> &di::DisplayItem { - self.iter.current_item() + self.cached_item.map_or(&self.iter.cur_item, |i| i.item()) } pub fn clip_chain_items(&self) -> ItemRange { @@ -312,11 +318,11 @@ impl<'a, 'b> DisplayItemRef<'a, 'b> { } pub fn glyphs(&self) -> ItemRange { - self.iter.glyphs() + self.cached_or_iter_data(self.iter.cur_glyphs) } pub fn gradient_stops(&self) -> ItemRange { - self.iter.gradient_stops() + self.cached_or_iter_data(self.iter.cur_stops) } pub fn filters(&self) -> ItemRange { @@ -487,8 +493,7 @@ impl BuiltDisplayList { Real::PopReferenceFrame => Debug::PopReferenceFrame, Real::PopStackingContext => Debug::PopStackingContext, Real::PopAllShadows => Debug::PopAllShadows, - Real::ReuseItems(_) | - Real::RetainedItems(_) => unreachable!("Unexpected item"), + Real::ReuseItem(_) => unreachable!("Unexpected item"), }; seq.serialize_element(&serial_di)? } @@ -521,8 +526,6 @@ impl<'a> BuiltDisplayListIter<'a> { list, data, cache, - pending_items: [].iter(), - cur_cached_item: None, cur_item: di::DisplayItem::PopStackingContext, cur_stops: ItemRange::default(), cur_glyphs: ItemRange::default(), @@ -535,52 +538,14 @@ impl<'a> BuiltDisplayListIter<'a> { debug_stats: DebugStats { last_addr: data.as_ptr() as usize, stats: HashMap::default(), - }, + } } } - pub fn sub_iter(&self) -> Self { - let mut iter = BuiltDisplayListIter::new( - self.list, self.data, self.cache - ); - iter.pending_items = self.pending_items.clone(); - iter - } - pub fn display_list(&self) -> &'a BuiltDisplayList { self.list } - pub fn current_item(&self) -> &di::DisplayItem { - match self.cur_cached_item { - Some(cached_item) => cached_item.display_item(), - None => &self.cur_item - } - } - - fn cached_item_range_or( - &self, - data: ItemRange<'a, T> - ) -> ItemRange<'a, T> { - match self.cur_cached_item { - Some(cached_item) => cached_item.data_as_item_range(), - None => data, - } - } - - pub fn glyphs(&self) -> ItemRange { - self.cached_item_range_or(self.cur_glyphs) - } - - pub fn gradient_stops(&self) -> ItemRange { - self.cached_item_range_or(self.cur_stops) - } - - fn advance_pending_items(&mut self) -> bool { - self.cur_cached_item = self.pending_items.next(); - self.cur_cached_item.is_some() - } - pub fn next<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; @@ -628,10 +593,6 @@ impl<'a> BuiltDisplayListIter<'a> { pub fn next_raw<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; - if self.advance_pending_items() { - return Some(self.as_ref()); - } - // A "red zone" of DisplayItem::max_size() bytes has been added to the // end of the serialized display list. If this amount, or less, is // remaining then we've reached the end of the display list. @@ -688,17 +649,6 @@ impl<'a> BuiltDisplayListIter<'a> { self.cur_glyphs = skip_slice::(&mut self.data); self.debug_stats.log_slice("text.glyphs", &self.cur_glyphs); } - ReuseItems(key) => { - match self.cache { - Some(cache) => { - self.pending_items = cache.get_items(key).iter(); - self.advance_pending_items(); - } - None => { - unreachable!("Cache marker without cache!"); - } - } - } _ => { /* do nothing */ } } @@ -706,8 +656,17 @@ impl<'a> BuiltDisplayListIter<'a> { } pub fn as_ref<'b>(&'b self) -> DisplayItemRef<'a, 'b> { + let cached_item = match self.cur_item { + di::DisplayItem::ReuseItem(key) => { + let cache = self.cache.expect("Cache marker without cache!"); + cache.get_item(key) + } + _ => None + }; + DisplayItemRef { iter: self, + cached_item } } @@ -910,6 +869,7 @@ impl<'de> Deserialize<'de> for BuiltDisplayList { Debug::PopStackingContext => Real::PopStackingContext, Debug::PopReferenceFrame => Real::PopReferenceFrame, Debug::PopAllShadows => Real::PopAllShadows, + Debug::ReuseItem(_) => unreachable!("Unexpected item"), }; poke_into_vec(&item, &mut data); // the aux data is serialized after the item, hence the temporary @@ -945,25 +905,14 @@ pub struct SaveState { next_clip_chain_id: u64, } -/// DisplayListSection determines the target buffer for the display items. -pub enum DisplayListSection { - /// The main/default buffer: contains item data and item group markers. - Data, - /// Auxiliary buffer: contains the item data for item groups. - ExtraData, - /// Temporary buffer: contains the data for pending item group. Flushed to - /// one of the buffers above, after item grouping finishes. - Chunk, -} - #[derive(Clone)] pub struct DisplayListBuilder { pub data: Vec, pub pipeline_id: PipelineId, extra_data: Vec, - pending_chunk: Vec, - writing_to_chunk: bool, + extra_data_chunk_start: usize, + writing_extra_data_chunk: bool, next_clip_index: usize, next_spatial_index: usize, @@ -996,8 +945,8 @@ impl DisplayListBuilder { pipeline_id, extra_data: Vec::new(), - pending_chunk: Vec::new(), - writing_to_chunk: false, + extra_data_chunk_start: 0, + writing_extra_data_chunk: false, next_clip_index: FIRST_CLIP_NODE_INDEX, next_spatial_index: FIRST_SPATIAL_NODE_INDEX, @@ -1086,47 +1035,17 @@ impl DisplayListBuilder { index } - /// Print the display items in the list to stdout. - pub fn dump_serialized_display_list(&mut self) { - self.serialized_content_buffer = Some(String::new()); - } - - fn add_to_display_list_dump(&mut self, item: T) { - if let Some(ref mut content) = self.serialized_content_buffer { - use std::fmt::Write; - write!(content, "{:?}\n", item).expect("DL dump write failed."); - } - } - - /// Returns the default section that DisplayListBuilder will write to, - /// if no section is specified explicitly. - fn default_section(&self) -> DisplayListSection { - if self.writing_to_chunk { - DisplayListSection::Chunk + fn active_buffer(&mut self) -> &mut Vec { + if self.writing_extra_data_chunk { + &mut self.extra_data } else { - DisplayListSection::Data + &mut self.data } } - fn buffer_from_section( - &mut self, - section: DisplayListSection - ) -> &mut Vec { - match section { - DisplayListSection::Data => &mut self.data, - DisplayListSection::ExtraData => &mut self.extra_data, - DisplayListSection::Chunk => &mut self.pending_chunk, - } - } - - #[inline] - pub fn push_item_to_section( - &mut self, - item: &di::DisplayItem, - section: DisplayListSection, - ) { - poke_into_vec(item, self.buffer_from_section(section)); - self.add_to_display_list_dump(item); + /// Print the display items in the list to stdout. + pub fn dump_serialized_display_list(&mut self) { + self.serialized_content_buffer = Some(String::new()); } /// Add an item to the display list. @@ -1136,7 +1055,12 @@ impl DisplayListBuilder { /// result in WebRender panicking or behaving in unexpected ways. #[inline] pub fn push_item(&mut self, item: &di::DisplayItem) { - self.push_item_to_section(item, self.default_section()); + poke_into_vec(item, self.active_buffer()); + + if let Some(ref mut content) = self.serialized_content_buffer { + use std::fmt::Write; + write!(content, "{:?}\n", item).expect("DL dump write failed."); + } } fn push_iter_impl(data: &mut Vec, iter_source: I) @@ -1182,8 +1106,7 @@ impl DisplayListBuilder { I::IntoIter: ExactSizeIterator, I::Item: Poke, { - let mut buffer = self.buffer_from_section(self.default_section()); - Self::push_iter_impl(&mut buffer, iter); + Self::push_iter_impl(self.active_buffer(), iter); } pub fn push_rect( @@ -1821,57 +1744,35 @@ impl DisplayListBuilder { self.push_item(&di::DisplayItem::PopAllShadows); } - pub fn start_item_group(&mut self) { - debug_assert!(!self.writing_to_chunk); - debug_assert!(self.pending_chunk.is_empty()); - - self.writing_to_chunk = true; + fn truncate_extra_data_chunk(&mut self) { + self.extra_data.truncate(self.extra_data_chunk_start) } - fn flush_pending_item_group(&mut self, key: di::ItemKey) { - // Push RetainedItems-marker to extra_data section. - self.push_retained_items(key); - - // Push pending chunk to extra_data section. - self.extra_data.append(&mut self.pending_chunk); - - // Push ReuseItems-marker to data section. - self.push_reuse_items(key); + pub fn start_item_group(&mut self, _key: di::ItemKey) { + self.writing_extra_data_chunk = true; + self.extra_data_chunk_start = self.extra_data.len(); } pub fn finish_item_group(&mut self, key: di::ItemKey) -> bool { - debug_assert!(self.writing_to_chunk); - self.writing_to_chunk = false; + self.writing_extra_data_chunk = false; - if self.pending_chunk.len() > 0 { - self.flush_pending_item_group(key); - true - } else { - debug_assert!(self.pending_chunk.is_empty()); - false + let chunk_size = self.extra_data.len() - self.extra_data_chunk_start; + if chunk_size > 0 { + self.push_reuse_items(key); + return true } + + false } pub fn cancel_item_group(&mut self) { - debug_assert!(self.writing_to_chunk); - self.writing_to_chunk = false; - - // Push pending chunk to data section. - self.data.append(&mut self.pending_chunk); + debug_assert!(self.writing_extra_data_chunk); + self.writing_extra_data_chunk = false; + self.truncate_extra_data_chunk(); } pub fn push_reuse_items(&mut self, key: di::ItemKey) { - self.push_item_to_section( - &di::DisplayItem::ReuseItems(key), - DisplayListSection::Data - ); - } - - fn push_retained_items(&mut self, key: di::ItemKey) { - self.push_item_to_section( - &di::DisplayItem::RetainedItems(key), - DisplayListSection::ExtraData - ); + self.push_item(&di::DisplayItem::ReuseItem(key)); } pub fn set_cache_size(&mut self, cache_size: usize) { From 46d63663c5cf661be9e4896da34987140a1dd13f Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Thu, 19 Mar 2020 04:02:35 +0000 Subject: [PATCH 6/7] Bug 1614655 - Part 1: Remove item_key from CommonItemProperties r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D66442 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/0e791acac783e4ef9728fd2feded2d405010a76d --- webrender_api/src/display_item.rs | 3 --- wrench/src/rawtest.rs | 10 ---------- wrench/src/yaml_frame_reader.rs | 2 -- 3 files changed, 15 deletions(-) diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs index c87249df6d..abc4dd22d2 100644 --- a/webrender_api/src/display_item.rs +++ b/webrender_api/src/display_item.rs @@ -78,8 +78,6 @@ pub struct CommonItemProperties { pub hit_info: Option, /// Various flags describing properties of this primitive. pub flags: PrimitiveFlags, - /// The unique id of this display item. - pub item_key: Option } impl CommonItemProperties { @@ -94,7 +92,6 @@ impl CommonItemProperties { clip_id: space_and_clip.clip_id, hit_info: None, flags: PrimitiveFlags::default(), - item_key: None, } } } diff --git a/wrench/src/rawtest.rs b/wrench/src/rawtest.rs index 030ccac79a..bb270e31e4 100644 --- a/wrench/src/rawtest.rs +++ b/wrench/src/rawtest.rs @@ -121,7 +121,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, } } @@ -137,7 +136,6 @@ impl<'a> RawtestHarness<'a> { spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, } } @@ -327,7 +325,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: root_space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; // setup some malicious image size parameters @@ -416,7 +413,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: root_space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; builder.push_repeating_image( @@ -514,7 +510,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: root_space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; builder.push_repeating_image( @@ -562,7 +557,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: root_space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; builder.push_repeating_image( @@ -612,7 +606,6 @@ impl<'a> RawtestHarness<'a> { spatial_id: root_space_and_clip.spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; builder.push_repeating_image( @@ -1131,7 +1124,6 @@ impl<'a> RawtestHarness<'a> { spatial_id, flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; builder.push_line( &info, @@ -1382,7 +1374,6 @@ impl<'a> RawtestHarness<'a> { builder.push_rect( &CommonItemProperties { hit_info: Some((0, 4)), - item_key: None, clip_rect: rect, clip_id: temp_clip_id, spatial_id: space_and_clip.spatial_id, @@ -1403,7 +1394,6 @@ impl<'a> RawtestHarness<'a> { builder.push_rect( &CommonItemProperties { hit_info: Some((0, 5)), - item_key: None, clip_rect: rect, clip_id: ClipId::ClipChain(clip_chain_id), spatial_id: space_and_clip.spatial_id, diff --git a/wrench/src/yaml_frame_reader.rs b/wrench/src/yaml_frame_reader.rs index 35fb664dcc..22ff0b801f 100644 --- a/wrench/src/yaml_frame_reader.rs +++ b/wrench/src/yaml_frame_reader.rs @@ -475,7 +475,6 @@ impl YamlFrameReader { spatial_id: SpatialId::new(0, PipelineId::dummy()), flags: PrimitiveFlags::default(), hit_info: None, - item_key: None, }; self.add_stacking_context_from_yaml(&mut builder, wrench, yaml, true, &mut info); self.display_lists.push(builder.finalize()); @@ -1745,7 +1744,6 @@ impl YamlFrameReader { clip_id: space_and_clip.clip_id, spatial_id: space_and_clip.spatial_id, hit_info: self.to_hit_testing_tag(&item["hit-testing-tag"]), - item_key: None, flags, }; From 81d03c6ba24b30a61459b051f60204cdecffdd4e Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Thu, 19 Mar 2020 04:02:43 +0000 Subject: [PATCH 7/7] Bug 1614655 - Part 2: Allow 1:n mapping of Gecko - WR display items r=jrmuizel This patch changes the underlying storage for WR display items in DisplayItemCache from Vec to Vec>. This allows storing multiple WebRender display items for one Gecko display item. The storage is populated by traversing BuiltDisplayList extra data portion in display list format, which is roughly as follows: RetainedItems(key k1) Item1(..) RetainedItems(key k2) ItemN(..) ItemN+1(..) This would store Item1 under key k1, and ItemN and ItemN+1 under the key k2, where k1 and k2 are arbitrary unique identifiers (currently of type uint16_t). The entries in the storage are accessed by DisplayItemCache::get_iterator(key), which is called by BuiltDisplayListIter, whenever it encounters a display item DisplayItem::ReuseItems(key). Differential Revision: https://phabricator.services.mozilla.com/D66443 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/aaed31bbe8f4e0a8892e632633c27033549eaf33 --- webrender/src/scene_building.rs | 5 +- webrender_api/src/display_item.rs | 8 +- webrender_api/src/display_item_cache.rs | 97 ++++------- webrender_api/src/display_list.rs | 215 +++++++++++++++++------- wrench/src/yaml_frame_writer.rs | 8 +- 5 files changed, 207 insertions(+), 126 deletions(-) diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index 257c8c02d8..b39dd8e86a 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -1510,8 +1510,9 @@ impl<'a> SceneBuilder<'a> { unreachable!("Should have returned in parent method.") } - DisplayItem::ReuseItem(..) => { - unreachable!("Iterator logic error") + DisplayItem::ReuseItems(key) | + DisplayItem::RetainedItems(key) => { + unreachable!("Iterator logic error: {:?}", key); } DisplayItem::PushShadow(info) => { diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs index abc4dd22d2..7863604ea4 100644 --- a/webrender_api/src/display_item.rs +++ b/webrender_api/src/display_item.rs @@ -162,7 +162,8 @@ pub enum DisplayItem { PopStackingContext, PopAllShadows, - ReuseItem(ItemKey), + ReuseItems(ItemKey), + RetainedItems(ItemKey), } /// This is a "complete" version of the DisplayItem, with all implicit trailing @@ -204,8 +205,6 @@ pub enum DebugDisplayItem { PopReferenceFrame, PopStackingContext, PopAllShadows, - - ReuseItem(ItemKey), } #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)] @@ -1502,7 +1501,8 @@ impl DisplayItem { DisplayItem::Rectangle(..) => "rectangle", DisplayItem::ScrollFrame(..) => "scroll_frame", DisplayItem::SetGradientStops => "set_gradient_stops", - DisplayItem::ReuseItem(..) => "reuse_item", + DisplayItem::ReuseItems(..) => "reuse_item", + DisplayItem::RetainedItems(..) => "retained_items", DisplayItem::StickyFrame(..) => "sticky_frame", DisplayItem::Text(..) => "text", DisplayItem::YuvImage(..) => "yuv_image", diff --git a/webrender_api/src/display_item_cache.rs b/webrender_api/src/display_item_cache.rs index 9118bd7c4a..3f85f0c68f 100644 --- a/webrender_api/src/display_item_cache.rs +++ b/webrender_api/src/display_item_cache.rs @@ -4,8 +4,6 @@ use crate::display_item::*; use crate::display_list::*; -#[cfg(debug_assertions)] -use std::collections::HashSet; #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct CachedDisplayItem { @@ -14,7 +12,7 @@ pub struct CachedDisplayItem { } impl CachedDisplayItem { - pub fn item(&self) -> &DisplayItem { + pub fn display_item(&self) -> &DisplayItem { &self.item } @@ -40,92 +38,71 @@ impl From> for CachedDisplayItem { } } -fn key_from_item(item: &DisplayItem) -> ItemKey { - let key = match item { - DisplayItem::Rectangle(ref info) => info.common.item_key, - DisplayItem::ClearRectangle(ref info) => info.common.item_key, - DisplayItem::HitTest(ref info) => info.common.item_key, - DisplayItem::Text(ref info) => info.common.item_key, - DisplayItem::Image(ref info) => info.common.item_key, - _ => unimplemented!("Unexpected item: {:?}", item) - }; - - key.expect("Cached item without a key") +#[derive(Clone, Deserialize, Serialize)] +struct CacheEntry { + items: Vec, + occupied: bool, } #[derive(Clone, Deserialize, Serialize)] pub struct DisplayItemCache { - items: Vec>, - - #[cfg(debug_assertions)] - keys: HashSet, + entries: Vec, } impl DisplayItemCache { - fn grow_if_needed( - &mut self, - capacity: usize - ) { - if capacity > self.items.len() { - self.items.resize_with(capacity, || None::); - // println!("Current cache size: {:?} elements, {:?} bytes", - // capacity, std::mem::size_of::() * capacity); - } + fn add_item(&mut self, key: ItemKey, item: CachedDisplayItem) { + let mut entry = &mut self.entries[key as usize]; + entry.items.push(item); + entry.occupied = true; } - fn add_item( - &mut self, - item: CachedDisplayItem, - key: ItemKey, - ) { - self.items[key as usize] = Some(item); + fn clear_entry(&mut self, key: ItemKey) { + let mut entry = &mut self.entries[key as usize]; + entry.items.clear(); + entry.occupied = false; } - pub fn get_item( - &self, - key: ItemKey - ) -> Option<&CachedDisplayItem> { - self.items[key as usize].as_ref() + fn grow_if_needed(&mut self, capacity: usize) { + if capacity > self.entries.len() { + self.entries.resize_with(capacity, || CacheEntry { + items: Vec::new(), + occupied: false, + }); + } + } + + pub fn get_items(&self, key: ItemKey) -> &[CachedDisplayItem] { + let entry = &self.entries[key as usize]; + debug_assert!(entry.occupied); + entry.items.as_slice() } pub fn new() -> Self { Self { - items: Vec::new(), - - #[cfg(debug_assertions)] - /// Used to check that there is only one item per key. - keys: HashSet::new(), + entries: Vec::new(), } } - pub fn update( - &mut self, - display_list: &BuiltDisplayList - ) { + pub fn update(&mut self, display_list: &BuiltDisplayList) { self.grow_if_needed(display_list.cache_size()); let mut iter = display_list.extra_data_iter(); - - #[cfg(debug_assertions)] - { - self.keys.clear(); - } - + let mut current_key: Option = None; loop { let item = match iter.next() { Some(item) => item, None => break, }; - let item_key = key_from_item(item.item()); - let cached_item = CachedDisplayItem::from(item); - - #[cfg(debug_assertions)] - { - debug_assert!(self.keys.insert(item_key)); + if let DisplayItem::RetainedItems(key) = item.item() { + current_key = Some(*key); + self.clear_entry(*key); + continue; } - self.add_item(cached_item, item_key); + let key = current_key.expect("Missing RetainedItems marker"); + let cached_item = CachedDisplayItem::from(item); + self.add_item(key, cached_item); } } } diff --git a/webrender_api/src/display_list.rs b/webrender_api/src/display_list.rs index 9cf4dd8933..eddd9b3e9a 100644 --- a/webrender_api/src/display_list.rs +++ b/webrender_api/src/display_list.rs @@ -204,6 +204,8 @@ pub struct BuiltDisplayListIter<'a> { list: &'a BuiltDisplayList, data: &'a [u8], cache: Option<&'a DisplayItemCache>, + pending_items: std::slice::Iter<'a, CachedDisplayItem>, + cur_cached_item: Option<&'a CachedDisplayItem>, cur_item: di::DisplayItem, cur_stops: ItemRange<'a, di::GradientStop>, cur_glyphs: ItemRange<'a, GlyphInstance>, @@ -284,29 +286,21 @@ pub struct ItemStats { pub struct DisplayItemRef<'a: 'b, 'b> { iter: &'b BuiltDisplayListIter<'a>, - cached_item: Option<&'a CachedDisplayItem>, } // Some of these might just become ItemRanges impl<'a, 'b> DisplayItemRef<'a, 'b> { - fn cached_or_iter_data( - &self, - data: ItemRange<'a, T> - ) -> ItemRange<'a, T> { - self.cached_item.map_or(data, |i| i.data_as_item_range()) - } - pub fn display_list(&self) -> &BuiltDisplayList { self.iter.display_list() } // Creates a new iterator where this element's iterator is, to hack around borrowck. pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> { - BuiltDisplayListIter::new(self.iter.list, self.iter.data, self.iter.cache) + self.iter.sub_iter() } pub fn item(&self) -> &di::DisplayItem { - self.cached_item.map_or(&self.iter.cur_item, |i| i.item()) + self.iter.current_item() } pub fn clip_chain_items(&self) -> ItemRange { @@ -318,11 +312,11 @@ impl<'a, 'b> DisplayItemRef<'a, 'b> { } pub fn glyphs(&self) -> ItemRange { - self.cached_or_iter_data(self.iter.cur_glyphs) + self.iter.glyphs() } pub fn gradient_stops(&self) -> ItemRange { - self.cached_or_iter_data(self.iter.cur_stops) + self.iter.gradient_stops() } pub fn filters(&self) -> ItemRange { @@ -493,7 +487,8 @@ impl BuiltDisplayList { Real::PopReferenceFrame => Debug::PopReferenceFrame, Real::PopStackingContext => Debug::PopStackingContext, Real::PopAllShadows => Debug::PopAllShadows, - Real::ReuseItem(_) => unreachable!("Unexpected item"), + Real::ReuseItems(_) | + Real::RetainedItems(_) => unreachable!("Unexpected item"), }; seq.serialize_element(&serial_di)? } @@ -526,6 +521,8 @@ impl<'a> BuiltDisplayListIter<'a> { list, data, cache, + pending_items: [].iter(), + cur_cached_item: None, cur_item: di::DisplayItem::PopStackingContext, cur_stops: ItemRange::default(), cur_glyphs: ItemRange::default(), @@ -538,14 +535,52 @@ impl<'a> BuiltDisplayListIter<'a> { debug_stats: DebugStats { last_addr: data.as_ptr() as usize, stats: HashMap::default(), - } + }, } } + pub fn sub_iter(&self) -> Self { + let mut iter = BuiltDisplayListIter::new( + self.list, self.data, self.cache + ); + iter.pending_items = self.pending_items.clone(); + iter + } + pub fn display_list(&self) -> &'a BuiltDisplayList { self.list } + pub fn current_item(&self) -> &di::DisplayItem { + match self.cur_cached_item { + Some(cached_item) => cached_item.display_item(), + None => &self.cur_item + } + } + + fn cached_item_range_or( + &self, + data: ItemRange<'a, T> + ) -> ItemRange<'a, T> { + match self.cur_cached_item { + Some(cached_item) => cached_item.data_as_item_range(), + None => data, + } + } + + pub fn glyphs(&self) -> ItemRange { + self.cached_item_range_or(self.cur_glyphs) + } + + pub fn gradient_stops(&self) -> ItemRange { + self.cached_item_range_or(self.cur_stops) + } + + fn advance_pending_items(&mut self) -> bool { + self.cur_cached_item = self.pending_items.next(); + self.cur_cached_item.is_some() + } + pub fn next<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; @@ -593,6 +628,10 @@ impl<'a> BuiltDisplayListIter<'a> { pub fn next_raw<'b>(&'b mut self) -> Option> { use crate::DisplayItem::*; + if self.advance_pending_items() { + return Some(self.as_ref()); + } + // A "red zone" of DisplayItem::max_size() bytes has been added to the // end of the serialized display list. If this amount, or less, is // remaining then we've reached the end of the display list. @@ -649,6 +688,17 @@ impl<'a> BuiltDisplayListIter<'a> { self.cur_glyphs = skip_slice::(&mut self.data); self.debug_stats.log_slice("text.glyphs", &self.cur_glyphs); } + ReuseItems(key) => { + match self.cache { + Some(cache) => { + self.pending_items = cache.get_items(key).iter(); + self.advance_pending_items(); + } + None => { + unreachable!("Cache marker without cache!"); + } + } + } _ => { /* do nothing */ } } @@ -656,17 +706,8 @@ impl<'a> BuiltDisplayListIter<'a> { } pub fn as_ref<'b>(&'b self) -> DisplayItemRef<'a, 'b> { - let cached_item = match self.cur_item { - di::DisplayItem::ReuseItem(key) => { - let cache = self.cache.expect("Cache marker without cache!"); - cache.get_item(key) - } - _ => None - }; - DisplayItemRef { iter: self, - cached_item } } @@ -869,7 +910,6 @@ impl<'de> Deserialize<'de> for BuiltDisplayList { Debug::PopStackingContext => Real::PopStackingContext, Debug::PopReferenceFrame => Real::PopReferenceFrame, Debug::PopAllShadows => Real::PopAllShadows, - Debug::ReuseItem(_) => unreachable!("Unexpected item"), }; poke_into_vec(&item, &mut data); // the aux data is serialized after the item, hence the temporary @@ -905,14 +945,25 @@ pub struct SaveState { next_clip_chain_id: u64, } +/// DisplayListSection determines the target buffer for the display items. +pub enum DisplayListSection { + /// The main/default buffer: contains item data and item group markers. + Data, + /// Auxiliary buffer: contains the item data for item groups. + ExtraData, + /// Temporary buffer: contains the data for pending item group. Flushed to + /// one of the buffers above, after item grouping finishes. + Chunk, +} + #[derive(Clone)] pub struct DisplayListBuilder { pub data: Vec, pub pipeline_id: PipelineId, extra_data: Vec, - extra_data_chunk_start: usize, - writing_extra_data_chunk: bool, + pending_chunk: Vec, + writing_to_chunk: bool, next_clip_index: usize, next_spatial_index: usize, @@ -945,8 +996,8 @@ impl DisplayListBuilder { pipeline_id, extra_data: Vec::new(), - extra_data_chunk_start: 0, - writing_extra_data_chunk: false, + pending_chunk: Vec::new(), + writing_to_chunk: false, next_clip_index: FIRST_CLIP_NODE_INDEX, next_spatial_index: FIRST_SPATIAL_NODE_INDEX, @@ -1035,17 +1086,47 @@ impl DisplayListBuilder { index } - fn active_buffer(&mut self) -> &mut Vec { - if self.writing_extra_data_chunk { - &mut self.extra_data + /// Print the display items in the list to stdout. + pub fn dump_serialized_display_list(&mut self) { + self.serialized_content_buffer = Some(String::new()); + } + + fn add_to_display_list_dump(&mut self, item: T) { + if let Some(ref mut content) = self.serialized_content_buffer { + use std::fmt::Write; + write!(content, "{:?}\n", item).expect("DL dump write failed."); + } + } + + /// Returns the default section that DisplayListBuilder will write to, + /// if no section is specified explicitly. + fn default_section(&self) -> DisplayListSection { + if self.writing_to_chunk { + DisplayListSection::Chunk } else { - &mut self.data + DisplayListSection::Data } } - /// Print the display items in the list to stdout. - pub fn dump_serialized_display_list(&mut self) { - self.serialized_content_buffer = Some(String::new()); + fn buffer_from_section( + &mut self, + section: DisplayListSection + ) -> &mut Vec { + match section { + DisplayListSection::Data => &mut self.data, + DisplayListSection::ExtraData => &mut self.extra_data, + DisplayListSection::Chunk => &mut self.pending_chunk, + } + } + + #[inline] + pub fn push_item_to_section( + &mut self, + item: &di::DisplayItem, + section: DisplayListSection, + ) { + poke_into_vec(item, self.buffer_from_section(section)); + self.add_to_display_list_dump(item); } /// Add an item to the display list. @@ -1055,12 +1136,7 @@ impl DisplayListBuilder { /// result in WebRender panicking or behaving in unexpected ways. #[inline] pub fn push_item(&mut self, item: &di::DisplayItem) { - poke_into_vec(item, self.active_buffer()); - - if let Some(ref mut content) = self.serialized_content_buffer { - use std::fmt::Write; - write!(content, "{:?}\n", item).expect("DL dump write failed."); - } + self.push_item_to_section(item, self.default_section()); } fn push_iter_impl(data: &mut Vec, iter_source: I) @@ -1106,7 +1182,8 @@ impl DisplayListBuilder { I::IntoIter: ExactSizeIterator, I::Item: Poke, { - Self::push_iter_impl(self.active_buffer(), iter); + let mut buffer = self.buffer_from_section(self.default_section()); + Self::push_iter_impl(&mut buffer, iter); } pub fn push_rect( @@ -1744,35 +1821,57 @@ impl DisplayListBuilder { self.push_item(&di::DisplayItem::PopAllShadows); } - fn truncate_extra_data_chunk(&mut self) { - self.extra_data.truncate(self.extra_data_chunk_start) + pub fn start_item_group(&mut self) { + debug_assert!(!self.writing_to_chunk); + debug_assert!(self.pending_chunk.is_empty()); + + self.writing_to_chunk = true; } - pub fn start_item_group(&mut self, _key: di::ItemKey) { - self.writing_extra_data_chunk = true; - self.extra_data_chunk_start = self.extra_data.len(); + fn flush_pending_item_group(&mut self, key: di::ItemKey) { + // Push RetainedItems-marker to extra_data section. + self.push_retained_items(key); + + // Push pending chunk to extra_data section. + self.extra_data.append(&mut self.pending_chunk); + + // Push ReuseItems-marker to data section. + self.push_reuse_items(key); } pub fn finish_item_group(&mut self, key: di::ItemKey) -> bool { - self.writing_extra_data_chunk = false; + debug_assert!(self.writing_to_chunk); + self.writing_to_chunk = false; - let chunk_size = self.extra_data.len() - self.extra_data_chunk_start; - if chunk_size > 0 { - self.push_reuse_items(key); - return true + if self.pending_chunk.len() > 0 { + self.flush_pending_item_group(key); + true + } else { + debug_assert!(self.pending_chunk.is_empty()); + false } - - false } pub fn cancel_item_group(&mut self) { - debug_assert!(self.writing_extra_data_chunk); - self.writing_extra_data_chunk = false; - self.truncate_extra_data_chunk(); + debug_assert!(self.writing_to_chunk); + self.writing_to_chunk = false; + + // Push pending chunk to data section. + self.data.append(&mut self.pending_chunk); } pub fn push_reuse_items(&mut self, key: di::ItemKey) { - self.push_item(&di::DisplayItem::ReuseItem(key)); + self.push_item_to_section( + &di::DisplayItem::ReuseItems(key), + DisplayListSection::Data + ); + } + + fn push_retained_items(&mut self, key: di::ItemKey) { + self.push_item_to_section( + &di::DisplayItem::RetainedItems(key), + DisplayListSection::ExtraData + ); } pub fn set_cache_size(&mut self, cache_size: usize) { diff --git a/wrench/src/yaml_frame_writer.rs b/wrench/src/yaml_frame_writer.rs index 6a1da76809..eb28049ebb 100644 --- a/wrench/src/yaml_frame_writer.rs +++ b/wrench/src/yaml_frame_writer.rs @@ -1498,8 +1498,12 @@ impl YamlFrameWriter { DisplayItem::PopAllShadows => { str_node(&mut v, "type", "pop-all-shadows"); } - DisplayItem::ReuseItem(key) => { - str_node(&mut v, "type", "reuse-item"); + DisplayItem::ReuseItems(key) => { + str_node(&mut v, "type", "reuse-items"); + usize_node(&mut v, "key", key as usize); + } + DisplayItem::RetainedItems(key) => { + str_node(&mut v, "type", "retained-items"); usize_node(&mut v, "key", key as usize); } }