Stylize PlaceholderImages with icons.

third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp

Index: third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp b/third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp
index 9023571e795998239fd43c78bc01b0cd951ac3c3..f5a6355854d9b103aa080e4e9c9c0a69bfc99594 100644
--- a/third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp
+++ b/third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp
@@ -5,8 +5,14 @@
 #include "platform/graphics/PlaceholderImage.h"
 
 #include "platform/geometry/FloatRect.h"
+#include "platform/geometry/IntPoint.h"
+#include "platform/geometry/IntRect.h"
+#include "platform/graphics/BitmapImage.h"
 #include "platform/graphics/Color.h"
+#include "platform/graphics/GraphicsContext.h"
 #include "platform/graphics/ImageObserver.h"
+#include "platform/graphics/paint/PaintCanvas.h"
+#include "platform/graphics/paint/PaintFlags.h"
 #include "platform/graphics/paint/PaintRecord.h"
 #include "platform/graphics/paint/PaintRecorder.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -15,12 +21,8 @@
 
 namespace blink {
 
-namespace {
-
-// Gray with 40% opacity.
-const RGBA32 kFillColor = 0x66808080;
-
-}  // namespace
+PlaceholderImage::PlaceholderImage(ImageObserver* observer, const IntSize& size)
+    : Image(observer), size_(size) {}
 
 PlaceholderImage::~PlaceholderImage() {}
 
@@ -30,6 +32,7 @@ sk_sp<SkImage> PlaceholderImage::ImageForCurrentFrame() {
 
   const FloatRect dest_rect(0.0f, 0.0f, static_cast<float>(size_.Width()),
                             static_cast<float>(size_.Height()));
+
   PaintRecorder paint_recorder;
   Draw(paint_recorder.beginRecording(dest_rect), PaintFlags(), dest_rect,
        dest_rect, kDoNotRespectImageOrientation, kClampImageToSourceRect);
@@ -46,8 +49,8 @@ void PlaceholderImage::Draw(PaintCanvas* canvas,
                             const PaintFlags& base_flags,
                             const FloatRect& dest_rect,
                             const FloatRect& src_rect,
-                            RespectImageOrientationEnum,
-                            ImageClampingMode) {
+                            RespectImageOrientationEnum respect_orientation,
+                            ImageClampingMode image_clamping_mode) {
   if (!src_rect.Intersects(FloatRect(0.0f, 0.0f,
                                      static_cast<float>(size_.Width()),
                                      static_cast<float>(size_.Height())))) {
@@ -56,8 +59,54 @@ void PlaceholderImage::Draw(PaintCanvas* canvas,
 

[kinuko, 2017/06/05 06:30:13]

Can you add a link to the 'spec' that explain these constants?

[sclittle, 2017/06/05 22:26:08]

Done.

   PaintFlags flags(base_flags);
   flags.setStyle(PaintFlags::kFill_Style);
-  flags.setColor(kFillColor);
+  flags.setColor(0x80D9D9D9);

[vmpstr, 2017/06/05 18:05:28]

nit: You can use SkColorSetARGB or SkColorSetARGBMacro here instead of a magic
hex

[sclittle, 2017/06/05 22:26:08]

Done.

   canvas->drawRect(dest_rect, flags);
+
+  constexpr int kIconWidth = 24;
+  constexpr int kIconHeight = 24;
+  constexpr int kIconPaddingX = 8;
+  constexpr int kIconPaddingY = 5;

[f(malita), 2017/06/05 13:04:27]

Instead of hard-coding UI constants here, is it feasible to let the resource own
this data?

E.g. make the png exactly 40x34 (including transparent padding), then only use
icon_image->width()/height() in the code here and just draw it at its natural
size.  That should also ensure we're not wasting any cycles scaling at raster
time.

[sclittle, 2017/06/05 22:26:08]

Right now, the resource itself is a 4x version of the image, which gets scaled
down when drawn so that it looks nice on high-DPI screens.

I don't think high-DPI PNGs are a thing, and SVGs are heavyweight, so I don't
know of a nice way to do this in the resource itself.

Do you know of any other way to save those cycles? We could maybe hold onto a
static ref of the already-scaled SkImage/PaintImage, but I'm not sure that'll
actually save cycles at raster time.

[f(malita), 2017/06/06 14:04:21]

Ah, yes, makes sense to have higher res for hidpi.
It's not a huge deal, if there's a good reason do scale (hidpi) then I'm fine
with it.

+
+  if (dest_rect.Width() < kIconWidth + 2 * kIconPaddingX ||
+      dest_rect.Height() < kIconHeight + 2 * kIconPaddingY) {
+    return;
+  }
+
+  DEFINE_STATIC_REF(Image, icon_image,
+                    (Image::LoadPlatformResource("placeholderIcon")));
+  DCHECK(!icon_image->IsNull());
+
+  FloatRect icon_dest_rect(
+      dest_rect.X() + (dest_rect.Width() - kIconWidth) / 2.0f,
+      dest_rect.Y() + (dest_rect.Height() - kIconHeight) / 2.0f, kIconWidth,
+      kIconHeight);
+
+  // Note that the |icon_image| is not scaled according to dest_rect / src_rect,
+  // and is always drawn at the same size. This is so that placeholder icons are
+  // visible (e.g. when replacing a large image that's scaled down to a small
+  // area) and so that all placeholder images on the same page look consistent.
+  icon_image->Draw(canvas, base_flags, icon_dest_rect,

[f(malita), 2017/06/05 13:04:27]

Do we want/need the placeholder to observe RespectImageOrientationEnum?

Depending on the answer, it may be OK to call
canvas->drawImage(icon_image->PaintImageForCurrentFrame(),...) directly here.

[sclittle, 2017/06/05 22:26:08]

We probably don't want it to observe the enum, nice catch, thanks. Waiting for
an answer about image scaling before changing it to canvas->drawImage(...),
which doesn't look like it supports scaling.

[f(malita), 2017/06/06 14:04:21]

You can use PaintCanvas::drawImageRect() to scale.

[sclittle, 2017/06/07 19:43:54]

Done.

+                   IntRect(IntPoint::Zero(), icon_image->Size()),
+                   respect_orientation, image_clamping_mode);
+}
+
+void PlaceholderImage::DrawPattern(GraphicsContext& context,
+                                   const FloatRect& src_rect,
+                                   const FloatSize& scale,
+                                   const FloatPoint& phase,
+                                   SkBlendMode mode,
+                                   const FloatRect& dest_rect,
+                                   const FloatSize& repeat_spacing) {
+  DCHECK(context.Canvas());
+
+  PaintFlags flags = context.FillFlags();
+  flags.setBlendMode(mode);
+
+  // Ignore the pattern specifications and just draw a single placeholder image
+  // over the whole |dest_rect|. This is done in order to prevent repeated icons
+  // from cluttering tiled background images.
+  Draw(context.Canvas(), flags, dest_rect, src_rect,
+       kDoNotRespectImageOrientation, kClampImageToSourceRect);
 }
 
 void PlaceholderImage::DestroyDecodedData() {