Bug 1745775 - Fix invalid surface info indexing r=gfx-reviewers,bradwerth

Some parts of the visibility pass have been ported to use the
picture graph infrastructure (update pass assignment and bounding
rect propagation) but the main visibility pass still relies on
the old-style recursive traversal, for now.

If an off-screen surface with a filter has a child primitive that
has backface-visibility: false, it was possible for it to be
excluded from surface assignment during the picture graph setup,
but still visited by the old-style recursive visibility pass.

In future, the main visibility pass will be ported to be based on
the picture graph infrastructure. In the meantime, this introduces
a band-aid fix by including the backface visibility check for a
picture in the general `is_visible` method, which is already
checked by the visibility and prepare passes, ensuring that the
traversals match.

Differential Revision: https://phabricator.services.mozilla.com/D133704
This commit is contained in:
Glenn Watson 2021-12-14 01:06:24 +00:00
parent d3e3f44322
commit 5ba4876993
5 changed files with 59 additions and 34 deletions

View File

@ -0,0 +1,24 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<style>
* {
backface-visibility: hidden ! important;
filter: blur(63pc) ! important;
padding-block-end: 87%;
transform: matrix3d(-55, 29, 49313, 4, 80, 16, -127, 112, 8, 45, 127, -27757, 116, 134, 123, 79.00237606484764);
scroll-padding-block-end: 22142ch;
}
</style>
<script>
window.addEventListener("load", async () => {
await new Promise(r => setTimeout(r, 20))
const label = document.createElement("label")
document.documentElement.appendChild(label)
label.scrollIntoView({ "behavior": "smooth", "block": "end", "inline": "end" })
setTimeout('document.documentElement.className = ""', 300);
})
</script>
</head>
</html>

View File

@ -208,3 +208,4 @@ load 1700232.html
load 1704321-1.html
load 1702638.html
load 1730695.html
load 1745775.html

View File

@ -4248,7 +4248,7 @@ impl PicturePrimitive {
}
}
fn resolve_scene_properties(&mut self, properties: &SceneProperties) -> bool {
fn resolve_scene_properties(&mut self, properties: &SceneProperties) {
match self.composite_mode {
Some(PictureCompositeMode::Filter(ref mut filter)) => {
match *filter {
@ -4257,20 +4257,35 @@ impl PicturePrimitive {
}
_ => {}
}
filter.is_visible()
}
_ => true,
_ => {}
}
}
pub fn is_visible(&self) -> bool {
match self.composite_mode {
Some(PictureCompositeMode::Filter(ref filter)) => {
filter.is_visible()
pub fn is_visible(
&self,
spatial_tree: &SpatialTree,
) -> bool {
if let Some(PictureCompositeMode::Filter(ref filter)) = self.composite_mode {
if !filter.is_visible() {
return false;
}
_ => true,
}
// For out-of-preserve-3d pictures, the backface visibility is determined by
// the local transform only.
// Note: we aren't taking the transform relativce to the parent picture,
// since picture tree can be more dense than the corresponding spatial tree.
if !self.is_backface_visible {
if let Picture3DContext::Out = self.context_3d {
match spatial_tree.get_local_visible_face(self.spatial_node_index) {
VisibleFace::Front => {}
VisibleFace::Back => return false,
}
}
}
true
}
// TODO(gw): We have the PictureOptions struct available. We
@ -4321,7 +4336,7 @@ impl PicturePrimitive {
self.primary_render_task_id = None;
self.secondary_render_task_id = None;
if !self.is_visible() {
if !self.is_visible(frame_context.spatial_tree) {
return None;
}
@ -5661,30 +5676,12 @@ impl PicturePrimitive {
/// Do initial checks to determine whether this picture should be drawn as part of the
/// frame build.
pub fn pre_update_visibility_check(
pub fn pre_update(
&mut self,
frame_context: &FrameBuildingContext,
) -> bool {
// Resolve animation properties, and early out if the filter
// properties make this picture invisible.
if !self.resolve_scene_properties(frame_context.scene_properties) {
return false;
}
// For out-of-preserve-3d pictures, the backface visibility is determined by
// the local transform only.
// Note: we aren't taking the transform relativce to the parent picture,
// since picture tree can be more dense than the corresponding spatial tree.
if !self.is_backface_visible {
if let Picture3DContext::Out = self.context_3d {
match frame_context.spatial_tree.get_local_visible_face(self.spatial_node_index) {
VisibleFace::Front => {}
VisibleFace::Back => return false,
}
}
}
true
) {
// Resolve animation properties
self.resolve_scene_properties(frame_context.scene_properties);
}
/// Called during initial picture traversal, before we know the

View File

@ -162,6 +162,9 @@ fn assign_update_pass(
info.parent = parent_pic_index;
// Run pre-update to resolve animation properties etc
pic.pre_update(frame_context);
let can_be_drawn = match info.update_pass {
Some(update_pass) => {
// No point in recursing into paths in the graph if this picture already
@ -174,7 +177,7 @@ fn assign_update_pass(
}
None => {
// Check if this picture can be dropped from the graph we're building this frame
pic.pre_update_visibility_check(frame_context)
pic.is_visible(frame_context.spatial_tree)
}
};

View File

@ -270,7 +270,7 @@ pub fn update_primitive_visibility(
PrimitiveInstanceKind::Picture { pic_index, .. } => {
let (is_visible, is_passthrough) = {
let pic = &store.pictures[pic_index.0];
(pic.is_visible(), pic.raster_config.is_none())
(pic.is_visible(frame_context.spatial_tree), pic.raster_config.is_none())
};
if !is_visible {