exo: Add max frames pending flag to motion event client.

components/exo/wayland/clients/motion_events.cc

Index: components/exo/wayland/clients/motion_events.cc
diff --git a/components/exo/wayland/clients/motion_events.cc b/components/exo/wayland/clients/motion_events.cc
index e1d89c1252aedab51190d1a4d575294b3c541039..e8db39514c48b8981076c5e0534717e0033b1359 100644
--- a/components/exo/wayland/clients/motion_events.cc
+++ b/components/exo/wayland/clients/motion_events.cc
@@ -11,6 +11,7 @@
 #include <wayland-client-core.h>
 #include <wayland-client-protocol.h>
 
+#include <deque>
 #include <iostream>
 #include <string>
 #include <vector>
@@ -35,6 +36,7 @@
 #include "ui/gl/gl_enums.h"
 #include "ui/gl/gl_surface.h"
 #include "ui/gl/gl_surface_egl.h"
+#include "ui/gl/gpu_timing.h"
 #include "ui/gl/init/gl_factory.h"
 #include "ui/gl/scoped_make_current.h"
 
@@ -98,7 +100,7 @@ const char kDriRenderNodeTemplate[] = "/dev/dri/renderD%u";
 #endif
 
 // Number of buffers.
-const size_t kBuffers = 3;
+const size_t kBuffers = 8;
 
 // Rotation speed (degrees/second).
 const double kRotationSpeed = 360.0;
@@ -300,29 +302,30 @@ class MotionEvents {
                size_t height,
                int scale,
                size_t num_rects,
-               const std::string* use_drm,
-               bool fullscreen)
+               size_t max_frames_pending,
+               bool fullscreen,
+               const std::string* use_drm)
       : width_(width),
         height_(height),
         scale_(scale),
         num_rects_(num_rects),
-        use_drm_(use_drm),
-        fullscreen_(fullscreen) {}
+        max_frames_pending_(max_frames_pending),
+        fullscreen_(fullscreen),
+        use_drm_(use_drm) {}
 
   // Initialize and run client main loop.
   int Run();
 
  private:
   bool CreateBuffer(Buffer* buffer);
-  size_t stride() const { return width_ * kBytesPerPixel; }
-  size_t buffer_size() const { return stride() * height_; }
 
   const size_t width_;
   const size_t height_;
   const int scale_;
   const size_t num_rects_;
-  const std::string* use_drm_;
+  const size_t max_frames_pending_;
   const bool fullscreen_;
+  const std::string* use_drm_;
 
   Globals globals_;
   std::unique_ptr<wl_display> display_;
@@ -354,10 +357,15 @@ int MotionEvents::Run() {
   gl_surface_ = gl::init::CreateOffscreenGLSurface(gfx::Size());
   gl_context_ =
       gl::init::CreateGLContext(nullptr,  // share_group
-                                gl_surface_.get(), gl::PreferIntegratedGpu);
+                                gl_surface_.get(), gl::GLContextAttribs());

[Daniele Castagna, 2016/11/12 00:04:05]

What does this do?

[reveman, 2016/11/12 01:18:13]

needed for ToT to compile

[Daniele Castagna, 2016/11/12 01:20:59]

We should at least build this target on the trybots.

 
   make_current_.reset(
       new ui::ScopedMakeCurrent(gl_context_.get(), gl_surface_.get()));
+  scoped_refptr<gl::GPUTimingClient> gpu_timing_client =
+      gl_context_->CreateGPUTimingClient();
+  if (!gpu_timing_client->IsAvailable())
+    LOG(WARNING) << "GPU timing is not available";
+
   wl_registry_listener registry_listener = {RegistryHandler, RegistryRemover};
 
   wl_registry* registry = wl_display_get_registry(display_.get());
@@ -430,6 +438,15 @@ int MotionEvents::Run() {
       kOpenGL_GrBackend,
       reinterpret_cast<GrBackendContext>(native_interface.get())));
   DCHECK(gr_context_);
