cc: Consider Surface active if frame received recently

cc/surfaces/display_scheduler.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_SURFACES_DISPLAY_SCHEDULER_H_
 #define CC_SURFACES_DISPLAY_SCHEDULER_H_
 
 #include "base/cancelable_callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 34 matching lines @@
   void OutputSurfaceLost();
 
   // BeginFrameObserverBase implementation
   bool OnBeginFrameDerivedImpl(const BeginFrameArgs& args) override;
 
  protected:
   base::TimeTicks DesiredBeginFrameDeadlineTime();
   virtual void ScheduleBeginFrameDeadline();
   void AttemptDrawAndSwap();
   void OnBeginFrameDeadline();
+  void UpdateSurfaceDamageExpectations();
   void DrawAndSwap();
 
   DisplaySchedulerClient* client_;
   BeginFrameSource* begin_frame_source_;
   base::SingleThreadTaskRunner* task_runner_;
 
   BeginFrameArgs current_begin_frame_args_;
   base::Closure begin_frame_deadline_closure_;
   base::CancelableClosure begin_frame_deadline_task_;
   base::TimeTicks begin_frame_deadline_task_time_;
 
   bool output_surface_lost_;
   bool root_surface_resources_locked_;
 
   bool inside_begin_frame_deadline_interval_;
   bool needs_draw_;
   bool entire_display_damaged_;
   bool all_active_child_surfaces_ready_to_draw_;
 
   int pending_swaps_;
   int max_pending_swaps_;
 
   SurfaceId root_surface_id_;
   bool root_surface_damaged_;
   bool expect_damage_from_root_surface_;
 
   std::set<SurfaceId> child_surface_ids_damaged_;
   std::set<SurfaceId> child_surface_ids_damaged_prev_;
+  std::set<SurfaceId> child_surface_ids_damaged_prev_prev_;

[sunnyps, 2015/07/23 03:02:40]

Can you use an array of sets here?

Optional: I've found that instead of storing damage for the previous N frames,
it's easier to store the expected damage for the next N frames. Then every time
you have damage you add it to the next N frames. This works well in conjunction
with a circular buffer so you can just move the pointer/index to the correct set
(or damage) every frame and you don't need to calculate it every frame.

This is what BufferQueue does - it tracks damage for ChromeOS ozone surfaceless
implementation.

If you do make this change can you also call it active_child_surface_ids_?

[brianderson, 2015/07/23 18:21:46]

Sure, although I'd like to keep child_surface_ids_damaged_ separate since it has
special meaning for the current frame.

If I stick with the current approach, I'll probably just put prev and prev_prev
in an array together. If I take your approach below, I could also include
child_surface_ids_to_expect_damage_from_ in the array which would be nice.
Nice observation! Now that the code does a union of the damaged IDs instead of
an intersection, this approach would work and I think it would result in more
readable code, especially with the variable renaming.

I think there would be a small overall increase in runtime cost for a large
number of damaged surfaces, but I'm not sure how much we care about that use
case, especially since this heuristic will likely be gone if a time comes where
we do care.

All per-frame costs of the current vs. proposed approach are equal except for
updating damage expectations. If we Let N be the number of surfaces damaged:
  Current cost: 2 * O(N): Two calls to STLSetUnion;
  Proposed cost: 2 * O(N log N): Two calls to set::insert per damaged surface.

For the values of N we are working with right now, it probably doesn't matter
though. Wdyt?
That would be a good name.

[sunnyps, 2015/07/23 21:10:38]

It doesn't look like this could ever be a performance bottleneck - either
std::set or base::hash_set is ok. I don't think we can use std::unordered_set
yet - it's a part of C++ 11 standard library.

   std::vector<SurfaceId> child_surface_ids_to_expect_damage_from_;
 
   base::WeakPtrFactory<DisplayScheduler> weak_ptr_factory_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DisplayScheduler);
 };
 
 }  // namespace cc
 
 #endif  // CC_SURFACES_DISPLAY_SCHEDULER_H_