Make PaintTabBackgroundUsingFillId() non-method

chrome/browser/ui/views/tabs/tab.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/tabs/tab.h"
 
 #include <stddef.h>
 #include <limits>
 #include <utility>
 
@@ skipping 98 matching lines @@
 
 // The minimum opacity (out of 1) when a tab (either active or inactive) is
 // throbbing in the immersive mode light strip.
 const double kImmersiveTabMinThrobOpacity = 0.66;
 
 // Number of steps in the immersive mode loading animation.
 const int kImmersiveLoadingStepCount = 32;
 
 const char kTabCloseButtonName[] = "TabCloseButton";
 
+// Parameters for PaintTabBackgroundUsingParams().
+struct PaintBackgroundParams {
+  PaintBackgroundParams(bool is_active,
+                        gfx::ImageSkia* fill_image_ptr,
+                        bool has_custom_image,
+                        gfx::Rect offset_rect,

[Peter Kasting, 2016/08/09 02:52:31]

|offset_rect| feels like a misleading name here.  A point is an offset; the
origin of this rect is an offset.  But that ignores the purpose of the rect's
size.  It seems like this isn't just about giving an offset, but defining the
whole region to paint in.  Probably a more general name like just |rect| would
be less misleading.  (multiple places)

Note that a shorter name would have the side effect of reducing the
line-wrapping you noted was added.

[Greg Levin, 2016/08/09 17:28:18]

Done.

+                        SkColor stroke_color,
+                        SkColor toolbar_color,
+                        SkColor background_color)
+      : is_active(is_active),
+        has_custom_image(has_custom_image),
+        offset_rect(offset_rect),
+        stroke_color(stroke_color),
+        toolbar_color(toolbar_color),
+        background_color(background_color) {
+    if (fill_image_ptr)
+      fill_image = *fill_image_ptr;
+  }
+
+  bool is_active;
+  gfx::ImageSkia fill_image;
+  bool has_custom_image;
+  gfx::Rect offset_rect;
+  SkColor stroke_color;
+  SkColor toolbar_color;
+  SkColor background_color;

[Peter Kasting, 2016/08/09 02:52:31]

Nit: All of these but |fill_image| could be const.  That could be const too with
some clever code in the initializer list.

[Greg Levin, 2016/08/09 17:28:17]

Done.

+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // ImageCacheEntryMetadata
 //
 // All metadata necessary to uniquely identify a cached image.
 struct ImageCacheEntryMetadata {
   ImageCacheEntryMetadata(int resource_id,
                           SkColor fill_color,
                           SkColor stroke_color,
                           ui::ScaleFactor scale_factor,
                           const gfx::Size& size);
@@ skipping 215 matching lines @@
                    1.5 * scale);
   }
   path->lineTo(right - 2 * scale, bottom - 1 - 1.5 * scale);
   path->rCubicTo(0.375 * scale, scale, 1.25 * scale, 1.5 * scale, 2 * scale,
                  1.5 * scale);
   path->rLineTo(0, 1);
   path->close();
 }
 
 void PaintTabFill(gfx::Canvas* canvas,
-                  gfx::ImageSkia* fill_image,
-                  int x_offset,
-                  int y_offset,
-                  const gfx::Size& size,
+                  const gfx::ImageSkia& fill_image,
+                  gfx::Rect offset_rect,
                   bool is_active) {
   const gfx::Insets tab_insets(GetLayoutInsets(TAB));
   // If this isn't the foreground tab, don't draw over the toolbar, but do
   // include the 1 px divider stroke at the bottom.
   const int toolbar_overlap = is_active ? 0 : (tab_insets.bottom() - 1);
 
   // Draw left edge.
   gfx::ImageSkia tab_l = gfx::ImageSkiaOperations::CreateTiledImage(
-      *fill_image, x_offset, y_offset, g_mask_images.l_width, size.height());
+      fill_image, offset_rect.x(), offset_rect.y(), g_mask_images.l_width,
+      offset_rect.height());
   gfx::ImageSkia theme_l = gfx::ImageSkiaOperations::CreateMaskedImage(
       tab_l, *g_mask_images.image_l);
   canvas->DrawImageInt(
       theme_l, 0, 0, theme_l.width(), theme_l.height() - toolbar_overlap, 0, 0,
       theme_l.width(), theme_l.height() - toolbar_overlap, false);
 
   // Draw right edge.
   gfx::ImageSkia tab_r = gfx::ImageSkiaOperations::CreateTiledImage(
-      *fill_image, x_offset + size.width() - g_mask_images.r_width, y_offset,
-      g_mask_images.r_width, size.height());
+      fill_image, offset_rect.x() + offset_rect.width() - g_mask_images.r_width,