+
+  EGLenum egl_sync_type = 0;
+  if (gl::GLSurfaceEGL::HasEGLExtension("EGL_EXT_image_flush_external") ||
+      gl::GLSurfaceEGL::HasEGLExtension("EGL_ARM_implicit_external_sync")) {
+    egl_sync_type = EGL_SYNC_FENCE_KHR;

[Daniele Castagna, 2016/11/12 00:04:05]

Nit: Can you use g_driver_egl.ext.b_EGL_EXT_image_flush_external later instead?

[reveman, 2016/11/12 01:18:13]

There's no g_driver_egl.ext.b_ANDROID_native_fence_sync yet

+  }
+  if (gl::GLSurfaceEGL::HasEGLExtension("EGL_ANDROID_native_fence_sync")) {
+    egl_sync_type = EGL_SYNC_NATIVE_FENCE_ANDROID;
+  }
 #endif
 
   wl_buffer_listener buffer_listener = {BufferRelease};
@@ -511,89 +528,158 @@ int MotionEvents::Run() {
   Frame frame;
   std::unique_ptr<wl_callback> frame_callback;
   wl_callback_listener frame_listener = {FrameCallback};
+  std::deque<wl_buffer*> pending_frames;
 
   uint32_t frames = 0;
   base::TimeTicks benchmark_time = base::TimeTicks::Now();
   base::TimeDelta benchmark_interval =
       base::TimeDelta::FromSeconds(kBenchmarkInterval);
+  base::TimeDelta benchmark_wall_time;

[Daniele Castagna, 2016/11/12 00:04:05]

Isn't this going be ~benchmark_interval?

[reveman, 2016/11/12 01:18:13]

This is different as discussed.

+  base::TimeDelta benchmark_cpu_time;
+  std::vector<std::unique_ptr<gl::GPUTimer>> gpu_timers;
 
+  int dispatch_status = 0;
   do {

[Daniele Castagna, 2016/11/12 00:04:05]

I think it'd be much more readable if we could manage to keep the do while body
in a pag

[reveman, 2016/11/12 01:18:13]

Let's clean this up in a follow up instead.

-    if (frame.callback_pending)
-      continue;
+    bool enqueue_frame = frame.callback_pending
+                             ? pending_frames.size() < max_frames_pending_
+                             : pending_frames.empty();
+    if (enqueue_frame) {
+      Buffer* buffer =
+          std::find_if(std::begin(buffers_), std::end(buffers_),
+                       [](const Buffer& buffer) { return !buffer.busy; });
+      if (buffer == std::end(buffers_)) {
+        LOG(WARNING) << "Can't find free buffer";

[Daniele Castagna, 2016/11/12 00:04:05]

nit: LOG(ERROR)?

[reveman, 2016/11/12 01:18:13]

Done.

+        return 1;
+      }
 
-    Buffer* buffer =
-        std::find_if(std::begin(buffers_), std::end(buffers_),
-                     [](const Buffer& buffer) { return !buffer.busy; });
-    if (buffer == std::end(buffers_))
-      continue;
+      base::TimeTicks wall_time_start = base::TimeTicks::Now();
+      if ((wall_time_start - benchmark_time) > benchmark_interval) {
+        base::TimeDelta benchmark_gpu_time;
+        bool gpu_timer_errors = true;
+        if (gpu_timing_client->IsAvailable())
+          gpu_timer_errors = gpu_timing_client->CheckAndResetTimerErrors();
+        if (!gpu_timer_errors) {
+          for (auto& gpu_timer : gpu_timers) {
+            if (gpu_timer->IsAvailable()) {

[Daniele Castagna, 2016/11/12 00:04:04]

If we produce frames as fast as we can just flushing in between and as the last
thing we just check if the timers are available, isn't it really likely the
timers won't be available since the command buffer won't have executed till the
point we inserted the timers?

[reveman, 2016/11/12 01:18:13]

Remove the GPU timers from latest patch.

+              benchmark_gpu_time += base::TimeDelta::FromMicroseconds(
+                  gpu_timer->GetDeltaElapsed());
+            }
+          }
+        }
+        std::cout << frames << " frames in " << benchmark_interval.InSeconds()

[Daniele Castagna, 2016/11/12 00:04:05]

Should this mention that this is the number of frames produced and not
necessarily displayed?

[reveman, 2016/11/12 01:18:13]

Added a comment here about it.

+                  << " seconds: " << frames / benchmark_interval.InSecondsF()
+                  << " fps (wall="
+                  << benchmark_wall_time.InMillisecondsF() / frames
+                  << " cpu=" << benchmark_cpu_time.InMillisecondsF() / frames
+                  << " gpu="
+                  << (!gpu_timer_errors
+                          ? base::DoubleToString(
+                                benchmark_gpu_time.InMillisecondsF() / frames)
+                          : std::string("?"))
+                  << ")" << std::endl;
+
+        frames = 0;
+        benchmark_time = wall_time_start;
+        benchmark_wall_time = base::TimeDelta();
+        benchmark_cpu_time = base::TimeDelta();
+        gpu_timers.clear();
+      }
 
-    base::TimeTicks current_time = base::TimeTicks::Now();
-    if ((current_time - benchmark_time) > benchmark_interval) {
-      std::cout << frames << " frames in " << benchmark_interval.InSeconds()
-                << " seconds: "
-                << static_cast<double>(frames) / benchmark_interval.InSeconds()
-                << " fps" << std::endl;
-      benchmark_time = current_time;
-      frames = 0;
-    }
+      DCHECK(base::ThreadTicks::IsSupported());
+      base::ThreadTicks::WaitUntilInitialized();

[Daniele Castagna, 2016/11/12 00:04:05]

Are you sure this is necessary? And if it is, can these two lines be before the
main loop?

[reveman, 2016/11/12 01:18:13]

Not needed.

+      base::ThreadTicks cpu_time_start = base::ThreadTicks::Now();
+      std::unique_ptr<gl::GPUTimer> gpu_timer;
+      if (gpu_timing_client->IsAvailable()) {
+        gpu_timer = gpu_timing_client->CreateGPUTimer(true);
+        gpu_timer->Start();
+      }
 
-    SkCanvas* canvas = buffer->sk_surface->getCanvas();
-    canvas->save();
-
-    if (event_times.empty()) {
-      canvas->clear(SK_ColorBLACK);
-    } else {
-      // Split buffer into one horizontal rectangle for each event received
-      // since last frame. Latest event at the top.
-      int y = 0;
-      // Note: Rounding up to ensure we cover the whole canvas.
-      int h = (height_ + (event_times.size() / 2)) / event_times.size();
-      while (!event_times.empty()) {
-        SkIRect rect = SkIRect::MakeXYWH(0, y, width_, h);
-        SkPaint paint;
-        paint.setColor(SkColorSetRGB((event_times.back() & 0x0000ff) >> 0,
-                                     (event_times.back() & 0x00ff00) >> 8,
-                                     (event_times.back() & 0xff0000) >> 16));
+      SkCanvas* canvas = buffer->sk_surface->getCanvas();
+      canvas->save();
+
+      if (event_times.empty()) {
+        canvas->clear(SK_ColorBLACK);
+      } else {
+        // Split buffer into one horizontal rectangle for each event received
+        // since last frame. Latest event at the top.
+        int y = 0;
+        // Note: Rounding up to ensure we cover the whole canvas.
+        int h = (height_ + (event_times.size() / 2)) / event_times.size();
+        while (!event_times.empty()) {
+          SkIRect rect = SkIRect::MakeXYWH(0, y, width_, h);
+          SkPaint paint;
+          paint.setColor(SkColorSetRGB((event_times.back() & 0x0000ff) >> 0,
+                                       (event_times.back() & 0x00ff00) >> 8,
+                                       (event_times.back() & 0xff0000) >> 16));
+          canvas->drawIRect(rect, paint);
+          event_times.pop_back();
+          y += h;
+        }
+      }
+
+      // Draw rotating rects.
+      SkScalar half_width = SkScalarHalf(width_);
+      SkScalar half_height = SkScalarHalf(height_);
+      SkIRect rect = SkIRect::MakeXYWH(-SkScalarHalf(half_width),
+                                       -SkScalarHalf(half_height), half_width,
+                                       half_height);
+      SkScalar rotation = SkScalarMulDiv(frame.time, kRotationSpeed, 1000);
+      SkPaint paint;
+      canvas->translate(half_width, half_height);
+      for (size_t i = 0; i < num_rects_; ++i) {
+        const SkColor kColors[] = {SK_ColorBLUE, SK_ColorGREEN,
+                                   SK_ColorRED,  SK_ColorYELLOW,
+                                   SK_ColorCYAN, SK_ColorMAGENTA};
+        paint.setColor(SkColorSetA(kColors[i % arraysize(kColors)], 0xA0));
+        canvas->rotate(rotation / num_rects_);
         canvas->drawIRect(rect, paint);
-        event_times.pop_back();
-        y += h;
       }
-    }
 
-    // Draw rotating rects.
-    SkScalar half_width = SkScalarHalf(width_);
-    SkScalar half_height = SkScalarHalf(height_);
-    SkIRect rect =
-        SkIRect::MakeXYWH(-SkScalarHalf(half_width), -SkScalarHalf(half_height),
-                          half_width, half_height);
-    SkScalar rotation = SkScalarMulDiv(frame.time, kRotationSpeed, 1000);
-    SkPaint paint;
-    canvas->translate(half_width, half_height);
-    for (size_t i = 0; i < num_rects_; ++i) {
-      const SkColor kColors[] = {SK_ColorBLUE, SK_ColorGREEN,
-                                 SK_ColorRED,  SK_ColorYELLOW,
-                                 SK_ColorCYAN, SK_ColorMAGENTA};
-      paint.setColor(SkColorSetA(kColors[i % arraysize(kColors)], 0xA0));
-      canvas->rotate(rotation / num_rects_);
-      canvas->drawIRect(rect, paint);
-    }
-    canvas->restore();
-    if (gr_context_) {
-      gr_context_->flush();
-      glFinish();
-    }
-    wl_surface_set_buffer_scale(surface.get(), scale_);
-    wl_surface_attach(surface.get(), buffer->buffer.get(), 0, 0);
-    buffer->busy = true;
+      canvas->restore();
+      if (gr_context_) {
+        gr_context_->flush();
+        glFlush();
+
+        if (egl_sync_type) {
+          EGLSyncKHR sync =
+              eglCreateSyncKHR(eglGetCurrentDisplay(), egl_sync_type, nullptr);
+          DCHECK(sync != EGL_NO_SYNC_KHR);

[Daniele Castagna, 2016/11/12 00:04:05]

nit: DCHECK_NE?
Nevermind, you'd need the cast to unsigned.

+          eglClientWaitSyncKHR(eglGetCurrentDisplay(), sync,
+                               EGL_SYNC_FLUSH_COMMANDS_BIT_KHR,
+                               EGL_FOREVER_KHR);
+          eglDestroySyncKHR(eglGetCurrentDisplay(), sync);
+        }
+      }
 
-    frame_callback.reset(wl_surface_frame(surface.get()));
-    wl_callback_add_listener(frame_callback.get(), &frame_listener, &frame);
-    frame.callback_pending = true;
+      buffer->busy = true;
+      pending_frames.push_back(buffer->buffer.get());
 
-    ++frames;
+      ++frames;
+      benchmark_wall_time += base::TimeTicks::Now() - wall_time_start;
+      benchmark_cpu_time += base::ThreadTicks::Now() - cpu_time_start;
+      if (gpu_timer) {
+        gpu_timer->End();
+        gpu_timers.push_back(std::move(gpu_timer));
+      }
+      continue;
+    }
 
-    wl_surface_commit(surface.get());
-  } while (wl_display_dispatch(display_.get()) != -1);
+    if (!frame.callback_pending) {
+      DCHECK_GT(pending_frames.size(), 0u);
+      wl_surface_set_buffer_scale(surface.get(), scale_);
+      wl_surface_attach(surface.get(), pending_frames.front(), 0, 0);
+      pending_frames.pop_front();
+
+      frame_callback.reset(wl_surface_frame(surface.get()));
+      wl_callback_add_listener(frame_callback.get(), &frame_listener, &frame);
+      frame.callback_pending = true;
+      wl_surface_commit(surface.get());
+      wl_display_flush(display_.get());
+      continue;
+    }
+
+    dispatch_status = wl_display_dispatch(display_.get());
+  } while (dispatch_status != -1);
 
   return 0;
 }
