Make SkBitmapToXcursorImage() handle premul bitmaps

This CL moves the conversion from premul alpha bitmap to unpremul into
SkBitmapTocursorImage(), instead of the caller. This wasn't being used
in Aura/X11 but present in Ozone/X11.

Also change the input parameter to a const reference.

Change-Id: I1768643e45472735e5ad7886b61f4443f8b74876
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2167276Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/master@{#763585}

content/common/cursors/webcursor_aurax11.cc

@@ -20,7 +20,7 @@ ui::PlatformCursor WebCursor::GetPlatformCursor(const ui::Cursor& cursor) {
   SkBitmap bitmap = GetCursorBitmap(cursor);
 
   XcursorImage* image =
-      ui::SkBitmapToXcursorImage(&bitmap, GetCursorHotspot(cursor));
+      ui::SkBitmapToXcursorImage(bitmap, GetCursorHotspot(cursor));
   platform_cursor_ = ui::CreateReffedCustomXCursor(image);
   return platform_cursor_;
 }

ui/base/cursor/cursor_loader_x11.cc

@@ -188,7 +188,7 @@ void CursorLoaderX11::LoadImageCursor(mojom::CursorType id,
   gfx::Point hotspot = hot;
 
   GetImageCursorBitmap(resource_id, scale(), rotation(), &hotspot, &bitmap);
-  XcursorImage* x_image = SkBitmapToXcursorImage(&bitmap, hotspot);
+  XcursorImage* x_image = SkBitmapToXcursorImage(bitmap, hotspot);
   image_cursors_[id] =
       std::make_unique<ImageCursor>(x_image, scale(), rotation());
 }
@@ -207,7 +207,7 @@ void CursorLoaderX11::LoadAnimatedCursor(mojom::CursorType id,
   x_images->nimage = bitmaps.size();
 
   for (unsigned int frame = 0; frame < bitmaps.size(); ++frame) {
-    XcursorImage* x_image = SkBitmapToXcursorImage(&bitmaps[frame], hotspot);
+    XcursorImage* x_image = SkBitmapToXcursorImage(bitmaps[frame], hotspot);
     x_image->delay = frame_delay_ms;
     x_images->images[frame] = x_image;
   }

ui/base/x/x11_util.cc

@@ -39,6 +39,7 @@
 #include "build/build_config.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
 #include "third_party/skia/include/core/SkTypes.h"
 #include "ui/base/x/x11_menu_list.h"
 #include "ui/base/x/x11_util_internal.h"
@@ -276,6 +277,20 @@ class XCustomCursorCache {
   DISALLOW_COPY_AND_ASSIGN(XCustomCursorCache);
 };
 
+// Converts a SKBitmap to unpremul alpha.
+SkBitmap ConvertSkBitmapToUnpremul(const SkBitmap& bitmap) {
+  DCHECK_NE(bitmap.alphaType(), kUnpremul_SkAlphaType);
+
+  SkImageInfo image_info = SkImageInfo::MakeN32(bitmap.width(), bitmap.height(),
+                                                kUnpremul_SkAlphaType);
+  SkBitmap converted_bitmap;
+  converted_bitmap.allocPixels(image_info);
+  bitmap.readPixels(image_info, converted_bitmap.getPixels(),
+                    image_info.minRowBytes(), 0, 0);
+
+  return converted_bitmap;
+}
+
 }  // namespace
 
 bool IsXInput2Available() {
@@ -316,11 +331,18 @@ void UnrefCustomXCursor(::Cursor cursor) {
   XCustomCursorCache::GetInstance()->Unref(cursor);
 }
 
-XcursorImage* SkBitmapToXcursorImage(const SkBitmap* cursor_image,
+XcursorImage* SkBitmapToXcursorImage(const SkBitmap& cursor_image,
                                      const gfx::Point& hotspot) {
   // TODO(crbug.com/596782): It is possible for cursor_image to be zeroed out
   // at this point, which leads to benign debug errors. Once this is fixed, we
-  // should  DCHECK_EQ(cursor_image->colorType(), kN32_SkColorType).
+  // should  DCHECK_EQ(cursor_image.colorType(), kN32_SkColorType).
+
+  // X11 expects bitmap with unpremul alpha. If bitmap is premul then convert,
+  // otherwise semi-transparent parts of cursor will look strange.
+  const SkBitmap converted = (cursor_image.alphaType() != kUnpremul_SkAlphaType)
+                                 ? ConvertSkBitmapToUnpremul(cursor_image)
+                                 : cursor_image;
+
   gfx::Point hotspot_point = hotspot;
   SkBitmap scaled;
 
@@ -328,31 +350,31 @@ XcursorImage* SkBitmapToXcursorImage(const SkBitmap* cursor_image,
   // pixels. So rescale the image if necessary.
   static const float kMaxPixel = GetMaxCursorSize();
   bool needs_scale = false;
-  if (cursor_image->width() > kMaxPixel || cursor_image->height() > kMaxPixel) {
+  if (converted.width() > kMaxPixel || converted.height() > kMaxPixel) {
     float scale = 1.f;
-    if (cursor_image->width() > cursor_image->height())
-      scale = kMaxPixel / cursor_image->width();
+    if (converted.width() > converted.height())
+      scale = kMaxPixel / converted.width();
     else
-      scale = kMaxPixel / cursor_image->height();
+      scale = kMaxPixel / converted.height();
 
-    scaled = skia::ImageOperations::Resize(*cursor_image,
+    scaled = skia::ImageOperations::Resize(converted,
         skia::ImageOperations::RESIZE_BETTER,
-        static_cast<int>(cursor_image->width() * scale),
-        static_cast<int>(cursor_image->height() * scale));
+        static_cast<int>(converted.width() * scale),
+        static_cast<int>(converted.height() * scale));
     hotspot_point = gfx::ScaleToFlooredPoint(hotspot, scale);
     needs_scale = true;
   }
 
-  const SkBitmap* bitmap = needs_scale ? &scaled : cursor_image;
-  XcursorImage* image = XcursorImageCreate(bitmap->width(), bitmap->height());
-  image->xhot = std::min(bitmap->width() - 1, hotspot_point.x());
-  image->yhot = std::min(bitmap->height() - 1, hotspot_point.y());
+  const SkBitmap& bitmap = needs_scale ? scaled : converted;
+  XcursorImage* image = XcursorImageCreate(bitmap.width(), bitmap.height());
+  image->xhot = std::min(bitmap.width() - 1, hotspot_point.x());
+  image->yhot = std::min(bitmap.height() - 1, hotspot_point.y());
 
-  if (bitmap->width() && bitmap->height()) {
+  if (bitmap.width() && bitmap.height()) {
     // The |bitmap| contains ARGB image, so just copy it.
     memcpy(image->pixels,
-           bitmap->getPixels(),
-           bitmap->width() * bitmap->height() * 4);
+           bitmap.getPixels(),
+           bitmap.width() * bitmap.height() * 4);
   }
 
   return image;

ui/base/x/x11_util.h

@@ -67,10 +67,10 @@ COMPONENT_EXPORT(UI_BASE_X) void RefCustomXCursor(::Cursor cursor);
 // Decreases the refcount of the custom cursor, and destroys it if it reaches 0.
 COMPONENT_EXPORT(UI_BASE_X) void UnrefCustomXCursor(::Cursor cursor);
 
-// Creates a XcursorImage and copies the SkBitmap |bitmap| on it. |bitmap|
-// should be non-null. Caller owns the returned object.
+// Creates a XcursorImage and copies the SkBitmap |bitmap| on it. Caller owns
+// the returned object.
 COMPONENT_EXPORT(UI_BASE_X)
-XcursorImage* SkBitmapToXcursorImage(const SkBitmap* bitmap,
+XcursorImage* SkBitmapToXcursorImage(const SkBitmap& bitmap,
                                      const gfx::Point& hotspot);
 
 // Coalesce all pending motion events (touch or mouse) that are at the top of

ui/ozone/platform/x11/x11_cursor_ozone.cc

@@ -4,7 +4,6 @@
 
 #include "ui/ozone/platform/x11/x11_cursor_ozone.h"
 
-#include "base/logging.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/gfx/geometry/point.h"
@@ -12,40 +11,9 @@
 
 namespace ui {
 
-namespace {
-
-// Converts a SKBitmap to unpremul alpha.
-SkBitmap ConvertSkBitmapToUnpremul(const SkBitmap& bitmap) {
-  DCHECK_NE(bitmap.alphaType(), kUnpremul_SkAlphaType);
-
-  SkImageInfo image_info = SkImageInfo::MakeN32(bitmap.width(), bitmap.height(),
-                                                kUnpremul_SkAlphaType);
-  SkBitmap converted_bitmap;
-  converted_bitmap.allocPixels(image_info);
-  bitmap.readPixels(image_info, converted_bitmap.getPixels(),
-                    image_info.minRowBytes(), 0, 0);
-
-  return converted_bitmap;
-}
-
-// Creates an XCursorImage for cursor bitmap.
-XcursorImage* CreateXCursorImage(const SkBitmap& bitmap,
-                                 const gfx::Point& hotspot) {
-  // X11 expects bitmap with unpremul alpha. If bitmap is premul then convert,
-  // otherwise semi-transparent parts of cursor will look strange.
-  if (bitmap.alphaType() != kUnpremul_SkAlphaType) {
-    SkBitmap converted_bitmap = ConvertSkBitmapToUnpremul(bitmap);
-    return SkBitmapToXcursorImage(&converted_bitmap, hotspot);
-  } else {
-    return SkBitmapToXcursorImage(&bitmap, hotspot);
-  }
-}
-
-}  // namespace
-
 X11CursorOzone::X11CursorOzone(const SkBitmap& bitmap,
                                const gfx::Point& hotspot) {
-  XcursorImage* image = CreateXCursorImage(bitmap, hotspot);
+  XcursorImage* image = SkBitmapToXcursorImage(bitmap, hotspot);
   xcursor_ = XcursorImageLoadCursor(gfx::GetXDisplay(), image);
   XcursorImageDestroy(image);
 }
@@ -58,7 +26,7 @@ X11CursorOzone::X11CursorOzone(const std::vector<SkBitmap>& bitmaps,
   XcursorImages* images = XcursorImagesCreate(bitmaps.size());
   images->nimage = bitmaps.size();
   for (size_t frame = 0; frame < bitmaps.size(); ++frame) {
-    XcursorImage* x_image = CreateXCursorImage(bitmaps[frame], hotspot);
+    XcursorImage* x_image = SkBitmapToXcursorImage(bitmaps[frame], hotspot);
     x_image->delay = frame_delay_ms;
     images->images[frame] = x_image;
   }