cc: Consider Surface active if frame received recently

cc/surfaces/display_scheduler.h

Index: cc/surfaces/display_scheduler.h
diff --git a/cc/surfaces/display_scheduler.h b/cc/surfaces/display_scheduler.h
index bc509c7b1caa080107b07c9e2bbeb63bbea8a931..c5fd6f5bd795abb1c496311f45e468c889081098 100644
--- a/cc/surfaces/display_scheduler.h
+++ b/cc/surfaces/display_scheduler.h
@@ -52,6 +52,7 @@ class CC_SURFACES_EXPORT DisplayScheduler : public BeginFrameObserverBase {
   virtual void ScheduleBeginFrameDeadline();
   void AttemptDrawAndSwap();
   void OnBeginFrameDeadline();
+  void UpdateSurfaceDamageExpectations();
   void DrawAndSwap();
 
   DisplaySchedulerClient* client_;
@@ -80,6 +81,7 @@ class CC_SURFACES_EXPORT DisplayScheduler : public BeginFrameObserverBase {
 
   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_;