@@ -615,8 +701,9 @@ bool MotionEvents::CreateBuffer(Buffer* buffer) {
         zwp_linux_dmabuf_v1_create_params(globals_.linux_dmabuf.get()));
     zwp_linux_buffer_params_v1_add_listener(buffer->params.get(),
                                             &params_listener, buffer);
-    zwp_linux_buffer_params_v1_add(buffer->params.get(), fd.get(), 0, 0,
-                                   stride(), 0, 0);
+    uint32_t stride = gbm_bo_get_stride(buffer->bo.get());
+    zwp_linux_buffer_params_v1_add(buffer->params.get(), fd.get(), 0, 0, stride,
+                                   0, 0);
     zwp_linux_buffer_params_v1_create(buffer->params.get(), width_, height_,
                                       kDrmFormat, 0);
 
@@ -629,7 +716,7 @@ bool MotionEvents::CreateBuffer(Buffer* buffer) {
                                 EGL_LINUX_DRM_FOURCC_EXT,
                                 kDrmFormat,
                                 EGL_DMA_BUF_PLANE0_PITCH_EXT,
-                                stride(),
+                                stride,
                                 EGL_DMA_BUF_PLANE0_OFFSET_EXT,
                                 0,
                                 EGL_NONE};
@@ -663,13 +750,15 @@ bool MotionEvents::CreateBuffer(Buffer* buffer) {
     return true;
   }
 #endif
