Update LocationBarView Background

ui/views/background.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/background.h"
 
 #include "base/logging.h"
 #include "skia/ext/skia_utils_win.h"
 #include "third_party/skia/include/core/SkPaint.h"
+#include "third_party/skia/include/core/SkPath.h"
+#include "third_party/skia/include/pathops/SkPathOps.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/views/painter.h"
 #include "ui/views/view.h"
 
 namespace views {
 
 // SolidBackground is a trivial Background implementation that fills the
 // background in a solid color.
 class SolidBackground : public Background {
  public:
   explicit SolidBackground(SkColor color) {
     SetNativeControlColor(color);
   }
 
   void Paint(gfx::Canvas* canvas, View* view) const override {
     // Fill the background. Note that we don't constrain to the bounds as
     // canvas is already clipped for us.
     canvas->DrawColor(get_color());
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SolidBackground);
 };
 
+// SolidScaledBackground renders a solid background color, with a one pixel
+// border. This accounts for the scaling of the canvas, so that the border is
+// one pixel regardless of display scaling. Optionally the border can have
+// rounded corners.
+class SolidScaledBackground : public Background {
+ public:
+  explicit SolidScaledBackground(SkColor background, SkColor border, bool round)

[Peter Kasting, 2015/10/19 21:09:44]

Nit: No explicit (only used for 1-arg constructors)

I would name the third arg (and member) "round_corners" or "round_rect" rather
than "round", as "round" sounds like a mathematical operation.

[jonross, 2015/10/20 18:54:53]

Done.

+      : border_color_(border), round_(round) {
+    SetNativeControlColor(background);
+  }
+
+  void Paint(gfx::Canvas* canvas, View* view) const override {
+    gfx::RectF border_rect_f(view->GetContentsBounds());

[Peter Kasting, 2015/10/19 21:09:44]

Nit: Declare |border_rect_f| and |kOffset| right above the conditional that uses
them.

[jonross, 2015/10/20 18:54:54]

Done.

+    const float scale = canvas->SaveAndUnscale();
+    const float kOffset = scale > 1.0f ? 1.5f : 0.5f;

[Peter Kasting, 2015/10/19 21:09:44]

I don't think this gets the right result for scales other than 1x and 2x, e.g.
1.5x.  My understanding from email was that the problem is that Sebastien wants
the omnibox sized based on scaling the interior up and outdenting the border,
rather than based on scaling the outer size up and indenting.  To implement that
you'd need to do one of the following:

(1) Inset the |border_rect_f| by 1 px on each side before scaling, then scale,
then outset by 0.5 px.
(2) Do something like what you're doing here, but compute |kOffset| as "scale -
0.5f" (and name it |inset| or something since it's not a pure constant).

[jonross, 2015/10/20 18:54:54]

I spoke with him, he wants the 200% inset by one pixel. So that the outermost
pixel of the view is transparent.

[Peter Kasting, 2015/10/20 23:04:05]

Yes, and my suggestions should accomplish that, but they will also work in modes
like 140% and 150%, whereas this code will break badly outside 1x and 2x.

[jonross, 2015/10/21 14:52:55]

Done.

+    border_rect_f.Scale(scale);
+
+    SkPaint stroke_paint;
+    stroke_paint.setStyle(SkPaint::Style::kStroke_Style);

[Peter Kasting, 2015/10/19 21:09:44]

Nit: No need for "Style::"

[jonross, 2015/10/20 18:54:53]

Done.

+    stroke_paint.setStrokeWidth(1);
+    stroke_paint.setColor(border_color_);
+    stroke_paint.setAntiAlias(true);
+
+    SkPaint fill_paint;
+    fill_paint.setStyle(SkPaint::kFill_Style);
+    fill_paint.setColor(get_color());
+    fill_paint.setAntiAlias(true);
+
+    SkPath path;
+    if (round_) {
+      border_rect_f.Inset(kOffset, kOffset);
+      const SkScalar kCornerRadius = SkDoubleToScalar(2.5f * scale);
+      path.addRoundRect(gfx::RectFToSkRect(border_rect_f), kCornerRadius,
+                        kCornerRadius);
+    } else {
+      border_rect_f.Inset(-kOffset, kOffset);

[Peter Kasting, 2015/10/19 21:09:44]

I don't think this is right for non-1x.  It seems like in this case you want the
x inset to always be -0.5, I think.  Probably the two will look equivalent
assuming the rect goes to the window bounds, but that would be technically more
correct.

[jonross, 2015/10/20 18:54:54]

Done.

+      path.addRect(gfx::RectFToSkRect(border_rect_f));
+    }
+
+    SkPath stroke_path;
+    stroke_paint.getFillPath(path, &stroke_path);
+
+    SkPath fill_path;
+    Op(path, stroke_path, kDifference_SkPathOp, &fill_path);
+    canvas->sk_canvas()->drawPath(fill_path, fill_paint);
+    canvas->sk_canvas()->drawPath(path, stroke_paint);

[Peter Kasting, 2015/10/19 21:09:44]

Nit: I think it might be slightly more performant to do

  fill_paint.setColor(border_color_);
  canvas->sk_canvas()->drawPath(stroke_path, fill_paint);

This allows you to further simplify things to just use one paint:

  SkPaint paint;
  paint.setAntiAlias(true);
  paint.setStyle(SkPaint::kStroke_Style);
  paint.setStrokeWidth(1);
  SkPath stroke_path;
  paint.getFillPath(path, &stroke_path);

  SkPath fill_path;
  Op(path, stroke_path, kDifference_SkPathOp, &fill_path);
  paint.setStyle(SkPaint::kFill_style);
  paint.setColor(get_color());
  canvas->sk_canvas()->drawPath(fill_path, paint);

  paint.setColor(border_color_);
  canvas->sk_canvas()->drawPath(stroke_path, paint);

[jonross, 2015/10/20 18:54:53]

Done.

+    canvas->Restore();
+  }
+
+ private:
+  // Color for the one pixel border.
+  SkColor border_color_;
+
+  // If true the rect will be a rounded rect.

[Peter Kasting, 2015/10/19 21:09:44]

Nit: Maybe "If true the border and interior corners will be rounded."

[Evan Stade, 2015/10/19 23:21:30]

I don't really care what this comment says because it's clear to me either way
but I think it's usually "roundrect" not "rounded rect".

[jonross, 2015/10/20 18:54:54]

Done.

+  bool round_;
+
+  DISALLOW_COPY_AND_ASSIGN(SolidScaledBackground);
+};
+
 class BackgroundPainter : public Background {
  public:
   BackgroundPainter(bool owns_painter, Painter* painter)
       : owns_painter_(owns_painter), painter_(painter) {
     DCHECK(painter);
   }
 
   ~BackgroundPainter() override {
     if (owns_painter_)
       delete painter_;
@@ skipping 38 matching lines @@
     native_control_brush_ = CreateSolidBrush(skia::SkColorToCOLORREF(color_));
   return native_control_brush_;
 }
 #endif
 
 // static
 Background* Background::CreateSolidBackground(SkColor color) {
   return new SolidBackground(color);
 }
 
+Background* Background::CreateSolidScaledBackground(SkColor background,
+                                                    SkColor border,
+                                                    bool round) {
+  return new SolidScaledBackground(background, border, round);
+}
+
 // static
 Background* Background::CreateStandardPanelBackground() {
   // TODO(beng): Should be in NativeTheme.
   return CreateSolidBackground(SK_ColorWHITE);
 }
 
 // static
 Background* Background::CreateVerticalGradientBackground(SkColor color1,
                                                          SkColor color2) {
   Background* background = CreateBackgroundPainter(
@@ skipping 17 matching lines @@
   return background;
 }
 
 // static
 Background* Background::CreateBackgroundPainter(bool owns_painter,
                                                 Painter* painter) {
   return new BackgroundPainter(owns_painter, painter);
 }
 
 }  // namespace views