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);
+             const gfx::Size& popup_size_,
+             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);
 
   // Show the bubble instantly.
   void Show();
 
   // Hide the bubble instantly.
   void Hide();
 
   // Resets any timers we have. Typically called when the user moves a
   // mouse.
   void ResetTimer();
 
   // This call backs the StatusView in order to fade the bubble in and out.
   void SetOpacity(float opacity);
 
   // Depending on the state of the bubble this will either hide the popup or
   // not.
   void OnAnimationEnded();
 
+  void SetWidth(int new_width);
+
  private:
   class InitialTimer;
 
   // Manage the timers that control the delay before a fade begins or ends.
   void StartTimer(base::TimeDelta time);
   void OnTimer();
   void CancelTimer();
   void RestartTimer(base::TimeDelta delay);
 
   // Manage the fades and starting and stopping the animations correctly.
   void StartFade(float start, float end, int duration);
   void StartHiding();
   void StartShowing();
 
   // views::View:
   const char* GetClassName() const override;
   void OnPaint(gfx::Canvas* canvas) override;
 
   BubbleState state_;
   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_;
+  gfx::Size popup_size_;
+
+  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,
+                                          const gfx::Size& popup_size,
+                                          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),
+      popup_size_(popup_size),
+      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 132 matching lines @@
 
 void StatusBubbleViews::StatusView::OnAnimationEnded() {
   if (state_ == BUBBLE_HIDING_FADE) {
     state_ = BUBBLE_HIDDEN;
     popup_->Hide();
   } else if (state_ == BUBBLE_SHOWING_FADE) {
     state_ = BUBBLE_SHOWN;
   }
 }
 
+void StatusBubbleViews::StatusView::SetWidth(int new_width) {
+  popup_size_.set_width(new_width);
+}
+
 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();
+  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);
+  // Snap to pixels to avoid shadow blurriness.
+  const int width = std::round(popup_size_.width() * scale);
+  const int height = std::round(popup_size_.height() * scale);
 
-  SkRRect rrect;
-  rrect.setRectRadii(RectToSkRect(rect), (const SkVector*)rad);
-  canvas->sk_canvas()->drawRRect(rrect, shadow_paint);
+  // We overlap the client edge or window frame by kShadowThickness dips above
+  // in Reposition(). In order to draw on the innermost pixel we inset the
+  // bubble a bit. This avoids the bubble drawing further and further outside
+  // the window as the scale increases.

[Peter Kasting, 2016/08/27 04:07:31]

Nit: I think if you move this down lower it can be put right on the block it
actually describes.  See lower comment for suggestion.

[Bret, 2016/08/29 21:57:42]

Done.

+  const int subdip_layout_inset = std::floor(kShadowThickness * scale);

[Peter Kasting, 2016/08/27 04:07:31]

Nit: I would call this |shadow_thickness_px| at this point, since that's
basically how it's computed, and I think that reads more clearly below as well.

I assume in testing 1.5x you found that the floor() call was indeed necessary to
snap our drawing to the pixel grid.  If so I'd leave a comment like "The floor()
call here ensures we stay aligned to the pixel grid at non-integral scale
factors."  You might also want to explain why floor(), and not round() or
ceil(), is correct.

[Bret, 2016/08/29 21:57:42]

Done.

 
-  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);
+  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.

[Peter Kasting, 2016/08/27 04:07:31]

Nit: Maybe this would be slightly less awkward?

    // When there's no client edge, parts of the shadow can stick outside the
    // window.  Clip these out when the bubble is docked.  When the bubble is
    // floating, we preserve the full shadow so the bubble looks complete.

[Bret, 2016/08/29 21:57:42]

Done; edited the suggestion.

+    const int clip_left = style_ == STYLE_STANDARD ? subdip_layout_inset : 0;
+    const int clip_right =
+        style_ == STYLE_STANDARD_RIGHT ? subdip_layout_inset : 0;
+    const int clip_bottom = clip_left || clip_right ? subdip_layout_inset : 0;
+    gfx::Rect clip_rect(clip_left, 0, width - clip_right, height - clip_bottom);
+    canvas->ClipRect(clip_rect);
+  }
+
+  gfx::RectF bubble_rect(width, height);
+  const int inset_left =
+      style_ != STYLE_STANDARD_RIGHT ? subdip_layout_inset : 1;

[Peter Kasting, 2016/08/27 04:07:31]

Nit: Prefer "style == STANDARD_RIGHT ? 1 : var" so the "else" portion of the
expression doesn't mentally read like a double-negative.

[Bret, 2016/08/29 21:57:42]

Done.

+  const int inset_right =
+      style_ == STYLE_STANDARD_RIGHT ? subdip_layout_inset : 1;

[Peter Kasting, 2016/08/27 04:07:31]

Nit: I was confused for a bit why we were insetting by 1 in these expressions,
then I realized it was to counteract the "- 0.5" below.

Might be clearer to do something like this.  This includes a possible rephrase
of the comment moved down from above.

12345678901234567890123456789012345678901234567890123456789012345678901234567890
  // Reposition() extends the bounds so they overlap the client edge (or window
  // exterior, when there is no client edge -- though this may have been clipped
  // out above) by 1 DIP.  We want to draw a 1 px shadow at all scale factors,
  // on the innermost pixel of that overlap.  So we need to inset the extended
  // edges by one DIP, less 1 px.
  const int inset = shadow_thickness_px - 1;
  bubble_rect.Inset((style == STANDARD_RIGHT) ? 0 : inset, 0,
                    (style == STANDARD_RIGHT) ? inset : 0, inset);

  // Now |bubble_rect| encloses the desired path and is aligned to the pixel
  // edges.  Place the path on pixel centers.
  bubble_rect.Inset(0.5, 0.5);
  SkPath path;
  ...

[Bret, 2016/08/29 21:57:42]

Done; edited the suggested comment.

+  bubble_rect.Inset(inset_left - 0.5, 0.5, inset_right - 0.5,
+                    subdip_layout_inset - 0.5);
+
+  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);
+
+  // Get the fill path by subtracting the shadow so they align neatly.
+  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(kTextPositionX, 0,
+                      popup_size_.width() - kTextHorizPadding,
+                      popup_size_.height());
+  text_rect.Inset(kShadowThickness, kShadowThickness);
   // 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 38 matching lines @@
         status_view_(status_view),
         expansion_start_(0),
         expansion_end_(0) {
   }
 
   // Manage the expansion of the bubble.
   void StartExpansion(const base::string16& expanded_text,
                       int current_width,
                       int expansion_end);
 
-  // Set width of fully expanded bubble.
-  void SetExpandedWidth(int expanded_width);
-
  private:
   // Animation functions.
   int GetCurrentBubbleWidth();
   void SetBubbleWidth(int width);
   void AnimateToState(double state) override;
   void AnimationEnded(const gfx::Animation* animation) override;
 
   // Manager that owns us.
   StatusBubbleViews* status_bubble_;
 
@@ skipping 41 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),
       base_view_(base_view),
       view_(NULL),
       download_shelf_is_visible_(false),
       is_expanded_(false),
+      has_client_edge_(has_client_edge),
       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(), size_,
+          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;
     // TODO(flackr): Get the non-transformed point so that the status bubble
     // popup window's position is consistent with the base_view_'s window.
     views::View::ConvertPointToScreen(base_view_, &top_left);
 
+    view_->SetWidth(size_.width());
     popup_->SetBounds(gfx::Rect(top_left.x() + position_.x(),
                                 top_left.y() + position_.y(),
                                 size_.width(), size_.height()));
   }
 }
 
 gfx::Size StatusBubbleViews::GetPreferredSize() {
   return gfx::Size(0, gfx::FontList().GetHeight() + kTotalVerticalPadding);
 }
 
@@ skipping 44 matching lines @@
     return;
   }
 
   // Reset expansion state only when bubble is completely hidden.
   if (view_->state() == StatusView::BUBBLE_HIDDEN) {
     is_expanded_ = false;
     SetBubbleWidth(GetStandardStatusBubbleWidth());
   }
 
   // Set Elided Text corresponding to the GURL object.
-  gfx::Rect popup_bounds = popup_->GetWindowBoundsInScreen();
-  int text_width = static_cast<int>(popup_bounds.width() -
-      (kShadowThickness * 2) - kTextPositionX - kTextHorizPadding - 1);
+  int text_width = static_cast<int>(size_.width() - (kShadowThickness * 2) -
+                                    kTextPositionX - kTextHorizPadding - 1);
   url_text_ =
       url_formatter::ElideUrl(url, gfx::FontList(), text_width);
 
   // An URL is always treated as a left-to-right string. On right-to-left UIs
   // we need to explicitly mark the URL as LTR to make sure it is displayed
   // correctly.
   url_text_ = base::i18n::GetDisplayStringInLTRDirectionality(url_text_);
 
   if (IsFrameVisible()) {
     view_->SetText(url_text_, true);
@@ skipping 143 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;
-  expand_view_->StartExpansion(url_text_, popup_bounds.width(),
-                               expanded_bubble_width);
+  expand_view_->StartExpansion(url_text_, size_.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();
 }