-  buffer->shared_memory.CreateAndMapAnonymous(buffer_size());
+
+  size_t stride = width_ * kBytesPerPixel;
+  buffer->shared_memory.CreateAndMapAnonymous(stride * height_);
   buffer->shm_pool.reset(
       wl_shm_create_pool(globals_.shm.get(), buffer->shared_memory.handle().fd,
                          buffer->shared_memory.requested_size()));
 
   buffer->buffer.reset(static_cast<wl_buffer*>(wl_shm_pool_create_buffer(
-      buffer->shm_pool.get(), 0, width_, height_, stride(), kShmFormat)));
+      buffer->shm_pool.get(), 0, width_, height_, stride, kShmFormat)));
   if (!buffer->buffer) {
     LOG(ERROR) << "Can't create buffer";
     return false;
@@ -677,7 +766,7 @@ bool MotionEvents::CreateBuffer(Buffer* buffer) {
 
   buffer->sk_surface = SkSurface::MakeRasterDirect(
       SkImageInfo::Make(width_, height_, kColorType, kOpaque_SkAlphaType),
-      static_cast<uint8_t*>(buffer->shared_memory.memory()), stride());
+      static_cast<uint8_t*>(buffer->shared_memory.memory()), stride);
   DCHECK(buffer->sk_surface);
   return true;
 }
