Prevent de-jelly from sticking

Produce a new un-skewed frame after any frame rendered with de-jelly
skew. This ensures a skewed frame is never shown for a long period of
time.

Bug: 995965
Change-Id: I6a8839646bacd88a1dff8c859a49c7c58ee6ba8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819495
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702997}

components/viz/service/display/display.cc

@@ -788,6 +788,14 @@ bool Display::SurfaceHasUnackedFrame(const SurfaceId& surface_id) const {
 void Display::DidFinishFrame(const BeginFrameAck& ack) {
   for (auto& observer : observers_)
     observer.OnDisplayDidFinishFrame(ack);
+
+  // Only used with experimental de-jelly effect. Forces us to produce a new
+  // un-skewed frame if the last one had a de-jelly skew applied. This prevents
+  // de-jelly skew from staying on screen for more than one frame.
+  if (aggregator_->last_frame_had_jelly()) {
+    scheduler_->SetNeedsOneBeginFrame();
+    scheduler_->set_needs_draw();
+  }
 }
 
 const SurfaceId& Display::CurrentSurfaceId() {

components/viz/service/display/display_scheduler.h

@@ -88,6 +88,7 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
   void OnSurfaceDestroyed(const SurfaceId& surface_id) override;
   void OnSurfaceDamageExpected(const SurfaceId& surface_id,
                                const BeginFrameArgs& args) override;
+  void set_needs_draw() { needs_draw_ = true; }
 
  protected:
   // These values inidicate how a response to the BeginFrame should be

components/viz/service/display/surface_aggregator.cc

@@ -169,8 +169,8 @@ SurfaceAggregator::SurfaceAggregator(SurfaceManager* manager,
       provider_(provider),
       next_render_pass_id_(1),
       aggregate_only_damaged_(aggregate_only_damaged),
-      needs_surface_occluding_damage_rect_(
-          needs_surface_occluding_damage_rect) {
+      needs_surface_occluding_damage_rect_(needs_surface_occluding_damage_rect),
+      de_jelly_enabled_(DeJellyEnabled()) {
   DCHECK(manager_);
 }
 
@@ -993,6 +993,14 @@ void SurfaceAggregator::CopyQuadsToPass(
             new_rounded_corner_info, occluding_damage_rect,
             occluding_damage_rect_valid);
 
+        if (de_jelly_enabled_) {
+          // If a surface is being drawn for a second time, clear our
+          // |de_jelly_delta_y|, as de-jelly is only needed the first time
+          // a surface draws.
+          if (!new_surfaces_.count(surface_id))
+            dest_shared_quad_state->de_jelly_delta_y = 0.0f;
+        }
+
         last_copied_source_shared_quad_state = quad->shared_quad_state;
         if (ignore_undamaged) {
           damage_rect_in_quad_space_valid = CalculateQuadSpaceDamageRect(
@@ -1440,6 +1448,9 @@ gfx::Rect SurfaceAggregator::PrewalkTree(Surface* surface,
   // them too.
   surface->TakeCopyOutputRequestsFromClient();
 
+  if (de_jelly_enabled_ && surface->HasUndrawnActiveFrame())
+    new_surfaces_.insert(surface->surface_id());
+
   if (will_draw)
     surface->OnWillBeDrawn();
 
@@ -1603,6 +1614,7 @@ CompositorFrame SurfaceAggregator::Aggregate(
   has_cached_render_passes_ = false;
   damage_ranges_.clear();
   damage_rects_union_of_surfaces_on_top_ = gfx::Rect();
+  new_surfaces_.clear();
   DCHECK(referenced_surfaces_.empty());
   PrewalkResult prewalk_result;
   root_damage_rect_ =
@@ -1735,8 +1747,10 @@ bool SurfaceAggregator::IsRootSurface(const Surface* surface) const {
 void SurfaceAggregator::HandleDeJelly(Surface* surface) {
   TRACE_EVENT0("viz", "SurfaceAggregator::HandleDeJelly");
 
-  if (dest_pass_list_->empty() || !DeJellyActive())
+  if (dest_pass_list_->empty() || !DeJellyActive()) {
+    SetLastFrameHadJelly(false);
     return;
+  }
 
   // |jelly_clip| is the rect that contains all de-jelly'd quads. It is used as
   // an approximation for the containing non-skewed clip rect.
@@ -1765,8 +1779,12 @@ void SurfaceAggregator::HandleDeJelly(Surface* surface) {
   }
 
   // Exit if nothing was skewed.
-  if (max_skew == 0.0f)
+  if (max_skew == 0.0f) {
+    SetLastFrameHadJelly(false);
     return;
+  }
+
+  SetLastFrameHadJelly(true);
 
   // Remove the existing root render pass and create a new one which we will
   // re-copy skewed quads / render-passes to.
@@ -1986,4 +2004,15 @@ void SurfaceAggregator::AppendDeJellyQuadsForSharedQuadState(
   }
 }
 
+void SurfaceAggregator::SetLastFrameHadJelly(bool had_jelly) {
+  // If we've just rendererd a jelly-free frame after one with jelly, we must
+  // damage the entire surface, as we may have removed jelly from an otherwise
+  // unchanged quad.
+  if (last_frame_had_jelly_ && !had_jelly) {
+    RenderPass* root_pass = dest_pass_list_->back().get();
+    root_pass->damage_rect = root_pass->output_rect;
+  }
+  last_frame_had_jelly_ = had_jelly;
+}
+
 }  // namespace viz

components/viz/service/display/surface_aggregator.h

@@ -64,6 +64,9 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
   void SetFullDamageForSurface(const SurfaceId& surface_id);
   void set_output_is_secure(bool secure) { output_is_secure_ = secure; }
 
+  // Only used with experimental de-jelly effect.
+  bool last_frame_had_jelly() const { return last_frame_had_jelly_; }
+
   // Set the color spaces for the created RenderPasses, which is propagated
   // to the output surface.
   void SetOutputColorSpace(const gfx::ColorSpace& output_color_space);
@@ -282,6 +285,8 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
       const cc::ListContainer<DrawQuad>::Iterator& end,
       RenderPass* render_pass,
       const SharedQuadState* state);
+  // Update |last_frame_had_jelly_|, should be called once per frame.
+  void SetLastFrameHadJelly(bool had_jelly);
 
   SurfaceManager* manager_;
   DisplayResourceProvider* provider_;
@@ -382,6 +387,15 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
   int64_t display_trace_id_ = -1;
   base::flat_set<SurfaceId> undrawn_surfaces_;
 
+  // Variables used for de-jelly:
+  // Whether de-jelly may be active.
+  bool de_jelly_enabled_ = false;
+  // The set of surfacees being drawn for the first time. Used to determine if
+  // de-jelly skew should be applied to a surface.
+  base::flat_set<SurfaceId> new_surfaces_;
+  // Whether the last drawn frame had de-jelly skew applied.
+  bool last_frame_had_jelly_ = false;
+
   base::WeakPtrFactory<SurfaceAggregator> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SurfaceAggregator);