[Peter Kasting, 2016/08/09 02:52:31]

Nit: Use right() in place of x() + width()

[Greg Levin, 2016/08/09 17:28:17]

Done.

+      offset_rect.y(), g_mask_images.r_width, offset_rect.height());
   gfx::ImageSkia theme_r = gfx::ImageSkiaOperations::CreateMaskedImage(
       tab_r, *g_mask_images.image_r);
-  canvas->DrawImageInt(theme_r, 0, 0, theme_r.width(),
-                       theme_r.height() - toolbar_overlap,
-                       size.width() - theme_r.width(), 0, theme_r.width(),
-                       theme_r.height() - toolbar_overlap, false);
+  canvas->DrawImageInt(
+      theme_r, 0, 0, theme_r.width(), theme_r.height() - toolbar_overlap,
+      offset_rect.width() - theme_r.width(), 0, theme_r.width(),
+      theme_r.height() - toolbar_overlap, false);
 
   // Draw center.  Instead of masking out the top portion we simply skip over it
   // by incrementing by the top padding, since it's a simple rectangle.
   canvas->TileImageInt(
-      *fill_image, x_offset + g_mask_images.l_width,
-      y_offset + tab_insets.top(), g_mask_images.l_width, tab_insets.top(),
-      size.width() - g_mask_images.l_width - g_mask_images.r_width,
-      size.height() - tab_insets.top() - toolbar_overlap);
+      fill_image, offset_rect.x() + g_mask_images.l_width,
+      offset_rect.y() + tab_insets.top(), g_mask_images.l_width,
+      tab_insets.top(),
+      offset_rect.width() - g_mask_images.l_width - g_mask_images.r_width,
+      offset_rect.height() - tab_insets.top() - toolbar_overlap);

[Peter Kasting, 2016/08/09 02:52:31]

Nit: Shorter:

  const int left = g_mask_images.l_width;
  const int top = tab_insets.top();
  offset_rect.Inset(left, top, g_mask_images.r_width, toolbar_overlap);
  canvas->TileImageInt(fill_image, offset_rect.x(), offset_rect.y(), left, top,
                       offset_rect.width(), offset_rect.height());

(This is all making me think for the millionth time that we should really change
TileImageInt()'s signature, and those of the related functions, to take Points
and Sizes instead of ints.  Then we could just do this:)

  const gfx::Point dest(g_mask_images.l_width, tab_insets.top());
  canvas->TileImageInt(fill_image, offset_rect.origin(), dest,
                       offset_rect.size());

[Greg Levin, 2016/08/09 17:28:17]

The offset_rect -> rect change had already shortened this to 5 lines, so I went
with a 4 line version w/o new left & top variables.

+}
+
+void PaintTabBackgroundUsingParams(gfx::Canvas* canvas,
+                                   views::GlowHoverController* hc,
+                                   const PaintBackgroundParams& params) {
+  const SkScalar kMinHoverRadius = 16;
+  const SkScalar radius = std::max(
+      SkFloatToScalar(params.offset_rect.width() / 4.f), kMinHoverRadius);
+  const bool draw_hover = !params.is_active && hc;
+  SkPoint hover_location(
+      PointToSkPoint(draw_hover ? hc->location() : gfx::Point()));
+  const SkColor hover_color =
+      SkColorSetA(params.toolbar_color, draw_hover ? hc->GetAlpha() : 255);
+
+  if (ui::MaterialDesignController::IsModeMaterial()) {
+    gfx::ScopedCanvas scoped_canvas(canvas);
+    const float scale = canvas->UndoDeviceScaleFactor();
+
+    // Draw the fill.
+    SkPath fill;
+    GetFillPath(scale, params.offset_rect.size(), &fill);
+    SkPaint paint;
+    paint.setAntiAlias(true);
+    {
+      gfx::ScopedCanvas clip_scoper(canvas);
+      canvas->ClipPath(fill, true);
+      if (!params.fill_image.isNull()) {
+        gfx::ScopedCanvas scale_scoper(canvas);
+        canvas->sk_canvas()->scale(scale, scale);
+        canvas->TileImageInt(
+            params.fill_image, params.offset_rect.x(), params.offset_rect.y(),
+            0, 0, params.offset_rect.width(), params.offset_rect.height());
+      } else {
+        paint.setColor(params.is_active ? params.toolbar_color
+                                        : params.background_color);
+        canvas->DrawRect(gfx::ScaleToEnclosingRect(
+                             gfx::Rect(params.offset_rect.size()), scale),
+                         paint);
+      }
+      if (draw_hover) {
+        hover_location.scale(SkFloatToScalar(scale));
+        DrawHighlight(canvas, hover_location, radius * scale, hover_color);
+      }
+    }
+
+    // Draw the stroke.
+    SkPath stroke;
+    GetBorderPath(scale, params.offset_rect.size(), false, &stroke);
+    Op(stroke, fill, kDifference_SkPathOp, &stroke);
+    if (!params.is_active) {
+      // Clip out the bottom line; this will be drawn for us by
+      // TabStrip::PaintChildren().
+      canvas->sk_canvas()->clipRect(
+          SkRect::MakeWH(params.offset_rect.width() * scale,
+                         params.offset_rect.height() * scale - 1));
+    }
+    paint.setColor(params.stroke_color);
+    canvas->DrawPath(stroke, paint);
+  } else {
+    if (draw_hover) {
+      // Draw everything to a temporary canvas so we can extract an image for
+      // use in masking the hover glow.
+      gfx::Canvas background_canvas(params.offset_rect.size(),
+                                    canvas->image_scale(), false);
+      PaintTabFill(&background_canvas, params.fill_image, params.offset_rect,
+                   params.is_active);
+      gfx::ImageSkia background_image(background_canvas.ExtractImageRep());
+      canvas->DrawImageInt(background_image, 0, 0);
+
+      gfx::Canvas hover_canvas(params.offset_rect.size(), canvas->image_scale(),
+                               false);
+      DrawHighlight(&hover_canvas, hover_location, radius, hover_color);
+      gfx::ImageSkia result = gfx::ImageSkiaOperations::CreateMaskedImage(
+          gfx::ImageSkia(hover_canvas.ExtractImageRep()), background_image);
+      canvas->DrawImageInt(result, 0, 0);
+    } else {
+      PaintTabFill(canvas, params.fill_image, params.offset_rect,
+                   params.is_active);
+    }
+
+    // Now draw the stroke, highlights, and shadows around the tab edge.
+    TabImages* stroke_images =
+        params.is_active ? &g_active_images : &g_inactive_images;
+    canvas->DrawImageInt(*stroke_images->image_l, 0, 0);
+    canvas->TileImageInt(*stroke_images->image_c, stroke_images->l_width, 0,
+                         params.offset_rect.width() - stroke_images->l_width -
+                             stroke_images->r_width,
+                         params.offset_rect.height());
+    canvas->DrawImageInt(*stroke_images->image_r,
+                         params.offset_rect.width() - stroke_images->r_width,
+                         0);
+  }
 }
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // FaviconCrashAnimation
 //
 //  A custom animation subclass to manage the favicon crash animation.
 class Tab::FaviconCrashAnimation : public gfx::LinearAnimation,
                                    public gfx::AnimationDelegate {
@@ skipping 1068 matching lines @@
     it = g_image_cache->begin();
   }
   canvas->DrawImageInt(it->image, 0, 0);
 }
 
 void Tab::PaintTabBackgroundUsingFillId(gfx::Canvas* canvas,
                                         bool is_active,
                                         int fill_id,
                                         bool has_custom_image,
                                         int y_offset) {
-  const ui::ThemeProvider* tp = GetThemeProvider();
-  const SkColor toolbar_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
-  gfx::ImageSkia* fill_image = tp->GetImageSkiaNamed(fill_id);
+  views::GlowHoverController* hc =
+      hover_controller_.ShouldDraw() ? &hover_controller_ : nullptr;
+  gfx::ImageSkia* fill_image =
+      has_custom_image || !ui::MaterialDesignController::IsModeMaterial()
+          ? GetThemeProvider()->GetImageSkiaNamed(fill_id)
+          : nullptr;
   // The tab image needs to be lined up with the background image
   // so that it feels partially transparent.  These offsets represent the tab
   // position within the frame background image.
-  const int x_offset = GetMirroredX() + background_offset_.x();
+  gfx::Rect offset_rect(GetMirroredX() + background_offset_.x(), y_offset,
+                        width(), height());

[Peter Kasting, 2016/08/09 02:52:31]

Nit: Do you think it's any more readable to do this?  This calls out a little
more (to me) that we're trying to paint "our tab's bounds, relative to some
other point".

  gfx::Rect rect(GetLocalBounds());
  rect.Offset(GetMirroredX() + background_offset_.x(), y_offset);

[Greg Levin, 2016/08/09 17:28:17]

Done.

+  PaintBackgroundParams params(
+      is_active, fill_image, has_custom_image, offset_rect,
+      controller_->GetToolbarTopSeparatorColor(),
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR),
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB));
 
