Change status bubble rendering at hidpi and when there is no client edge

chrome/browser/ui/views/status_bubble_views.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 "chrome/browser/ui/views/status_bubble_views.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/i18n/rtl.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "components/url_formatter/elide_url.h"
 #include "components/url_formatter/url_formatter.h"
 #include "third_party/skia/include/core/SkPaint.h"
-#include "third_party/skia/include/core/SkRRect.h"
+#include "third_party/skia/include/core/SkPath.h"
+#include "third_party/skia/include/pathops/SkPathOps.h"
 #include "ui/base/theme_provider.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/animation/animation_delegate.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/skia_util.h"
@@ skipping 95 matching lines @@
   };
 
   enum BubbleStyle {
     STYLE_BOTTOM,
     STYLE_FLOATING,
     STYLE_STANDARD,
     STYLE_STANDARD_RIGHT
   };
 
   StatusView(views::Widget* popup,
-             const ui::ThemeProvider* theme_provider);
+             SkColor bubble_color,
+             SkColor bubble_text_color,
+             bool has_client_edge);
   ~StatusView() override;
 
   // Set the bubble text to a certain value, hides the bubble if text is
   // an empty string.  Trigger animation sequence to display if
   // |should_animate_open|.
   void SetText(const base::string16& text, bool should_animate_open);
 
   BubbleState state() const { return state_; }
   BubbleStyle style() const { return style_; }
   void SetStyle(BubbleStyle style);
@@ skipping 37 matching lines @@
   BubbleStyle style_;
 
   std::unique_ptr<StatusViewAnimation> animation_;
 
   // Handle to the widget that contains us.
   views::Widget* popup_;
 
   // The currently-displayed text.
   base::string16 text_;
 
-  // Holds the theme provider of the frame that created us.
-  const ui::ThemeProvider* theme_provider_;
+  const SkColor bubble_color_;
+  const SkColor bubble_text_color_;
+  const bool has_client_edge_;
 
   base::WeakPtrFactory<StatusBubbleViews::StatusView> timer_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(StatusView);
 };
 
-StatusBubbleViews::StatusView::StatusView(
-    views::Widget* popup,
-    const ui::ThemeProvider* theme_provider)
+StatusBubbleViews::StatusView::StatusView(views::Widget* popup,
+                                          SkColor bubble_color,
+                                          SkColor bubble_text_color,
+                                          bool has_client_edge)
     : state_(BUBBLE_HIDDEN),
       style_(STYLE_STANDARD),
       animation_(new StatusViewAnimation(this, 0, 0)),
       popup_(popup),
