Fix Clipboard::ReadImage

ui/base/clipboard/clipboard_win.cc

Index: ui/base/clipboard/clipboard_win.cc
diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc
index 867448a4ee8cf30dd56e35cc70e76ae261e90548..16ac8cc400382638f3574c4bdf7db8b00b5011b4 100644
--- a/ui/base/clipboard/clipboard_win.cc
+++ b/ui/base/clipboard/clipboard_win.cc
@@ -438,27 +438,54 @@ SkBitmap Clipboard::ReadImage(Buffer buffer) const {
   if (!clipboard.Acquire(GetClipboardWindow()))
     return SkBitmap();
 
-  HBITMAP source_bitmap = static_cast<HBITMAP>(::GetClipboardData(CF_BITMAP));
-  if (!source_bitmap)
-    return SkBitmap();
-
-  base::win::ScopedHDC source_dc(::CreateCompatibleDC(NULL));
-  if (!source_dc)
-    return SkBitmap();
-  ::SelectObject(source_dc, source_bitmap);
-
-  // Get the dimensions of the bitmap.
-  BITMAPINFO bitmap_info = {};
-  bitmap_info.bmiHeader.biSize = sizeof(bitmap_info.bmiHeader);
-  ::GetDIBits(source_dc, source_bitmap, 0, 0, 0, &bitmap_info, DIB_RGB_COLORS);
-  int width = bitmap_info.bmiHeader.biWidth;
-  int height = bitmap_info.bmiHeader.biHeight;
-
-  gfx::CanvasSkia canvas(width, height, false);
-
-  skia::ScopedPlatformPaint scoped_platform_paint(&canvas);
-  HDC destination_dc = scoped_platform_paint.GetPlatformSurface();
-  ::BitBlt(destination_dc, 0, 0, width, height, source_dc, 0, 0, SRCCOPY);
+  // We use a DIB rather than a DDB here since ::GetObject() with the
+  // HBITMAP returned from ::GetClipboardData(CF_BITMAP) always reports a color
+  // depth of 32bpp.

[Wez, 2011/08/05 00:12:58]

The reason for using a DIB is really that DIBs are self-contained; if you
requested a DDB and the display were palettized then you'd get a bunch of pixel
data specific to the currently-in-effect palette, which isn't what you want.

[dcheng, 2011/08/05 20:33:08]

Not having to deal with the palette makes things easier, but the primary reason
I documented this is because the inability to differentiate between different
color depths with DDBs would break the alpha fixup code below.

+  BITMAPINFO* bitmap = static_cast<BITMAPINFO*>(::GetClipboardData(CF_DIB));
+  int color_table_length = 0;
+  switch (bitmap->bmiHeader.biBitCount) {
+    case 1:
+    case 4:
+    case 8:
+      color_table_length = bitmap->bmiHeader.biClrUsed
+          ? bitmap->bmiHeader.biClrUsed
+          : 1 << bitmap->bmiHeader.biBitCount;
+      break;
+    case 16:
+    case 32:
+      if (bitmap->bmiHeader.biCompression == BI_BITFIELDS)
+        color_table_length = 3;
+      break;
+    case 24:

[tony, 2011/08/04 22:06:56]

The msdn page about BITMAPINFOHEADER says, "The bmiColors color table is used
for optimizing colors used on palette-based devices, and must contain the number
of entries specified by the biClrUsed member of the BITMAPINFOHEADER."

Does that mean we need to check the value of biClrUsed when we have a 24bit
bitmap?

[dcheng, 2011/08/04 22:13:54]

It also says it's NULL right before that. Honestly, I have no idea how to
interpret that sentence.

[tony, 2011/08/04 22:20:52]

So it does.  That's really confusing.  Ok, LGTM.  pkasting might know more about
the bitmap format and might be able to translate what this means.

[Wez, 2011/08/05 00:12:58]

For bit counts of >=16, I think biClrUsed is supposed to specify the number of
palette entries provided to help optimize conversion should the bitmap be blit
to a palettized target.  Presumably that only makes sense for BI_RGB bitmaps,
but I'm not sure.

[dcheng, 2011/08/05 20:33:08]

Hmm. All the code I can find treats BI_RGB identically to how I handle it here.
Would it be worth adding a DCHECK()?

+      break;
+    default:
+      NOTREACHED();
+  }
+  const void* bitmap_bits = reinterpret_cast<const char*>(bitmap)
+      + bitmap->bmiHeader.biSize + color_table_length * sizeof(RGBQUAD);
+
+  gfx::CanvasSkia canvas(bitmap->bmiHeader.biWidth, bitmap->bmiHeader.biHeight,
+                         true);
+  {
+    skia::ScopedPlatformPaint scoped_platform_paint(&canvas);
+    HDC dc = scoped_platform_paint.GetPlatformSurface();
+    ::SetDIBitsToDevice(dc, 0, 0, bitmap->bmiHeader.biWidth,
+                        bitmap->bmiHeader.biHeight, 0, 0, 0,
+                        bitmap->bmiHeader.biHeight, bitmap_bits, bitmap,
+                        DIB_RGB_COLORS);
+  }
+  // SetDIBitsToDevice doesn't properly set alpha values for bitmaps with
+  // depth < 32bpp so manually fix it up.
+  if (bitmap->bmiHeader.biBitCount < 32) {
+    const SkBitmap& device_bitmap = canvas.getDevice()->accessBitmap(true);
+    SkAutoLockPixels lock(device_bitmap);
+    for (int i = 0; i < device_bitmap.height(); ++i) {
+      for (int j = 0; j < device_bitmap.width(); ++j) {
+        *device_bitmap.getAddr32(i, j) =
+            SkColorSetA(*device_bitmap.getAddr32(i, j), 0xFF);
+      }
+    }
+  }
   return canvas.ExtractBitmap();
 }