Skip to content

Commit

Permalink
view-transitions: Don't damage full surface during transitions.
Browse files Browse the repository at this point in the history
If the frame for a Surface was replaced by SurfaceAnimationManager, the
code assumes the whole surface needs to be damaged. This was needed when
the animation was viz driven, that code didn't compute the damage rect.
But now that its all driven in the renderer, CC should be setting up
the damage rects correctly.

[email protected], [email protected]

Change-Id: I4e5053237c9533faef16b78874fa66e06f7542df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4367538
Commit-Queue: Maggie Chen <[email protected]>
Auto-Submit: Khushal Sagar <[email protected]>
Reviewed-by: Maggie Chen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1121404}
  • Loading branch information
khushalsagar authored and Chromium LUCI CQ committed Mar 23, 2023
1 parent a6087b4 commit 05671df
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 37 deletions.
10 changes: 0 additions & 10 deletions components/viz/service/display/resolved_frame_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,16 +269,6 @@ bool ResolvedFrameData::IsNextFrameSinceLastAggregation() const {
gfx::Rect ResolvedFrameData::GetSurfaceDamage() const {
DCHECK(valid_);

// The |damage_rect| set in |SurfaceAnimationManager| is the |output_rect|.
// However, we dont use |damage_rect| because when we transition from
// interpolated frame we would end up using the |damage_rect| from the
// original non interpolated frame.
// TODO(vmpstr): This damage may be too large, but I think it's hard to figure
// out a small bounds on the damage given an animation that happens in
// SurfaceAnimationManager.
if (surface_->HasSurfaceAnimationDamage())
return GetOutputRect();

if (IsSameFrameAsLastAggregation()) {
return gfx::Rect();
} else if (IsNextFrameSinceLastAggregation()) {
Expand Down
10 changes: 1 addition & 9 deletions components/viz/service/display/surface_aggregator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,6 @@ const DrawQuad* SurfaceAggregator::FindQuadWithOverlayDamage(
const gfx::Transform& parent_target_transform,
const Surface* surface,
size_t* overlay_damage_index) {
// If we have damage from a surface animation, then we shouldn't have an
// overlay candidate from the root render pass, since that's an interpolated
// pass with "artificial" damage.
if (surface->HasSurfaceAnimationDamage())
return nullptr;

// Only process the damage rect at the root render pass, once per surface.
const CompositorFrame& frame = surface->GetActiveFrame();
bool is_last_pass_on_src_surface =
Expand Down Expand Up @@ -552,8 +546,7 @@ ResolvedFrameData* SurfaceAggregator::GetResolvedFrame(
// If there is a new CompositorFrame for `surface` compute resolved frame
// data for the new resolved CompositorFrame.
if (resolved_frame.previous_frame_index() !=
surface->GetActiveFrameIndex() ||
surface->HasSurfaceAnimationDamage()) {
surface->GetActiveFrameIndex()) {
base::ElapsedTimer timer;
ProcessResolvedFrame(resolved_frame);
stats_->declare_resources_time += timer.Elapsed();
Expand Down Expand Up @@ -912,7 +905,6 @@ void SurfaceAggregator::EmitSurfaceContent(
}

referenced_surfaces_.erase(surface_id);
surface->DidAggregate();
}

void SurfaceAggregator::EmitDefaultBackgroundColorQuad(
Expand Down
13 changes: 0 additions & 13 deletions components/viz/service/surfaces/surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -732,23 +732,10 @@ const CompositorFrameMetadata& Surface::GetActiveFrameMetadata() const {
return active_frame_data_->frame.metadata;
}

void Surface::ResetInterpolatedFrame() {
interpolated_frame_.reset();
has_damage_from_interpolated_frame_ = true;
}

void Surface::SetInterpolatedFrame(CompositorFrame frame) {
interpolated_frame_.emplace(std::move(frame));
}

bool Surface::HasSurfaceAnimationDamage() const {
return interpolated_frame_.has_value() || has_damage_from_interpolated_frame_;
}

void Surface::DidAggregate() {
has_damage_from_interpolated_frame_ = false;
}

const CompositorFrame& Surface::GetPendingFrame() {
DCHECK(pending_frame_data_);
return pending_frame_data_->frame;
Expand Down
5 changes: 0 additions & 5 deletions components/viz/service/surfaces/surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ class VIZ_SERVICE_EXPORT Surface final {
const CompositorFrame& GetActiveFrame() const;
const CompositorFrameMetadata& GetActiveFrameMetadata() const;

void ResetInterpolatedFrame();
void SetInterpolatedFrame(CompositorFrame frame);
const CompositorFrame& GetActiveOrInterpolatedFrame() const;
bool HasInterpolatedFrame() const;
Expand Down Expand Up @@ -311,8 +310,6 @@ class VIZ_SERVICE_EXPORT Surface final {
std::unique_ptr<CopyOutputRequest> copy_request,
CompositorRenderPassId render_pass_id);

void DidAggregate();

// Returns frame id of the oldest uncommitted frame if any,
absl::optional<BeginFrameId> GetFirstUncommitedFrameId();

Expand Down Expand Up @@ -440,8 +437,6 @@ class VIZ_SERVICE_EXPORT Surface final {

const raw_ptr<SurfaceAllocationGroup> allocation_group_;

bool has_damage_from_interpolated_frame_ = false;

const size_t max_uncommitted_frames_;

base::WeakPtrFactory<Surface> weak_factory_{this};
Expand Down

0 comments on commit 05671df

Please sign in to comment.