-      theme_provider_(theme_provider),
+      bubble_color_(bubble_color),
+      bubble_text_color_(bubble_text_color),
+      has_client_edge_(has_client_edge),
       timer_factory_(this) {}
 
 StatusBubbleViews::StatusView::~StatusView() {
   animation_->Stop();
   CancelTimer();
 }
 
 void StatusBubbleViews::StatusView::SetText(const base::string16& text,
                                             bool should_animate_open) {
   if (text.empty()) {
@@ skipping 137 matching lines @@
   } else if (state_ == BUBBLE_SHOWING_FADE) {
     state_ = BUBBLE_SHOWN;
   }
 }
 
 const char* StatusBubbleViews::StatusView::GetClassName() const {
   return "StatusBubbleViews::StatusView";
 }
 
 void StatusBubbleViews::StatusView::OnPaint(gfx::Canvas* canvas) {
-  SkPaint paint;
-  paint.setStyle(SkPaint::kFill_Style);
-  paint.setAntiAlias(true);
-  SkColor toolbar_color = theme_provider_->GetColor(
-      ThemeProperties::COLOR_TOOLBAR);
-  paint.setColor(toolbar_color);
+  gfx::Rect popup_bounds = popup_->GetWindowBoundsInScreen();
 
-  gfx::Rect popup_bounds = popup_->GetWindowBoundsInScreen();
+  canvas->Save();
+  float scale = canvas->UndoDeviceScaleFactor();
+  const float radius = kBubbleCornerRadius * scale;
 
   SkScalar rad[8] = {};
 
   // Top Edges - if the bubble is in its bottom position (sticking downwards),
   // then we square the top edges. Otherwise, we square the edges based on the
   // position of the bubble within the window (the bubble is positioned in the
   // southeast corner in RTL and in the southwest corner in LTR).
   if (style_ != STYLE_BOTTOM) {
     if (base::i18n::IsRTL() != (style_ == STYLE_STANDARD_RIGHT)) {
       // The text is RtL or the bubble is on the right side (but not both).
 
       // Top Left corner.
-      rad[0] = kBubbleCornerRadius;
-      rad[1] = kBubbleCornerRadius;
+      rad[0] = radius;
+      rad[1] = radius;
     } else {
       // Top Right corner.
-      rad[2] = kBubbleCornerRadius;
-      rad[3] = kBubbleCornerRadius;
+      rad[2] = radius;
+      rad[3] = radius;
     }
   }
 
   // Bottom edges - Keep these squared off if the bubble is in its standard
   // position (sticking upward).
   if (style_ != STYLE_STANDARD && style_ != STYLE_STANDARD_RIGHT) {
     // Bottom Right Corner.
-    rad[4] = kBubbleCornerRadius;
-    rad[5] = kBubbleCornerRadius;
+    rad[4] = radius;
+    rad[5] = radius;
 
     // Bottom Left Corner.
-    rad[6] = kBubbleCornerRadius;
-    rad[7] = kBubbleCornerRadius;
+    rad[6] = radius;
+    rad[7] = radius;
   }
 
-  // Draw the bubble's shadow.
-  int width = popup_bounds.width();
-  int height = popup_bounds.height();
-  gfx::Rect rect(gfx::Rect(popup_bounds.size()));
-  SkPaint shadow_paint;
-  shadow_paint.setAntiAlias(true);
-  shadow_paint.setColor(kShadowColor);
+  const float width = popup_bounds.width() * scale;
+  const float height = popup_bounds.height() * scale;
 
-  SkRRect rrect;
-  rrect.setRectRadii(RectToSkRect(rect), (const SkVector*)rad);
-  canvas->sk_canvas()->drawRRect(rrect, shadow_paint);
+  if (!has_client_edge_) {
+    // Clip out the edges when the bubble is docked so that it doesn't draw over
+    // the window border, since there's no client edge. But we do want the
+    // border when the bubble is floating so it looks complete.
+    const int clip_left = style_ == STYLE_STANDARD ? 1 : 0;
+    const bool clip_right = style_ == STYLE_STANDARD_RIGHT ? 1 : 0;
+    const bool clip_bottom = clip_left || clip_right ? 1 : 0;
+    gfx::Rect clip_rect(clip_left, 0, width - clip_right, height - clip_bottom);
+    canvas->ClipRect(clip_rect);
+  }
 
-  const int shadow_size = 2 * kShadowThickness;
-  // Draw the bubble.
-  rect.SetRect(SkIntToScalar(kShadowThickness), SkIntToScalar(kShadowThickness),
-               SkIntToScalar(width - shadow_size),
-               SkIntToScalar(height - shadow_size));
-  rrect.setRectRadii(RectToSkRect(rect), (const SkVector*)rad);
-  canvas->sk_canvas()->drawRRect(rrect, paint);
+  // At non-integral device scale factors the size we get from
+  // GetWindowBoundsInScreen is slightly too high and the edge of our rectangle
+  // will get clipped off. The error never seems to be more than 3 pixels.
+  const int scale_error_adjustment = scale - std::floor(scale) > 0 ? 3 : 0;

[Peter Kasting, 2016/08/20 00:42:42]

Wat.

3 px seems entirely magic.  I would expect that number to be governed by the
scale factor you're at, not clamped to 3.

And what is actually clipping you here?  Why is this not a problem for the old
code as well?

[Bret, 2016/08/24 22:04:47]

It is a problem for the old code, I just got a bit frustrated with it before.

I investigated this more thoroughly and part of the problem was that we weren't
snapping the width and height to pixels, so sometimes the border line would be
aliased and it looks similar to clipping (I fixed that). But the real problem
was that GetWindowBoundsInScreen is using ScreenWin::DIPToScreenRect which uses
ScaleToEnclosingRect to coerce to integers, so it's going to be an overestimate
on the width most of the time. That means if we draw to that width we'll go
outside the layer bounds and get clipped. So I changed the plumbing to just use
the original size_ in this file (it wasn't as problematic as I thought earlier).

+  gfx::RectF bubble_rect(width - scale_error_adjustment, height);
+  bubble_rect.Inset(0.5, 0.5);

[Peter Kasting, 2016/08/20 00:42:42]

You're still overlapping by 1 DIP in Reposition() and then painting in the
outermost px here.  This is still going to result in incorrect positioning at
DSF > 1.  You need to point in the innermost px of that outer DIP, not the
outermost.

Something akin to:

  // The left and bottom edges are outdented by kShadowThickness DIPs in
  // Reposition().  We want to paint on the inner edge of that outdent.  We
floor
  // the scaled outdent to snap the border coordinates to the pixel grid.
  float aligned_scale = std::floor(kShadowThickness * scale);
  bubble_rect.Inset(aligned_scale - 0.5, 0.5, 0.5, aligned_scale - 0.5);

[Bret, 2016/08/24 22:04:47]

Ok I misunderstood what you meant before. The relative position is annoyingly
dependent on the absolute position so it's not really possible to be pixel
perfect without fixing that. I added a big comment, hopefully it's clear enough.

+
+  SkPath path;
+  path.addRoundRect(gfx::RectFToSkRect(bubble_rect), rad);
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setStrokeWidth(1);
+  paint.setAntiAlias(true);
+
+  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(bubble_color_);
+  canvas->sk_canvas()->drawPath(fill_path, paint);
+
+  paint.setColor(kShadowColor);
+  canvas->sk_canvas()->drawPath(stroke_path, paint);
+
+  canvas->Restore();
 
   // Compute text bounds.
-  const gfx::FontList font_list;
-  int text_width =
-      std::min(gfx::GetStringWidth(text_, font_list),
-               width - shadow_size - kTextPositionX - kTextHorizPadding);
-  int text_height = height - shadow_size;
-  gfx::Rect text_bounds(kShadowThickness + kTextPositionX,
-                        kShadowThickness,
-                        std::max(0, text_width),
-                        std::max(0, text_height));
+  gfx::Rect text_rect(popup_bounds.width() - kTextHorizPadding -
+                          std::ceil(scale_error_adjustment / scale),
+                      popup_bounds.height());
+  text_rect.Inset(kShadowThickness, kShadowThickness);
+  text_rect.Inset(kTextPositionX, 0);
   // Make sure the text is aligned to the right on RTL UIs.
-  text_bounds.set_x(GetMirroredXForRect(text_bounds));
+  text_rect.set_x(GetMirroredXForRect(text_rect));
 
   // Text color is the foreground tab text color at 60% alpha.
-  SkColor text_color = color_utils::AlphaBlend(
-      theme_provider_->GetColor(ThemeProperties::COLOR_TAB_TEXT), toolbar_color,
-      0x99);
+  SkColor blended_text_color =
+      color_utils::AlphaBlend(bubble_text_color_, bubble_color_, 0x99);
   canvas->DrawStringRect(
-      text_, font_list,
-      color_utils::GetReadableColor(text_color, toolbar_color), text_bounds);
+      text_, gfx::FontList(),
+      color_utils::GetReadableColor(blended_text_color, bubble_color_),
+      text_rect);
 }
 
 
 // StatusBubbleViews::StatusViewAnimation --------------------------------------
 
 StatusBubbleViews::StatusViewAnimation::StatusViewAnimation(
     StatusView* status_view,
     float opacity_start,
     float opacity_end)
     : gfx::LinearAnimation(kFramerate, this),
