Fix pixelation of tab borders when device scale changes (abandoned)

chrome/browser/ui/views/tabs/tab.cc

Index: chrome/browser/ui/views/tabs/tab.cc
diff --git a/chrome/browser/ui/views/tabs/tab.cc b/chrome/browser/ui/views/tabs/tab.cc
index 13209f1bb0e482b956f1a648b0d673e6b5963a6a..2522467107a0be94b2a8dce6e57759c2219178c6 100644
--- a/chrome/browser/ui/views/tabs/tab.cc
+++ b/chrome/browser/ui/views/tabs/tab.cc
@@ -44,6 +44,7 @@
 #include "ui/gfx/color_analysis.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/geometry/rect_conversions.h"
+#include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/path.h"
@@ -117,6 +118,61 @@ const int kImmersiveLoadingStepCount = 32;
 const char kTabCloseButtonName[] = "TabCloseButton";
 
 ////////////////////////////////////////////////////////////////////////////////
+// PaintBackgroundParams
+
+struct PaintBackgroundParams {
+  bool is_active;
+  gfx::ImageSkia fill_image;
+  bool has_custom_image;
+  gfx::Point offset;
+  gfx::Size size;

[oshima, 2016/07/28 16:56:06]

Sorry I should have noticed this before, but looks like these two are for tab
bounds? If so, we can just use gfx::Rect.

[Greg Levin, 2016/07/28 20:03:54]

Is this a strong preference?  I don't have a sense of how close the logical
connection of these two are, and replacing these with "offset_rect" is going to
wrap several more lines.  Also, in some places where it's used, generic "size()"
seems to make more sense than "offset_rect.size()".

[oshima, 2016/07/28 20:45:48]

It just looked to me that drawing code uses it as bounds. (DrawXxx(x, y, width,
height)).

[Greg Levin, 2016/08/10 20:03:09]

Done.

+  SkColor stroke_color;
+  SkColor toolbar_color;
+  SkColor background_color;
+};
+
+void PaintTabBackgroundUsingFillId(gfx::Canvas* canvas,
+                                   views::GlowHoverController* hc,
+                                   const PaintBackgroundParams& params);
+
+////////////////////////////////////////////////////////////////////////////////
+// TabImageSource
+//
+// This subclass of CanvasImageSource allows the inactive tab image to be cached
+// once and rendered correctly for all scale factors.
+class TabImageSource : public gfx::CanvasImageSource {
+ public:
+  TabImageSource(Tab* tab, int fill_id, int y_offset);
+  ~TabImageSource() override {}
+
+  // gfx::CanvasImageSource override.
+  void Draw(gfx::Canvas* canvas) override;
+
+ private:
+  PaintBackgroundParams params_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabImageSource);
+};
+
+TabImageSource::TabImageSource(Tab* tab, int fill_id, int y_offset)
+    : gfx::CanvasImageSource(tab->size(), true) {
+  params_.is_active = false;
+  params_.fill_image = *tab->GetThemeProvider()->GetImageSkiaNamed(fill_id);
+  params_.has_custom_image = false;
+  params_.offset.SetPoint(0, y_offset);
+  params_.size = tab->size();
+  params_.stroke_color = tab->controller()->GetToolbarTopSeparatorColor();
+  params_.toolbar_color =
+      tab->GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR);
+  params_.background_color =
+      tab->GetThemeProvider()->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB);
+}
+
+void TabImageSource::Draw(gfx::Canvas* canvas) {
+  PaintTabBackgroundUsingFillId(canvas, nullptr, params_);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // ImageCacheEntryMetadata
 //
 // All metadata necessary to uniquely identify a cached image.
@@ -124,7 +180,6 @@ struct ImageCacheEntryMetadata {
   ImageCacheEntryMetadata(int resource_id,
                           SkColor fill_color,
                           SkColor stroke_color,
-                          ui::ScaleFactor scale_factor,
                           const gfx::Size& size);
 
   ~ImageCacheEntryMetadata();
@@ -134,22 +189,17 @@ struct ImageCacheEntryMetadata {
   int resource_id;     // Only needed by pre-MD
   SkColor fill_color;  // Both colors only needed by MD
   SkColor stroke_color;
-  ui::ScaleFactor scale_factor;
   gfx::Size size;
 };
 
 ImageCacheEntryMetadata::ImageCacheEntryMetadata(int resource_id,
                                                  SkColor fill_color,
                                                  SkColor stroke_color,
-                                                 ui::ScaleFactor scale_factor,
                                                  const gfx::Size& size)
     : resource_id(resource_id),
       fill_color(fill_color),
       stroke_color(stroke_color),
-      scale_factor(scale_factor),
       size(size) {
-  DCHECK_NE(ui::SCALE_FACTOR_NONE, scale_factor);
-
   // Some fields are only relevant for pre-MD vs. MD.  Erase the irrelevant ones
   // so they don't cause incorrect cache misses.
   // TODO(pkasting): Remove |resource_id| field when non-MD code is deleted.
@@ -164,8 +214,7 @@ ImageCacheEntryMetadata::~ImageCacheEntryMetadata() {}
 bool ImageCacheEntryMetadata::operator==(
     const ImageCacheEntryMetadata& rhs) const {
   return resource_id == rhs.resource_id && fill_color == rhs.fill_color &&
-         stroke_color == rhs.stroke_color && scale_factor == rhs.scale_factor &&
-         size == rhs.size;
+         stroke_color == rhs.stroke_color && size == rhs.size;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -174,7 +223,9 @@ bool ImageCacheEntryMetadata::operator==(
 // A cached image and the metadata used to generate it.
 struct ImageCacheEntry {
   ImageCacheEntry(const ImageCacheEntryMetadata& metadata,
-                  const gfx::ImageSkia& image);
+                  Tab* tab,
+                  int fill_id,
+                  int y_offset);
   ~ImageCacheEntry();
 
   ImageCacheEntryMetadata metadata;
@@ -182,8 +233,11 @@ struct ImageCacheEntry {
 };
 
 ImageCacheEntry::ImageCacheEntry(const ImageCacheEntryMetadata& metadata,
-                                 const gfx::ImageSkia& image)
-    : metadata(metadata), image(image) {}
+                                 Tab* tab,
+                                 int fill_id,
+                                 int y_offset)
+    : metadata(metadata),
+      image(new TabImageSource(tab, fill_id, y_offset), tab->size()) {}
 
 ImageCacheEntry::~ImageCacheEntry() {}
 
@@ -273,8 +327,7 @@ void DrawHighlight(gfx::Canvas* canvas,
 bool ShouldThemifyFaviconForUrl(const GURL& url) {
   return url.SchemeIs(content::kChromeUIScheme) &&
          url.host() != chrome::kChromeUIHelpHost &&
-         url.host() != chrome::kChromeUIUberHost &&
-         url.host() != chrome::kChromeUIAppLauncherPageHost;
+         url.host() != chrome::kChromeUIUberHost;
 }
 
 // Computes a path corresponding to the tab's content region inside the outer
@@ -351,7 +404,7 @@ void GetBorderPath(float scale,
 }
 
 void PaintTabFill(gfx::Canvas* canvas,
-                  gfx::ImageSkia* fill_image,
+                  const gfx::ImageSkia& fill_image,
                   int x_offset,
                   int y_offset,
                   const gfx::Size& size,
@@ -363,7 +416,7 @@ void PaintTabFill(gfx::Canvas* canvas,
 
   // 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, x_offset, y_offset, g_mask_images.l_width, size.height());
   gfx::ImageSkia theme_l = gfx::ImageSkiaOperations::CreateMaskedImage(
       tab_l, *g_mask_images.image_l);
   canvas->DrawImageInt(
@@ -372,7 +425,7 @@ void PaintTabFill(gfx::Canvas* canvas,
 
   // Draw right edge.
   gfx::ImageSkia tab_r = gfx::ImageSkiaOperations::CreateTiledImage(
-      *fill_image, x_offset + size.width() - g_mask_images.r_width, y_offset,
+      fill_image, x_offset + size.width() - g_mask_images.r_width, y_offset,
       g_mask_images.r_width, size.height());
   gfx::ImageSkia theme_r = gfx::ImageSkiaOperations::CreateMaskedImage(
       tab_r, *g_mask_images.image_r);
@@ -384,12 +437,100 @@ void PaintTabFill(gfx::Canvas* canvas,
   // 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(),
+      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);
 }
 
+// For use by Tab and TabImageSource
+void PaintTabBackgroundUsingFillId(gfx::Canvas* canvas,
+                                   views::GlowHoverController* hc,
+                                   const PaintBackgroundParams& params) {
+  const SkScalar kMinHoverRadius = 16;
+  const SkScalar radius =
+      std::max(SkFloatToScalar(params.size.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.size, &fill);
+    SkPaint paint;
+    paint.setAntiAlias(true);
+    {
+      gfx::ScopedCanvas clip_scoper(canvas);
+      canvas->ClipPath(fill, true);
+      if (params.has_custom_image) {
+        gfx::ScopedCanvas scale_scoper(canvas);
+        canvas->sk_canvas()->scale(scale, scale);
+        canvas->TileImageInt(params.fill_image, params.offset.x(),
+                             params.offset.y(), 0, 0, params.size.width(),
+                             params.size.height());
+      } else {
+        paint.setColor(params.is_active ? params.toolbar_color
+                                        : params.background_color);
+        canvas->DrawRect(
+            gfx::ScaleToEnclosingRect(gfx::Rect(params.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.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.size.width() * scale, params.size.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.size, canvas->image_scale(), false);
+      PaintTabFill(&background_canvas, params.fill_image, params.offset.x(),
+                   params.offset.y(), params.size, params.is_active);
+      gfx::ImageSkia background_image(background_canvas.ExtractImageRep());
+      canvas->DrawImageInt(background_image, 0, 0);
+
+      gfx::Canvas hover_canvas(params.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.x(),
+                   params.offset.y(), params.size, 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.size.width() - stroke_images->l_width - stroke_images->r_width,
+        params.size.height());
+    canvas->DrawImageInt(*stroke_images->image_r,
+                         params.size.width() - stroke_images->r_width, 0);
+  }
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1454,16 +1595,12 @@ void Tab::PaintInactiveTabBackground(gfx::Canvas* canvas) {
 
   const ImageCacheEntryMetadata metadata(
       fill_id, tp->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB),
-      controller_->GetToolbarTopSeparatorColor(),
-      ui::GetSupportedScaleFactor(canvas->image_scale()), size());
+      controller_->GetToolbarTopSeparatorColor(), size());
   auto it = std::find_if(
       g_image_cache->begin(), g_image_cache->end(),
       [&metadata](const ImageCacheEntry& e) { return e.metadata == metadata; });
   if (it == g_image_cache->end()) {
-    gfx::Canvas tmp_canvas(size(), canvas->image_scale(), false);
-    PaintTabBackgroundUsingFillId(&tmp_canvas, false, fill_id, false, y_offset);
-    g_image_cache->emplace_front(metadata,
-                                 gfx::ImageSkia(tmp_canvas.ExtractImageRep()));
+    g_image_cache->emplace_front(metadata, this, fill_id, y_offset);
     if (g_image_cache->size() > kMaxImageCacheSize)
       g_image_cache->pop_back();
     it = g_image_cache->begin();
@@ -1476,93 +1613,24 @@ void Tab::PaintTabBackgroundUsingFillId(gfx::Canvas* canvas,
                                         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;
+  PaintBackgroundParams params;
+  params.is_active = is_active;
+  params.fill_image = *GetThemeProvider()->GetImageSkiaNamed(fill_id);

[oshima, 2016/07/28 16:56:06]

you can set this only if
has_custom_image || !ui::MaterialDesignController::IsModeMaterial()

[Greg Levin, 2016/07/28 20:03:54]

Done.
(I see this is good from a resource saving perspective, but wasn't sure, from a
code design perspective, if it was okay to tweak the parameters this
specifically based on the internal workings of the function, rather than
treating the function more as a black box.)

[oshima, 2016/07/28 20:45:48]

It wasn't for saving resource. It's creating a image even though we know it will
not be used, and that's what I feel odd about.

[Greg Levin, 2016/08/10 20:03:09]

Acknowledged.

+  params.has_custom_image = has_custom_image;
   // 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();
-
-  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);
-  }
+  params.offset.SetPoint(GetMirroredX() + background_offset_.x(), y_offset);
+  params.size = size();
+  params.stroke_color = controller_->GetToolbarTopSeparatorColor();
+  params.toolbar_color =
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR);
+  params.background_color =
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB);
+
+  ::PaintTabBackgroundUsingFillId(canvas, hc, params);
 }
 
 void Tab::PaintPinnedTabTitleChangedIndicatorAndIcon(