@@ -697,12 +786,15 @@ const char kScale[] = "scale";
 // Specifies the number of rotating rects to draw.
 const char kNumRects[] = "num-rects";
 
-// Use drm buffer instead of shared memory.
-const char kUseDrm[] = "use-drm";
+// Specifies the maximum number of pending frames.
+const char kMaxFramesPending[] = "max-frames-pending";
 
 // Specifies if client should be fullscreen.
 const char kFullscreen[] = "fullscreen";
 
+// Use drm buffer instead of shared memory.
+const char kUseDrm[] = "use-drm";
+
 }  // namespace switches
 
 int main(int argc, char* argv[]) {
@@ -736,15 +828,23 @@ int main(int argc, char* argv[]) {
     return 1;
   }
 
+  size_t max_frames_pending = 0;
+  if (command_line->HasSwitch(switches::kMaxFramesPending) &&
+      (!base::StringToSizeT(
+          command_line->GetSwitchValueASCII(switches::kMaxFramesPending),
+          &max_frames_pending))) {
+    LOG(ERROR) << "Invalid value for " << switches::kMaxFramesPending;
+    return 1;
+  }
+
   std::unique_ptr<std::string> use_drm;
   if (command_line->HasSwitch(switches::kUseDrm)) {
     use_drm.reset(
         new std::string(command_line->GetSwitchValueASCII(switches::kUseDrm)));
   }
 
-  bool fullscreen = command_line->HasSwitch(switches::kFullscreen);
-
-  exo::wayland::clients::MotionEvents client(width, height, scale, num_rects,
-                                             use_drm.get(), fullscreen);
+  exo::wayland::clients::MotionEvents client(
+      width, height, scale, num_rects, max_frames_pending,
+      command_line->HasSwitch(switches::kFullscreen), use_drm.get());
   return client.Run();
 }