Commit f38119e6 authored by yjliu's avatar yjliu Committed by Commit Bot

Fixed the flickering when pixel-moving backdrop filters are present.

The problem was caused by the expansion of damage under the pixel-moving
backdrop filters didn't take into account the damage coming from an
earlier stage (cc) in the pipeline.

Bug: 1107376
Change-Id: Ie734cdbec910b17bb4b12fc9f209345e04ad12e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310389
Commit-Queue: Jun Liu <yjliu@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791448}
parent e2fb697b
...@@ -1229,6 +1229,7 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1229,6 +1229,7 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
base::flat_map<RenderPassId, RenderPassMapEntry>* render_pass_map, base::flat_map<RenderPassId, RenderPassMapEntry>* render_pass_map,
bool will_draw, bool will_draw,
const gfx::Rect& damage_from_parent, const gfx::Rect& damage_from_parent,
const gfx::Transform& target_to_root_transform,
PrewalkResult* result) { PrewalkResult* result) {
if (render_pass_entry->is_visited) { if (render_pass_entry->is_visited) {
// This render pass is an ancestor of itself and is not supported. // This render pass is an ancestor of itself and is not supported.
...@@ -1255,11 +1256,25 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1255,11 +1256,25 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
has_pixel_moving_filter || has_pixel_moving_filter ||
base::Contains(moved_pixel_passes_, remapped_pass_id); base::Contains(moved_pixel_passes_, remapped_pass_id);
gfx::Transform root_to_target_transform(gfx::Transform::kSkipInitialization);
const bool transform_inverted =
target_to_root_transform.GetInverse(&root_to_target_transform);
DCHECK(transform_inverted);
const CompositorFrame& frame = surface->GetActiveFrame(); const CompositorFrame& frame = surface->GetActiveFrame();
RenderPass* last_pass = frame.render_pass_list.back().get(); RenderPass* last_pass = frame.render_pass_list.back().get();
gfx::Rect full_damage = last_pass->output_rect; gfx::Rect full_damage = last_pass->output_rect;
gfx::Rect damage_rect_for_surface =
// The damage on the root render pass of the surface comes from damage
// accumulated from all quads in the surface, and needs to be expanded by any
// pixel-moving backdrop filter in the render pass if intersecting. Transform
// this damage into the local space of the render pass for this purpose.
gfx::Rect surface_root_rp_damage =
DamageRectForSurface(surface, *last_pass, full_damage); DamageRectForSurface(surface, *last_pass, full_damage);
if (!surface_root_rp_damage.IsEmpty()) {
surface_root_rp_damage = cc::MathUtil::ProjectEnclosingClippedRect(
root_to_target_transform, surface_root_rp_damage);
}
gfx::Rect damage_rect; gfx::Rect damage_rect;
// Iterate through the quad list back-to-front and accumulate damage from // Iterate through the quad list back-to-front and accumulate damage from
...@@ -1298,19 +1313,20 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1298,19 +1313,20 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
// If the surface quad is to be merged potentially, the current // If the surface quad is to be merged potentially, the current
// effective accumulated damage needs to be taken into account. This // effective accumulated damage needs to be taken into account. This
// includes the damage from quads under the surface quad, i.e. // includes the damage from quads under the surface quad, i.e.
// |damage_rect|, |damage_rect_for_surface| and |damage_from_parent|. // |damage_rect|, |surface_root_rp_damage|, which can contain damage
// The damage is first transformed into the local space of the surface // contributed by quads under the surface quad in the previous stage
// quad and then passed to the embedding surface. The condition for // (cc), and |damage_from_parent|. The damage is first transformed into
// deciding if the surface quad will merge is loose here, so for those // the local space of the surface quad and then passed to the embedding
// quads passed this condition but eventually don't merge, there is // surface. The condition for deciding if the surface quad will merge is
// over-contribution of the damage passed from parent, but this // loose here, so for those quads passed this condition but eventually
// shouldn't affect correctness. // don't merge, there is over-contribution of the damage passed from
// parent, but this shouldn't affect correctness.
gfx::Rect accumulated_damage_in_child_space; gfx::Rect accumulated_damage_in_child_space;
if (CanPotentiallyMergePass(*surface_quad)) { if (CanPotentiallyMergePass(*surface_quad)) {
accumulated_damage_in_child_space.Union(damage_rect); accumulated_damage_in_child_space.Union(damage_rect);
accumulated_damage_in_child_space.Union(damage_from_parent); accumulated_damage_in_child_space.Union(damage_from_parent);
accumulated_damage_in_child_space.Union(damage_rect_for_surface); accumulated_damage_in_child_space.Union(surface_root_rp_damage);
if (!accumulated_damage_in_child_space.IsEmpty()) { if (!accumulated_damage_in_child_space.IsEmpty()) {
gfx::Transform inverse(gfx::Transform::kSkipInitialization); gfx::Transform inverse(gfx::Transform::kSkipInitialization);
bool inverted = bool inverted =
...@@ -1334,7 +1350,8 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1334,7 +1350,8 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
continue; continue;
if (in_moved_pixel_pass) { if (in_moved_pixel_pass) {
damage_rect = full_damage; damage_rect = cc::MathUtil::ProjectEnclosingClippedRect(
root_to_target_transform, full_damage);
continue; continue;
} }
} else if (quad->material == DrawQuad::Material::kRenderPass) { } else if (quad->material == DrawQuad::Material::kRenderPass) {
...@@ -1351,16 +1368,19 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1351,16 +1368,19 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
quad->shared_quad_state->quad_to_target_transform, quad->shared_quad_state->quad_to_target_transform,
child_render_pass.output_rect); child_render_pass.output_rect);
// Both |damage_rect| and |damage_from_parent| are from under the quad, // |damage_rect|, |damage_from_parent| and |surface_root_rp_damage|
// so if they intersect the quad render pass output rect, we have to // either are or can possible contain damage from under the quad, so if
// invalidate the |can_use_backdrop_filter_cache| flag. Note the // they intersect the quad render pass output rect, we have to invalidate
// intersection test can be done against backdrop filter bounds as an // the |can_use_backdrop_filter_cache| flag. Note the intersection test
// improvement. // can be done against backdrop filter bounds as an improvement.
bool intersects_current_damage = bool intersects_current_damage =
rect_in_target_space.Intersects(damage_rect); rect_in_target_space.Intersects(damage_rect);
bool intersects_damage_from_parent = bool intersects_damage_from_parent =
rect_in_target_space.Intersects(damage_from_parent); rect_in_target_space.Intersects(damage_from_parent);
if (intersects_current_damage || intersects_damage_from_parent) { bool intersects_damage_from_surface =
rect_in_target_space.Intersects(surface_root_rp_damage);
if (intersects_current_damage || intersects_damage_from_parent ||
intersects_damage_from_surface) {
render_pass_quad->can_use_backdrop_filter_cache = false; render_pass_quad->can_use_backdrop_filter_cache = false;
if (child_render_pass.backdrop_filters.HasFilterThatMovesPixels()) { if (child_render_pass.backdrop_filters.HasFilterThatMovesPixels()) {
...@@ -1368,12 +1388,16 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1368,12 +1388,16 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
// rect and it has to be expanded because of the pixel-moving // rect and it has to be expanded because of the pixel-moving
// backdrop filters. We expand the |damage_rect| to include quad // backdrop filters. We expand the |damage_rect| to include quad
// render pass output rect (which can be optimized to be backdrop // render pass output rect (which can be optimized to be backdrop
// filter bounds). |damage_from_parent| only has to be included when // filter bounds). |damage_from_parent| and |surface_root_rp_damage|
// it also has intersection with the quad. // only have to be included when they also have intersection with the
// quad.
damage_rect.Union(rect_in_target_space); damage_rect.Union(rect_in_target_space);
if (intersects_damage_from_parent) { if (intersects_damage_from_parent) {
damage_rect.Union(damage_from_parent); damage_rect.Union(damage_from_parent);
} }
if (intersects_damage_from_surface) {
damage_rect.Union(surface_root_rp_damage);
}
} }
} }
...@@ -1384,9 +1408,13 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass( ...@@ -1384,9 +1408,13 @@ gfx::Rect SurfaceAggregator::PrewalkRenderPass(
render_pass_dependencies_[remapped_pass_id].insert( render_pass_dependencies_[remapped_pass_id].insert(
remapped_child_pass_id); remapped_child_pass_id);
quad_damage_rect =
PrewalkRenderPass(&child_render_pass_entry, surface, render_pass_map, const gfx::Transform child_to_root_transform(
will_draw, gfx::Rect(), result); target_to_root_transform,
quad->shared_quad_state->quad_to_target_transform);
quad_damage_rect = PrewalkRenderPass(
&child_render_pass_entry, surface, render_pass_map, will_draw,
gfx::Rect(), child_to_root_transform, result);
} else { } else {
continue; continue;
...@@ -1486,7 +1514,8 @@ gfx::Rect SurfaceAggregator::PrewalkSurface(Surface* surface, ...@@ -1486,7 +1514,8 @@ gfx::Rect SurfaceAggregator::PrewalkSurface(Surface* surface,
RenderPassMapEntry& entry = it->second; RenderPassMapEntry& entry = it->second;
damage_rect.Union(PrewalkRenderPass(&entry, surface, &render_pass_map, damage_rect.Union(PrewalkRenderPass(&entry, surface, &render_pass_map,
will_draw, damage_from_parent, result)); will_draw, damage_from_parent,
gfx::Transform(), result));
damage_rect = cc::MathUtil::MapEnclosedRectWith2dAxisAlignedTransform( damage_rect = cc::MathUtil::MapEnclosedRectWith2dAxisAlignedTransform(
root_pass_transform, damage_rect); root_pass_transform, damage_rect);
......
...@@ -204,6 +204,8 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator { ...@@ -204,6 +204,8 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
// the surface quad in the render pass merged to, plus its |damage_rect| // the surface quad in the render pass merged to, plus its |damage_rect|
// and damage passed onto it by its parent if any. // and damage passed onto it by its parent if any.
// If there's no merging of |surface|, |accummulated_damage| is empty. // If there's no merging of |surface|, |accummulated_damage| is empty.
// - |target_to_root_transform| is the transform from current render pass to
// the root.
// - |result| is the result of a prewalk of the surface that contains the // - |result| is the result of a prewalk of the surface that contains the
// render pass. // render pass.
gfx::Rect PrewalkRenderPass( gfx::Rect PrewalkRenderPass(
...@@ -212,6 +214,7 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator { ...@@ -212,6 +214,7 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
base::flat_map<RenderPassId, RenderPassMapEntry>* render_pass_map, base::flat_map<RenderPassId, RenderPassMapEntry>* render_pass_map,
bool will_draw, bool will_draw,
const gfx::Rect& damage_from_parent, const gfx::Rect& damage_from_parent,
const gfx::Transform& target_to_root_transform,
PrewalkResult* result); PrewalkResult* result);
// Walk the Surface tree from |surface|. Validate the resources of the // Walk the Surface tree from |surface|. Validate the resources of the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment