exo: Add FPS counter 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 6d913b4ae4e73f0e37166c904fccd34c7bd4f438..822667902d5a418c61c4a57b2558452b3d915c35 100644
--- a/components/exo/wayland/clients/motion_events.cc
+++ b/components/exo/wayland/clients/motion_events.cc
@@ -303,6 +303,7 @@ class MotionEvents {
                size_t num_rects,
                size_t max_frames_pending,
                bool fullscreen,
+               bool show_fps_counter,
                const std::string* use_drm)
       : width_(width),
         height_(height),
@@ -310,6 +311,7 @@ class MotionEvents {
         num_rects_(num_rects),
         max_frames_pending_(max_frames_pending),
         fullscreen_(fullscreen),
+        show_fps_counter_(show_fps_counter),
         use_drm_(use_drm) {}
 
   // Initialize and run client main loop.
@@ -324,6 +326,7 @@ class MotionEvents {
   const size_t num_rects_;
   const size_t max_frames_pending_;
   const bool fullscreen_;
+  const bool show_fps_counter_;
   const std::string* use_drm_;
 
   Globals globals_;
@@ -531,6 +534,12 @@ int MotionEvents::Run() {
       base::TimeDelta::FromSeconds(kBenchmarkInterval);
   base::TimeDelta benchmark_wall_time;
   base::TimeDelta benchmark_cpu_time;
+  std::string fps_counter_text;

[Daniele Castagna, 2016/11/14 03:04:31]

Should it be initialized to something like "fps: N/A" in case show_fps_counter_
is true?

[reveman, 2016/11/14 15:00:49]

I'd like to have the ability to run this without drawing any text for benchmark
purposes.

[Daniele Castagna, 2016/11/14 15:17:03]

Sure, but what I was suggesting is to have the N/A string in case you want the
text (show_fps_counter_ is true), and it's the first benchmark period.

[reveman, 2016/11/14 16:52:35]

Done. But "??" as I want to keep text layout simple.

+
+  SkPaint text_paint;
+  text_paint.setTextSize(32.0f);
+  text_paint.setColor(SK_ColorWHITE);

[Daniele Castagna, 2016/11/14 03:04:32]

It'd be much more readable if it had a black border.

[reveman, 2016/11/14 15:00:49]

The background is typically black and that makes this easy to read. I'd also
like to keep this as simple as possible and I think this is good enough.

[Daniele Castagna, 2016/11/14 15:17:03]

Acknowledged.

+  text_paint.setStyle(SkPaint::kFill_Style);
 
   int dispatch_status = 0;
   do {
@@ -558,6 +567,11 @@ int MotionEvents::Run() {
                   << " cpu=" << benchmark_cpu_time.InMillisecondsF() / frames
                   << ")" << std::endl;
 
+        if (show_fps_counter_) {
+          fps_counter_text =
+              base::UintToString(frames / benchmark_interval.InSeconds());

[Daniele Castagna, 2016/11/14 03:04:32]

Why not a floating point here?
Should it also include something like "fps: " before the actual number?

[reveman, 2016/11/14 15:00:49]

Just printing 2 digits makes it easy from a layout perspective and I think just
the number is clean and good enough.

[Daniele Castagna, 2016/11/14 15:17:03]

Should you round the number instead of getting floor(fps) then?

[reveman, 2016/11/14 16:52:35]

Done.

+        }
+
         frames = 0;
         benchmark_time = wall_time_start;
         benchmark_wall_time = base::TimeDelta();
@@ -567,8 +581,6 @@ int MotionEvents::Run() {
       base::ThreadTicks cpu_time_start = base::ThreadTicks::Now();
 
       SkCanvas* canvas = buffer->sk_surface->getCanvas();
-      canvas->save();
-
       if (event_times.empty()) {
         canvas->clear(SK_ColorBLACK);
       } else {
@@ -584,6 +596,8 @@ int MotionEvents::Run() {
                                        (event_times.back() & 0x00ff00) >> 8,
                                        (event_times.back() & 0xff0000) >> 16));
           canvas->drawIRect(rect, paint);
+          std::string text = base::UintToString(event_times.back());

[Daniele Castagna, 2016/11/14 03:04:32]

Should we have something like "event ts: " before the timestamp?

[reveman, 2016/11/14 15:00:49]

Like to keep this consistent with above and just print the number. The user
needs to look at the code to understand how what this time stamp is anyhow.

+          canvas->drawText(text.c_str(), text.length(), 8, y + 32, text_paint);
           event_times.pop_back();
           y += h;
         }
@@ -596,18 +610,25 @@ int MotionEvents::Run() {
                                        -SkScalarHalf(half_height), half_width,
                                        half_height);
       SkScalar rotation = SkScalarMulDiv(frame.time, kRotationSpeed, 1000);
-      SkPaint paint;
+      canvas->save();
       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};
+        SkPaint paint;
         paint.setColor(SkColorSetA(kColors[i % arraysize(kColors)], 0xA0));
         canvas->rotate(rotation / num_rects_);
         canvas->drawIRect(rect, paint);
       }
-
       canvas->restore();
+
+      // Draw FPS counter.
+      if (fps_counter_text.length()) {
+        canvas->drawText(fps_counter_text.c_str(), fps_counter_text.length(),
+                         width_ - 48, 32, text_paint);
+      }
+
       if (gr_context_) {
         gr_context_->flush();
         glFlush();
@@ -760,6 +781,9 @@ const char kMaxFramesPending[] = "max-frames-pending";
 // Specifies if client should be fullscreen.
 const char kFullscreen[] = "fullscreen";
 
+// Specifies if FPS counter should be shown.
+const char kShowFpsCounter[] = "show-fps-counter";
+
 // Use drm buffer instead of shared memory.
 const char kUseDrm[] = "use-drm";
 
@@ -813,6 +837,7 @@ int main(int argc, char* argv[]) {
 
   exo::wayland::clients::MotionEvents client(
       width, height, scale, num_rects, max_frames_pending,
-      command_line->HasSwitch(switches::kFullscreen), use_drm.get());
+      command_line->HasSwitch(switches::kFullscreen),
+      command_line->HasSwitch(switches::kShowFpsCounter), use_drm.get());

[Daniele Castagna, 2016/11/14 03:04:32]

Why would you *not* want to see the fps? And if there is any reason for that,
wouldn't it be better to have it on by default and have a flag to disable it?

[reveman, 2016/11/14 15:00:49]

I'd like to use this client as a benchmark that would produce the maximum number
of bytes that can be transferred to screen per second without having any text
rendering influence the results.

[Daniele Castagna, 2016/11/14 15:17:03]

Then you might want to be able not to print the event timestamps too.

[reveman, 2016/11/14 16:52:35]

I think this is OK as these will only be printed as a result of motion events
and that's not going to happen in the benchmark case I'm thinking of.

   return client.Run();
 }