Change HUD colors to distinguish status easily.

cc/layers/heads_up_display_layer_impl.cc

Index: cc/layers/heads_up_display_layer_impl.cc
diff --git a/cc/layers/heads_up_display_layer_impl.cc b/cc/layers/heads_up_display_layer_impl.cc
index 496452250f61599ac25fbc66a464aca7c7c74fc4..a5b14d15f60334a3d084cded286dfd31e1d23ff3 100644
--- a/cc/layers/heads_up_display_layer_impl.cc
+++ b/cc/layers/heads_up_display_layer_impl.cc
@@ -22,8 +22,10 @@
 #include "skia/ext/platform_canvas.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
+#include "third_party/skia/include/core/SkRRect.h"
 #include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/effects/SkColorMatrixFilter.h"
+#include "third_party/skia/include/effects/SkGradientShader.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/size_conversions.h"
@@ -270,13 +272,21 @@ void HeadsUpDisplayLayerImpl::DrawHudContents(SkCanvas* canvas) {
   if (!debug_state.show_fps_counter)
     return;
 
-  SkRect area =
-      DrawFPSDisplay(canvas, layer_tree_impl()->frame_rate_counter(), 0, 0);
+  SkRect area = DrawFPSDisplay(canvas, layer_tree_impl()->frame_rate_counter(),
+                               0, 0, false);
+  area = DrawDisplaySeparator(canvas, 0, area.bottom(),
+                              SkMaxScalar(area.width(), 150));
+  bool show_memory_display =
+      debug_state.ShowMemoryStats() && !!memory_entry_.total_bytes_used;
   area = DrawGpuRasterizationStatus(canvas, 0, area.bottom(),
-                                    SkMaxScalar(area.width(), 150));
-
-  if (debug_state.ShowMemoryStats())
-    DrawMemoryDisplay(canvas, 0, area.bottom(), SkMaxScalar(area.width(), 150));
+                                    SkMaxScalar(area.width(), 150),
+                                    !show_memory_display);
+  if (show_memory_display) {
+    area = DrawDisplaySeparator(canvas, 0, area.bottom(),
+                                SkMaxScalar(area.width(), 150));
+    DrawMemoryDisplay(canvas, 0, area.bottom(), SkMaxScalar(area.width(), 150),
+                      true);
+  }
 }
 int HeadsUpDisplayLayerImpl::MeasureText(SkPaint* paint,
                                          const std::string& text,
@@ -299,12 +309,10 @@ void HeadsUpDisplayLayerImpl::DrawText(SkCanvas* canvas,
                                        int y) const {
   const bool anti_alias = paint->isAntiAlias();
   paint->setAntiAlias(true);
-
   paint->setTextSize(size);
   paint->setTextAlign(align);
   paint->setTypeface(typeface_.get());
   canvas->drawText(text.c_str(), text.length(), x, y, *paint);
-
   paint->setAntiAlias(anti_alias);
 }
 
@@ -319,9 +327,20 @@ void HeadsUpDisplayLayerImpl::DrawText(SkCanvas* canvas,
 
 void HeadsUpDisplayLayerImpl::DrawGraphBackground(SkCanvas* canvas,
                                                   SkPaint* paint,
-                                                  const SkRect& bounds) const {
+                                                  const SkRect& bounds,
+                                                  bool rounded) const {
   paint->setColor(DebugColors::HUDBackgroundColor());
-  canvas->drawRect(bounds, *paint);
+
+  if (rounded) {

[danakj, 2015/10/08 17:48:09]

I don't think it's really worth adding code to make a rounded border on the
display :/ Less code is better here.

[prashant.n, 2015/10/09 01:01:52]

Hmm. I will remove them.

+    const bool anti_alias = paint->isAntiAlias();
+    paint->setAntiAlias(true);
+    SkRRect rounded_bounds;
+    rounded_bounds.setNinePatch(bounds, 4, 0, 0, 4);
+    canvas->drawRRect(rounded_bounds, *paint);
+    paint->setAntiAlias(anti_alias);
+  } else {
+    canvas->drawRect(bounds, *paint);
+  }
 }
 
 void HeadsUpDisplayLayerImpl::DrawGraphLines(SkCanvas* canvas,
@@ -357,11 +376,13 @@ SkRect HeadsUpDisplayLayerImpl::DrawFPSDisplay(
     SkCanvas* canvas,
     const FrameRateCounter* fps_counter,
     int right,
-    int top) const {
+    int top,
+    bool last_display) const {
   const int kPadding = 4;
   const int kGap = 6;
 
-  const int kFontHeight = 15;
+  const int kTitleFontHeight = 13;
+  const int kFontHeight = 12;
 
   const int kGraphWidth =
       base::saturated_cast<int>(fps_counter->time_stamp_history_size()) - 2;
@@ -370,18 +391,19 @@ SkRect HeadsUpDisplayLayerImpl::DrawFPSDisplay(
   const int kHistogramWidth = 37;
 
   int width = kGraphWidth + kHistogramWidth + 4 * kPadding;
-  int height = kFontHeight + kGraphHeight + 4 * kPadding + 2;
+  int height = kTitleFontHeight + kFontHeight + kGraphHeight + 6 * kPadding + 2;
   int left = bounds().width() - width - right;
   SkRect area = SkRect::MakeXYWH(left, top, width, height);
 
   SkPaint paint = CreatePaint();
-  DrawGraphBackground(canvas, &paint, area);
+  DrawGraphBackground(canvas, &paint, area, last_display);
 
+  SkRect title_bounds = SkRect::MakeXYWH(
+      left + kPadding, top + kPadding, kGraphWidth + kHistogramWidth + kGap + 2,
+      kTitleFontHeight);
   SkRect text_bounds =
-      SkRect::MakeXYWH(left + kPadding,
-                       top + kPadding,
-                       kGraphWidth + kHistogramWidth + kGap + 2,
-                       kFontHeight);
+      SkRect::MakeXYWH(left + kPadding, title_bounds.bottom() + 2 * kPadding,
+                       kGraphWidth + kHistogramWidth + kGap + 2, kFontHeight);
   SkRect graph_bounds = SkRect::MakeXYWH(left + kPadding,
                                          text_bounds.bottom() + 2 * kPadding,
                                          kGraphWidth,
@@ -391,13 +413,18 @@ SkRect HeadsUpDisplayLayerImpl::DrawFPSDisplay(
                                              kHistogramWidth,
                                              kGraphHeight);
 
+  const std::string title("Frame Rate");
   const std::string value_text =
-      base::StringPrintf("FPS:%5.1f", fps_graph_.value);
+      base::StringPrintf("%5.1f fps", fps_graph_.value);
   const std::string min_max_text =
       base::StringPrintf("%.0f-%.0f", fps_graph_.min, fps_graph_.max);
 
   VLOG(1) << value_text;
 
+  paint.setColor(DebugColors::HUDTitleColor());
+  DrawText(canvas, &paint, title, SkPaint::kLeft_Align, kTitleFontHeight,
+           title_bounds.left(), title_bounds.bottom());
+
   paint.setColor(DebugColors::FPSDisplayTextAndGraphColor());
   DrawText(canvas,
            &paint,
@@ -497,36 +524,33 @@ SkRect HeadsUpDisplayLayerImpl::DrawFPSDisplay(
 SkRect HeadsUpDisplayLayerImpl::DrawMemoryDisplay(SkCanvas* canvas,
                                                   int right,
                                                   int top,
-                                                  int width) const {
-  if (!memory_entry_.total_bytes_used)
-    return SkRect::MakeEmpty();
-
+                                                  int width,
+                                                  bool last_display) const {
   const int kPadding = 4;
-  const int kFontHeight = 13;
+  const int kTitleFontHeight = 13;
+  const int kFontHeight = 12;
 
-  const int height = 3 * kFontHeight + 4 * kPadding;
+  const int height = kTitleFontHeight + 2 * kFontHeight + 5 * kPadding;
   const int left = bounds().width() - width - right;
   const SkRect area = SkRect::MakeXYWH(left, top, width, height);
 
   const double kMegabyte = 1024.0 * 1024.0;
 
   SkPaint paint = CreatePaint();
-  DrawGraphBackground(canvas, &paint, area);
+  DrawGraphBackground(canvas, &paint, area, last_display);
 
-  SkPoint title_pos = SkPoint::Make(left + kPadding, top + kFontHeight);
+  SkPoint title_pos =
+      SkPoint::Make(left + kPadding, top + kFontHeight + kPadding);
   SkPoint stat1_pos = SkPoint::Make(left + width - kPadding - 1,
                                     top + kPadding + 2 * kFontHeight);
   SkPoint stat2_pos = SkPoint::Make(left + width - kPadding - 1,
                                     top + 2 * kPadding + 3 * kFontHeight);
 
-  paint.setColor(DebugColors::MemoryDisplayTextColor());
-  DrawText(canvas,
-           &paint,
-           "GPU memory",
-           SkPaint::kLeft_Align,
-           kFontHeight,
+  paint.setColor(DebugColors::HUDTitleColor());
+  DrawText(canvas, &paint, "GPU Memory", SkPaint::kLeft_Align, kTitleFontHeight,
            title_pos);
 
+  paint.setColor(DebugColors::MemoryDisplayTextColor());
   std::string text = base::StringPrintf(
       "%6.1f MB used", memory_entry_.total_bytes_used / kMegabyte);
   DrawText(canvas, &paint, text, SkPaint::kRight_Align, kFontHeight, stat1_pos);
@@ -537,13 +561,52 @@ SkRect HeadsUpDisplayLayerImpl::DrawMemoryDisplay(SkCanvas* canvas,
                             memory_entry_.total_budget_in_bytes / kMegabyte);
   DrawText(canvas, &paint, text, SkPaint::kRight_Align, kFontHeight, stat2_pos);
 
+  // Draw memory graph.
+  int length = 2 * kFontHeight + kPadding + 12;
+  SkRect oval =
+      SkRect::MakeXYWH(left + kPadding * 6,
+                       top + kTitleFontHeight + kPadding * 3, length, length);
+  paint.setAntiAlias(true);
+  paint.setStyle(SkPaint::kFill_Style);
+
+  paint.setColor(SkColorSetARGB(64, 255, 255, 0));
+  canvas->drawArc(oval, 180, 180, true, paint);
+
+  int radius = length / 2;
+  int cx = oval.left() + radius;
+  int cy = oval.top() + radius;
+  double angle = ((double)memory_entry_.total_bytes_used /
+                  memory_entry_.total_budget_in_bytes) *
+                 180;
+
+  SkColor colors[] = {SK_ColorRED, SK_ColorGREEN, SK_ColorGREEN,
+                      SkColorSetARGB(255, 255, 140, 0), SK_ColorRED};
+  const SkScalar pos[] = {0.2, 0.4, 0.6, 0.8, 1.0};
+  skia::RefPtr<SkShader> gradient_shader =
+      skia::AdoptRef(SkGradientShader::CreateSweep(cx, cy, colors, pos, 5));
+  paint.setShader(gradient_shader.get());
+  paint.setFlags(SkPaint::kAntiAlias_Flag);
+
+  // Draw current status.
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setAlpha(32);
+  paint.setStrokeWidth(4);
+  canvas->drawArc(oval, 180, angle, true, paint);
+
+  paint.setStyle(SkPaint::kFill_Style);
+  paint.setColor(SkColorSetARGB(255, 0, 255, 0));
+  canvas->drawArc(oval, 180, angle, true, paint);
+  paint.setShader(NULL);
+
   return area;
 }
 
-SkRect HeadsUpDisplayLayerImpl::DrawGpuRasterizationStatus(SkCanvas* canvas,
-                                                           int right,
-                                                           int top,
-                                                           int width) const {
+SkRect HeadsUpDisplayLayerImpl::DrawGpuRasterizationStatus(
+    SkCanvas* canvas,
+    int right,
+    int top,
+    int width,
+    bool last_display) const {
   std::string status;
   SkColor color = SK_ColorRED;
   switch (layer_tree_impl()->GetGpuRasterizationStatus()) {
@@ -577,27 +640,41 @@ SkRect HeadsUpDisplayLayerImpl::DrawGpuRasterizationStatus(SkCanvas* canvas,
     return SkRect::MakeEmpty();
 
   const int kPadding = 4;
-  const int kFontHeight = 13;
+  const int kTitleFontHeight = 13;
+  const int kFontHeight = 12;
 
-  const int height = 2 * kFontHeight + 3 * kPadding;
+  const int height = kTitleFontHeight + kFontHeight + 3 * kPadding;
   const int left = bounds().width() - width - right;
   const SkRect area = SkRect::MakeXYWH(left, top, width, height);
 
   SkPaint paint = CreatePaint();
-  DrawGraphBackground(canvas, &paint, area);
+  DrawGraphBackground(canvas, &paint, area, last_display);
 
   SkPoint gpu_status_pos = SkPoint::Make(left + width - kPadding,
                                          top + 2 * kFontHeight + 2 * kPadding);
-
-  paint.setColor(color);
-  DrawText(canvas, &paint, "GPU raster: ", SkPaint::kLeft_Align, kFontHeight,
+  paint.setColor(DebugColors::HUDTitleColor());
+  DrawText(canvas, &paint, "GPU Raster", SkPaint::kLeft_Align, kTitleFontHeight,
            left + kPadding, top + kFontHeight + kPadding);
+  paint.setColor(color);
   DrawText(canvas, &paint, status, SkPaint::kRight_Align, kFontHeight,
            gpu_status_pos);
 
   return area;
 }
 
+SkRect HeadsUpDisplayLayerImpl::DrawDisplaySeparator(SkCanvas* canvas,
+                                                     int right,
+                                                     int top,
+                                                     int width) const {
+  const int height = 1;
+  const int left = bounds().width() - width - right;
+  const SkRect area = SkRect::MakeXYWH(left, top, width, height);
+  SkPaint paint = CreatePaint();
+  paint.setColor(DebugColors::HUDDisplaySeparatorColor());

[danakj, 2015/10/08 17:48:09]

Similar comment here, I don't know that these separators are adding that much.
The headers are nice how you did them. These lines seem extra. I'd remove that.

[prashant.n, 2015/10/09 01:01:52]

Adding separators between different displays makes them as different categories
visually, so IMO we should have them.

[danakj, 2015/10/09 04:41:35]

I disagee, as I would like as little code in the HUD as is reasonable, and I
don't think they are adding that much. You can tell what is going on without
them.

[prashant.n, 2015/10/09 05:05:12]

Basically it shows that fps counter and GPU status displays are different. It
separates visually, so it is easier to understand what is contained in each
category of display by having visual boundary.

In the Chrome Menu, we have separators to differentiate the categories.
Similarly I thought to have separators in different displays to follow general
UX practice.

I understand that HUD should be simpler. But if we keep adding more displays on
hud then making boudaries between them will become essential and it makes
focusing on what developer expects easily.

[danakj, 2015/10/09 16:59:57]

This isn't user-targeted UI like the chrome UI is. It's an FPS meter for people
to use, and some other pieces of data for chrome developers more or less.
There's no need to build something extravagant here, it's not trying to be
devtools. The header titles already do a fine job explaining the UI.

[prashant.n, 2015/10/10 00:29:35]

IMO, even the ui is not user targetted, it is published and available using
chrome://flags, so that it can treated as important as devtools. On top of
chrome UI, it looks like different UI patched. It looks nice visually and
costwise it just adds simple rect with different color. If you feel it's
overhead, I'll remove it.

+  canvas->drawRect(area, paint);
+  return area;
+}
+
 void HeadsUpDisplayLayerImpl::DrawDebugRect(
     SkCanvas* canvas,
     SkPaint* paint,