Bug 1664719 - Pt 4 - Create a new slice if shared clips are incompatible. r=nical

This patch changes the picture cache logic to create a new slice
if it finds a primitive with an incompatible set of shared clips.

In testing, I have been unable to find any real world content that
hits this case. There is a single test case in CI that hits this
case, which confirms that a new slice is created correctly. Due
to this, it should have minimal effect on real world browsing.

The benefit of this is that we know the set of shared clips won't
change as we continue to build the scene. The follow up patch to
this can then filter out and remove shared clips from primitives
during scene building, rather than frame building.

Differential Revision: https://phabricator.services.mozilla.com/D90349
This commit is contained in:
Glenn Watson 2020-09-23 23:22:01 +00:00
parent a684544a0d
commit 5d7903a3ce
4 changed files with 97 additions and 82 deletions

View File

@ -116,7 +116,7 @@ use smallvec::SmallVec;
// Type definitions for interning clip nodes.
#[derive(Copy, Clone, Debug, MallocSizeOf)]
#[derive(Copy, Clone, Debug, MallocSizeOf, PartialEq)]
#[cfg_attr(any(feature = "serde"), derive(Deserialize, Serialize))]
pub enum ClipIntern {}
@ -125,7 +125,7 @@ pub type ClipDataHandle = intern::Handle<ClipIntern>;
/// Defines a clip that is positioned by a specific spatial node
#[cfg_attr(feature = "capture", derive(Serialize))]
#[derive(Copy, Clone)]
#[derive(Copy, Clone, PartialEq)]
pub struct ClipInstance {
/// Handle to the interned clip
pub handle: ClipDataHandle,

View File

@ -110,7 +110,7 @@ impl ItemUid {
#[cfg_attr(feature = "capture", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
#[derive(Debug, MallocSizeOf)]
#[derive(Debug, MallocSizeOf, PartialEq)]
pub struct Handle<I> {
index: u32,
epoch: Epoch,

View File

@ -110,59 +110,94 @@ impl TileCacheBuilder {
};
let requires_own_slice = is_scrollbar_container || is_clear_prim;
// Also create a new slice if there was a barrier previously set
let mut want_new_tile_cache = self.need_new_tile_cache || requires_own_slice;
// Check if we want to create a new slice based on the current / next scroll root
let scroll_root = self.find_scroll_root(spatial_node_index, spatial_tree);
// Also create a new slice if there was a barrier previously set
let mut want_new_tile_cache =
self.need_new_tile_cache ||
requires_own_slice ||
self.pending_tile_caches.is_empty();
let current_scroll_root = self.pending_tile_caches
.last()
.map(|p| p.params.spatial_node_index)
.unwrap_or(ROOT_SPATIAL_NODE_INDEX);
.map(|p| p.params.spatial_node_index);
want_new_tile_cache |= match (current_scroll_root, scroll_root) {
(ROOT_SPATIAL_NODE_INDEX, ROOT_SPATIAL_NODE_INDEX) => {
// Both current slice and this cluster are fixed position, no need to cut
false
}
(ROOT_SPATIAL_NODE_INDEX, _) => {
// A real scroll root is being established, so create a cache slice
true
}
(_, ROOT_SPATIAL_NODE_INDEX) => {
// If quality settings force subpixel AA over performance, skip creating
// a slice for the fixed position element(s) here.
if quality_settings.force_subpixel_aa_where_possible {
if let Some(current_scroll_root) = current_scroll_root {
want_new_tile_cache |= match (current_scroll_root, scroll_root) {
(ROOT_SPATIAL_NODE_INDEX, ROOT_SPATIAL_NODE_INDEX) => {
// Both current slice and this cluster are fixed position, no need to cut
false
} else {
// A fixed position slice is encountered within a scroll root. Only create
// a slice in this case if all the clips referenced by this cluster are also
// fixed position. There's no real point in creating slices for these cases,
// since we'll have to rasterize them as the scrolling clip moves anyway. It
// also allows us to retain subpixel AA in these cases. For these types of
// slices, the intra-slice dirty rect handling typically works quite well
// (a common case is parallax scrolling effects).
let mut create_slice = true;
let mut current_clip_chain_id = prim_instance.clip_set.clip_chain_id;
while current_clip_chain_id != ClipChainId::NONE {
let clip_chain_node = &clip_store.clip_chain_nodes[current_clip_chain_id.0 as usize];
let spatial_root = self.find_scroll_root(clip_chain_node.spatial_node_index, spatial_tree);
if spatial_root != ROOT_SPATIAL_NODE_INDEX {
create_slice = false;
break;
}
current_clip_chain_id = clip_chain_node.parent_clip_chain_id;
}
create_slice
}
(ROOT_SPATIAL_NODE_INDEX, _) => {
// A real scroll root is being established, so create a cache slice
true
}
(_, ROOT_SPATIAL_NODE_INDEX) => {
// If quality settings force subpixel AA over performance, skip creating
// a slice for the fixed position element(s) here.
if quality_settings.force_subpixel_aa_where_possible {
false
} else {
// A fixed position slice is encountered within a scroll root. Only create
// a slice in this case if all the clips referenced by this cluster are also
// fixed position. There's no real point in creating slices for these cases,
// since we'll have to rasterize them as the scrolling clip moves anyway. It
// also allows us to retain subpixel AA in these cases. For these types of
// slices, the intra-slice dirty rect handling typically works quite well
// (a common case is parallax scrolling effects).
let mut create_slice = true;
let mut current_clip_chain_id = prim_instance.clip_set.clip_chain_id;
while current_clip_chain_id != ClipChainId::NONE {
let clip_chain_node = &clip_store.clip_chain_nodes[current_clip_chain_id.0 as usize];
let spatial_root = self.find_scroll_root(clip_chain_node.spatial_node_index, spatial_tree);
if spatial_root != ROOT_SPATIAL_NODE_INDEX {
create_slice = false;
break;
}
current_clip_chain_id = clip_chain_node.parent_clip_chain_id;
}
create_slice
}
}
(curr_scroll_root, scroll_root) => {
// Two scrolling roots - only need a new slice if they differ
curr_scroll_root != scroll_root
}
};
// Update the list of clips that apply to this primitive instance, to track which are the
// shared clips for this tile cache that can be applied during compositing.
if self.last_checked_clip_chain != prim_instance.clip_set.clip_chain_id {
let prim_clips_buffer = &mut self.prim_clips_buffer;
prim_clips_buffer.clear();
add_clips(
current_scroll_root,
prim_instance.clip_set.clip_chain_id,
prim_clips_buffer,
clip_store,
interners,
spatial_tree,
);
let current_shared_clips = &self.pending_tile_caches
.last()
.unwrap()
.params
.shared_clips;
// If the shared clips are not compatible, create a new slice.
// TODO(gw): Does Gecko ever supply duplicate or out-of-order
// shared clips? It doesn't seem to, but if it does,
// we will need to be more clever here to check if
// the shared clips are compatible.
want_new_tile_cache |= current_shared_clips != prim_clips_buffer;
self.last_checked_clip_chain = prim_instance.clip_set.clip_chain_id;
}
(curr_scroll_root, scroll_root) => {
// Two scrolling roots - only need a new slice if they differ
curr_scroll_root != scroll_root
}
};
}
if want_new_tile_cache {
// If the page would create too many slices (an arbitrary definition where
@ -241,39 +276,16 @@ impl TileCacheBuilder {
}
}
let pending_tile_cache = self.pending_tile_caches.last_mut().unwrap();
// Update the list of clips that apply to this primitive instance, to track which are the
// shared clips for this tile cache that can be applied during compositing.
if self.last_checked_clip_chain != prim_instance.clip_set.clip_chain_id {
let prim_clips_buffer = &mut self.prim_clips_buffer;
prim_clips_buffer.clear();
add_clips(
pending_tile_cache.params.spatial_node_index,
prim_instance.clip_set.clip_chain_id,
prim_clips_buffer,
clip_store,
interners,
spatial_tree,
self.pending_tile_caches
.last_mut()
.unwrap()
.prim_list
.add_prim(
prim_instance,
prim_rect,
spatial_node_index,
prim_flags,
);
pending_tile_cache.params.shared_clips.retain(|h1: &ClipInstance| {
let uid = h1.handle.uid();
prim_clips_buffer.iter().any(|h2| {
uid == h2.handle.uid() &&
h1.spatial_node_index == h2.spatial_node_index
})
});
self.last_checked_clip_chain = prim_instance.clip_set.clip_chain_id;
}
pending_tile_cache.prim_list.add_prim(
prim_instance,
prim_rect,
spatial_node_index,
prim_flags,
);
}
/// Consume this object and build the list of tile cache primitives

View File

@ -10,9 +10,12 @@ root:
- type: rect
bounds: [0, 0, 500, 200]
color: white
- type: rect
- type: clip
bounds: [0, 0, 500, 200]
color: white
items:
- type: rect
bounds: [0, 0, 500, 200]
color: white
- text: "The sun has frightened off the night!"
origin: 20 40
size: 20