Commit 82f758de authored by Khushal's avatar Khushal Committed by Commit Bot

android: Don't use partial damage with SurfaceControl.

When using the SurfaceControl API, the damage rect specified for doing
partial invalidations of a Surface should be in the content space
(including pre-transform for rotations). Since we were currently setting
it in screen space, this caused a rendering bug on devices using the
damage rect.

For now, avoid setting any damage rect on the Surface. This will result
in full invalidation of the Surface for each draw. While its not ideal,
its consistent with SurfaceView based rendering path. We can attempt to
use partial damage going forward, with some benchmarking, once
SurfaceControl is launched/stable.

Note that we still take advantage of only compositing the damaged region
in chrome's display compositor. This change only makes it so this region
is not forwarded to the system compositor.

R=zmo@chromium.org

Bug: 988857
Change-Id: I625dcda578e90aa81324c4e35f739bd44b83c3f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725416
Auto-Submit: Khushal <khushalsagar@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682145}
parent 252adb9c
...@@ -191,30 +191,20 @@ void GLSurfaceEGLSurfaceControl::CommitPendingTransaction( ...@@ -191,30 +191,20 @@ void GLSurfaceEGLSurfaceControl::CommitPendingTransaction(
return; return;
} }
// Mark the intersection of a surface's rect with the damage rect as the dirty
// rect for that surface.
DCHECK_LE(pending_surfaces_count_, surface_list_.size());
const gfx::Rect damage_rect_in_screen_space =
ApplyDisplayInverse(damage_rect);
for (size_t i = 0; i < pending_surfaces_count_; ++i) {
const auto& surface_state = surface_list_[i];
if (!surface_state.buffer_updated_in_pending_transaction)
continue;
gfx::Rect surface_damage_rect = surface_state.dst;
surface_damage_rect.Intersect(damage_rect_in_screen_space);
pending_transaction_->SetDamageRect(*surface_state.surface,
surface_damage_rect);
}
// Surfaces which are present in the current frame but not in the next frame // Surfaces which are present in the current frame but not in the next frame
// need to be explicitly updated in order to get a release fence for them in // need to be explicitly updated in order to get a release fence for them in
// the next transaction. // the next transaction.
DCHECK_LE(pending_surfaces_count_, surface_list_.size());
for (size_t i = pending_surfaces_count_; i < surface_list_.size(); ++i) { for (size_t i = pending_surfaces_count_; i < surface_list_.size(); ++i) {
pending_transaction_->SetBuffer(*surface_list_[i].surface, nullptr, pending_transaction_->SetBuffer(*surface_list_[i].surface, nullptr,
base::ScopedFD()); base::ScopedFD());
} }
// TODO(khushalsagar): Consider using the SetDamageRect API for partial
// invalidations. Note that the damage rect set should be in the space in
// which the content is rendered (including the pre-transform). See
// crbug.com/988857 for details.
// Release resources for the current frame once the next frame is acked. // Release resources for the current frame once the next frame is acked.
ResourceRefs resources_to_release; ResourceRefs resources_to_release;
resources_to_release.swap(current_frame_resources_); resources_to_release.swap(current_frame_resources_);
......
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