-  const SkScalar kMinHoverRadius = 16;
-  const SkScalar radius =
-      std::max(SkFloatToScalar(width() / 4.f), kMinHoverRadius);
-  const bool draw_hover = !is_active && hover_controller_.ShouldDraw();
-  SkPoint hover_location(PointToSkPoint(hover_controller_.location()));
-  const SkColor hover_color =
-      SkColorSetA(toolbar_color, hover_controller_.GetAlpha());
-
-  if (ui::MaterialDesignController::IsModeMaterial()) {
-    gfx::ScopedCanvas scoped_canvas(canvas);
-    const float scale = canvas->UndoDeviceScaleFactor();
-
-    // Draw the fill.
-    SkPath fill;
-    GetFillPath(scale, size(), &fill);
-    SkPaint paint;
-    paint.setAntiAlias(true);
-    {
-      gfx::ScopedCanvas clip_scoper(canvas);
-      canvas->ClipPath(fill, true);
-      if (has_custom_image) {
-        gfx::ScopedCanvas scale_scoper(canvas);
-        canvas->sk_canvas()->scale(scale, scale);
-        canvas->TileImageInt(*fill_image, x_offset, y_offset, 0, 0, width(),
-                             height());
-      } else {
-        paint.setColor(
-            is_active ? toolbar_color
-                      : tp->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB));
-        canvas->DrawRect(gfx::ScaleToEnclosingRect(GetLocalBounds(), scale),
-                         paint);
-      }
-      if (draw_hover) {
-        hover_location.scale(SkFloatToScalar(scale));
-        DrawHighlight(canvas, hover_location, radius * scale, hover_color);
-      }
-    }
-
-    // Draw the stroke.
-    SkPath stroke;
-    GetBorderPath(scale, size(), false, &stroke);
-    Op(stroke, fill, kDifference_SkPathOp, &stroke);
-    if (!is_active) {
-      // Clip out the bottom line; this will be drawn for us by
-      // TabStrip::PaintChildren().
-      canvas->sk_canvas()->clipRect(
-          SkRect::MakeWH(width() * scale, height() * scale - 1));
-    }
-    paint.setColor(controller_->GetToolbarTopSeparatorColor());
-    canvas->DrawPath(stroke, paint);
-  } else {
-    if (draw_hover) {
-      // Draw everything to a temporary canvas so we can extract an image for
-      // use in masking the hover glow.
-      gfx::Canvas background_canvas(size(), canvas->image_scale(), false);
-      PaintTabFill(&background_canvas, fill_image, x_offset, y_offset, size(),
-                   is_active);
-      gfx::ImageSkia background_image(background_canvas.ExtractImageRep());
-      canvas->DrawImageInt(background_image, 0, 0);
-
-      gfx::Canvas hover_canvas(size(), canvas->image_scale(), false);
-      DrawHighlight(&hover_canvas, hover_location, radius, hover_color);
-      gfx::ImageSkia result = gfx::ImageSkiaOperations::CreateMaskedImage(
-          gfx::ImageSkia(hover_canvas.ExtractImageRep()), background_image);
-      canvas->DrawImageInt(result, 0, 0);
-    } else {
-      PaintTabFill(canvas, fill_image, x_offset, y_offset, size(), is_active);
-    }
-
-    // Now draw the stroke, highlights, and shadows around the tab edge.
-    TabImages* stroke_images =
-        is_active ? &g_active_images : &g_inactive_images;
-    canvas->DrawImageInt(*stroke_images->image_l, 0, 0);
-    canvas->TileImageInt(
-        *stroke_images->image_c, stroke_images->l_width, 0,
-        width() - stroke_images->l_width - stroke_images->r_width, height());
-    canvas->DrawImageInt(*stroke_images->image_r,
-                         width() - stroke_images->r_width, 0);
-  }
+  PaintTabBackgroundUsingParams(canvas, hc, params);
 }
 
 void Tab::PaintPinnedTabTitleChangedIndicatorAndIcon(
     gfx::Canvas* canvas,
     const gfx::Rect& favicon_draw_bounds) {
   // The pinned tab title changed indicator consists of two parts:
   // . a clear (totally transparent) part over the bottom right (or left in rtl)
   //   of the favicon. This is done by drawing the favicon to a canvas, then
   //   drawing the clear part on top of the favicon.
   // . a circle in the bottom right (or left in rtl) of the favicon.
@@ skipping 228 matching lines @@
 gfx::Rect Tab::GetImmersiveBarRect() const {
   // The main bar is as wide as the normal tab's horizontal top line.
   // This top line of the tab extends a few pixels left and right of the
   // center image due to pixels in the rounded corner images.
   const int kBarPadding = 1;
   int main_bar_left = g_active_images.l_width - kBarPadding;
   int main_bar_right = width() - g_active_images.r_width + kBarPadding;
   return gfx::Rect(
       main_bar_left, 0, main_bar_right - main_bar_left, kImmersiveBarHeight);
 }