@@ skipping 102 matching lines @@
 void StatusBubbleViews::StatusViewExpander::SetBubbleWidth(int width) {
   status_bubble_->SetBubbleWidth(width);
   status_view_->SchedulePaint();
 }
 
 
 // StatusBubbleViews -----------------------------------------------------------
 
 const int StatusBubbleViews::kShadowThickness = 1;
 
-StatusBubbleViews::StatusBubbleViews(views::View* base_view)
+StatusBubbleViews::StatusBubbleViews(views::View* base_view,
+                                     bool has_client_edge)
     : contains_mouse_(false),
       offset_(0),
+      has_client_edge_(has_client_edge),
       base_view_(base_view),
       view_(NULL),
       download_shelf_is_visible_(false),
       is_expanded_(false),
       expand_timer_factory_(this) {
   expand_view_.reset();
 }
 
 StatusBubbleViews::~StatusBubbleViews() {
   CancelExpandTimer();
   if (popup_.get())
     popup_->CloseNow();
 }
 
 void StatusBubbleViews::Init() {
   if (!popup_.get()) {
     popup_.reset(new views::Widget);
     views::Widget* frame = base_view_->GetWidget();
-    if (!view_)
-      view_ = new StatusView(popup_.get(), frame->GetThemeProvider());
+    if (!view_) {
+      const ui::ThemeProvider* theme_provider = frame->GetThemeProvider();
+      view_ = new StatusView(
+          popup_.get(),
+          theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR),
+          theme_provider->GetColor(ThemeProperties::COLOR_TAB_TEXT),
+          has_client_edge_);
+    }
     if (!expand_view_.get())
       expand_view_.reset(new StatusViewExpander(this, view_));
 
     views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
 #if defined(OS_WIN)
     // On Windows use the software compositor to ensure that we don't block
     // the UI thread blocking issue during command buffer creation. We can
     // revert this change once http://crbug.com/125248 is fixed.
     params.force_software_compositing = true;
 #endif
@@ skipping 14 matching lines @@
     popup_->SetContentsView(view_);
 #if defined(USE_ASH)
     ash::wm::GetWindowState(popup_->GetNativeWindow())->
         set_ignored_by_shelf(true);
 #endif
     RepositionPopup();
   }
 }
 
 void StatusBubbleViews::Reposition() {
-  // In restored mode, the client area has a client edge between it and the
-  // frame.
+  // Overlap the client edge that's shown in restored mode, or when there is no
+  // client edge this makes the bubble snug with the corner of the window.
   int overlap = kShadowThickness;
   int height = GetPreferredSize().height();
   int base_view_height = base_view()->bounds().height();
   gfx::Point origin(-overlap, base_view_height - height + overlap);
   SetBounds(origin.x(), origin.y(), base_view()->bounds().width() / 3, height);
 }
 
 void StatusBubbleViews::RepositionPopup() {
   if (popup_.get()) {
     gfx::Point top_left;
@@ skipping 224 matching lines @@
 
 bool StatusBubbleViews::IsFrameMaximized() {
   views::Widget* frame = base_view_->GetWidget();
   views::Widget* window = frame->GetTopLevelWidget();
   return window && window->IsMaximized();
 }
 
 void StatusBubbleViews::ExpandBubble() {
   // Elide URL to maximum possible size, then check actual length (it may
   // still be too long to fit) before expanding bubble.
-  gfx::Rect popup_bounds = popup_->GetWindowBoundsInScreen();
   int max_status_bubble_width = GetMaxStatusBubbleWidth();
   const gfx::FontList font_list;
   url_text_ = url_formatter::ElideUrl(url_, font_list, max_status_bubble_width);
   int expanded_bubble_width =
       std::max(GetStandardStatusBubbleWidth(),
                std::min(gfx::GetStringWidth(url_text_, font_list) +
                             (kShadowThickness * 2) + kTextPositionX +
                             kTextHorizPadding + 1,
                         max_status_bubble_width));
   is_expanded_ = true;
+  gfx::Rect popup_bounds = popup_->GetWindowBoundsInScreen();
   expand_view_->StartExpansion(url_text_, popup_bounds.width(),
                                expanded_bubble_width);
 }
 
 int StatusBubbleViews::GetStandardStatusBubbleWidth() {
   return base_view_->bounds().width() / 3;
 }
 
 int StatusBubbleViews::GetMaxStatusBubbleWidth() {
   const ui::NativeTheme* theme = base_view_->GetNativeTheme();
   return static_cast<int>(std::max(0, base_view_->bounds().width() -
       (kShadowThickness * 2) - kTextPositionX - kTextHorizPadding - 1 -
       views::NativeScrollBar::GetVerticalScrollBarWidth(theme)));
 }
 
 void StatusBubbleViews::SetBubbleWidth(int width) {
   size_.set_width(width);
   SetBounds(original_position_.x(), original_position_.y(),
             size_.width(), size_.height());
 }
 
 void StatusBubbleViews::CancelExpandTimer() {
   if (expand_timer_factory_.HasWeakPtrs())
     expand_timer_factory_.InvalidateWeakPtrs();
 }