Mozart: Reduce pipeline depth and unify frame queue.

services/gfx/compositor/backend/gpu_output.h

Index: services/gfx/compositor/backend/gpu_output.h
diff --git a/services/gfx/compositor/backend/gpu_output.h b/services/gfx/compositor/backend/gpu_output.h
index d7fe5f559cefef592824f44c0640c4f11c67efca..91c9510d10d51673ac8fbcaef17c3a6799ba94bc 100644
--- a/services/gfx/compositor/backend/gpu_output.h
+++ b/services/gfx/compositor/backend/gpu_output.h
@@ -34,22 +34,20 @@ class GpuOutput : public Output, private GpuRasterizer::Callbacks {
   void SubmitFrame(const scoped_refptr<RenderFrame>& frame) override;
 
  private:
-  struct FrameData : public base::RefCountedThreadSafe<FrameData> {
+  struct FrameData {
     FrameData(const scoped_refptr<RenderFrame>& frame, int64_t submit_time);
+    ~FrameData();
 
-    void Recycle();
+    void ClearDrawState();
 
     const scoped_refptr<RenderFrame> frame;
     const int64_t submit_time;

[mikejurka, 2016/05/25 21:11:46]

i would be a bit more descriptive, call this draw_submitted_time

[jeffbrown, 2016/05/25 23:45:44]

I think that might get confused with draw_time.  submit_time is actually set in
SubmitFrame which I think makes sense.

-    bool drawn = false;     // set when DrawFrame is called
-    int64_t draw_time = 0;  // 0 if not drawn
-    int64_t wait_time = 0;  // 0 if not drawn
+    bool drawn = false;     // set when |DrawFrame| is called
+    bool finished = false;  // set when drawing is finished

[mikejurka, 2016/05/25 21:11:46]

why not call these draw_started and draw_finished?

also, as mentioned in the other file, consider having a pinned_for_draw
variable.

[jeffbrown, 2016/05/25 23:45:44]

Sure, draw_started and draw_finished makes sense.

+    int64_t draw_time = 0;  // time when drawing began

[mikejurka, 2016/05/25 21:11:46]

similarly, why not call this draw_start_time

[jeffbrown, 2016/05/25 23:45:44]

Ok.

+    int64_t wait_time = 0;  // time when awaiting for finish began

[mikejurka, 2016/05/25 21:11:46]

wait_time implies a timespan rather than an absolute time. what about something
like rasterizer_done_time or draw_end_time (avoiding the word "finished" because
you have specific semantics for that ie thats when your callback fires).

[jeffbrown, 2016/05/25 23:45:43]

draw_issued_time?

 
    private:
-    friend class base::RefCountedThreadSafe<FrameData>;
-
-    ~FrameData();
-
     DISALLOW_COPY_AND_ASSIGN(FrameData);
   };
 
@@ -86,12 +84,20 @@ class GpuOutput : public Output, private GpuRasterizer::Callbacks {
   struct {
     std::mutex mutex;  // guards all shared state
 
-    // The most recently submitted frame.
-    // Only null until the first frame has been submitted.
-    scoped_refptr<FrameData> current_frame_data;
-
-    // Frames drawn and awaiting completion by the rasterizer.
-    std::queue<scoped_refptr<FrameData>> drawn_frames_awaiting_finish;
+    // Queue of frames.
+    //
+    // The head of this queue consists of up to |pipeline_depth| frames
+    // which are drawn and awaiting finish.  These frames are popped off
+    // the queue when finished unless the queue would become empty (such
+    // that we always retain the current frame as the tail).
+    //
+    // The tail of this queue is a single frame which is either drawn or
+    // finished and represents the current (most recently submitted)
+    // content.
+    //
+    // The queue is only ever empty until the first frame is submitted.
+    // Subsequently, it always contains at least one frame.
+    std::queue<std::unique_ptr<FrameData>> frames;
 
     // Set to true when the rasterizer is ready to draw.
     bool rasterizer_ready = false;