Consolidate Windows bitmap and DC code

skia/ext/skia_utils_win.cc

 // Copyright (c) 2006-2008 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 "skia/ext/skia_utils_win.h"
 
 #include <stddef.h>
 #include <windows.h>
 
+#include "base/debug/gdi_debug_util_win.h"
+#include "base/win/win_util.h"
 #include "third_party/skia/include/core/SkRect.h"
+#include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/core/SkTypes.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 
 namespace {
 
 static_assert(offsetof(RECT, left) == offsetof(SkIRect, fLeft), "o1");
 static_assert(offsetof(RECT, top) == offsetof(SkIRect, fTop), "o2");
 static_assert(offsetof(RECT, right) == offsetof(SkIRect, fRight), "o3");
 static_assert(offsetof(RECT, bottom) == offsetof(SkIRect, fBottom), "o4");
 static_assert(sizeof(RECT().left) == sizeof(SkIRect().fLeft), "o5");
 static_assert(sizeof(RECT().top) == sizeof(SkIRect().fTop), "o6");
 static_assert(sizeof(RECT().right) == sizeof(SkIRect().fRight), "o7");
 static_assert(sizeof(RECT().bottom) == sizeof(SkIRect().fBottom), "o8");
 static_assert(sizeof(RECT) == sizeof(SkIRect), "o9");
 
+void CreateBitmapHeaderWithColorDepth(LONG width,
+                                      LONG height,
+                                      WORD color_depth,
+                                      BITMAPINFOHEADER* hdr) {
+  // These values are shared with gfx::PlatformDevice

[Peter Kasting, 2016/06/22 02:23:34]

Nit: Trailing periods

[tomhudson, 2016/06/22 21:29:58]

Done.

+  hdr->biSize = sizeof(BITMAPINFOHEADER);
+  hdr->biWidth = width;
+  hdr->biHeight = -height;  // minus means top-down bitmap

[Peter Kasting, 2016/06/22 02:23:34]

Nit: Initial caps

[tomhudson, 2016/06/22 21:29:58]

Done.

+  hdr->biPlanes = 1;
+  hdr->biBitCount = color_depth;
+  hdr->biCompression = BI_RGB;  // no compression
+  hdr->biSizeImage = 0;
+  hdr->biXPelsPerMeter = 1;
+  hdr->biYPelsPerMeter = 1;
+  hdr->biClrUsed = 0;
+  hdr->biClrImportant = 0;
+}
+
+HBITMAP CreateHBitmap(int width, int height) {
+  // CreateDIBSection appears to get unhappy if we create an empty bitmap, so

[tomhudson, 2016/06/22 21:29:58]

Changed "appears to get unhappy if we create" to "fails to allocate anything if
we try to create"

+  // just create a minimal bitmap.
+  if ((width == 0) || (height == 0)) {
+    width = 1;
+    height = 1;
+  }
+
+  BITMAPINFOHEADER hdr = {0};
+  CreateBitmapHeaderWithColorDepth(width, height, 32, &hdr);
+
+  HBITMAP hbitmap = CreateDIBSection(nullptr,
+                                     reinterpret_cast<BITMAPINFO*>(&hdr),
+                                     0, nullptr, nullptr, 0);
+
+  // If CreateDIBSection() failed, try to get some useful information out
+  // before we crash for post-mortem analysis.
+  if (!hbitmap)
+    base::debug::GDIBitmapAllocFailure(&hdr, nullptr);
+
+  return hbitmap;
+}
+
+
 }  // namespace
 
 namespace skia {
 
+void CreateBitmapHeader(int width, int height, BITMAPINFOHEADER* hdr) {
+  CreateBitmapHeaderWithColorDepth(width, height, 32, hdr);
+}

[Peter Kasting, 2016/06/22 02:23:34]

Nit Function definition order should match .h declaration order.

[tomhudson, 2016/06/22 21:29:58]

Done.

+
 POINT SkPointToPOINT(const SkPoint& point) {
   POINT win_point = {
       SkScalarRoundToInt(point.fX), SkScalarRoundToInt(point.fY)
   };
   return win_point;
 }
 
 SkRect RECTToSkRect(const RECT& rect) {
   SkRect sk_rect = { SkIntToScalar(rect.left), SkIntToScalar(rect.top),
                      SkIntToScalar(rect.right), SkIntToScalar(rect.bottom) };
@@ skipping 26 matching lines @@
   // and arcs themselves fully respect the device context's world-to-device
   // transformation.
   BOOL res = SetGraphicsMode(context, GM_ADVANCED);
   SkASSERT(res != 0);
 
   // Enables dithering.
   res = SetStretchBltMode(context, HALFTONE);
   SkASSERT(res != 0);
   // As per SetStretchBltMode() documentation, SetBrushOrgEx() must be called
   // right after.
-  res = SetBrushOrgEx(context, 0, 0, NULL);
+  res = SetBrushOrgEx(context, 0, 0, nullptr);
   SkASSERT(res != 0);
 
   // Sets up default orientation.
   res = SetArcDirection(context, AD_CLOCKWISE);
   SkASSERT(res != 0);
 
   // Sets up default colors.
   res = SetBkColor(context, RGB(255, 255, 255));
   SkASSERT(res != CLR_INVALID);
   res = SetTextColor(context, RGB(0, 0, 0));
   SkASSERT(res != CLR_INVALID);
   res = SetDCBrushColor(context, RGB(255, 255, 255));
   SkASSERT(res != CLR_INVALID);
   res = SetDCPenColor(context, RGB(0, 0, 0));
   SkASSERT(res != CLR_INVALID);
 
   // Sets up default transparency.
   res = SetBkMode(context, OPAQUE);
   SkASSERT(res != 0);
   res = SetROP2(context, R2_COPYPEN);
   SkASSERT(res != 0);
 }
 
+SkImageInfo PrepareAllocation(HDC context, BITMAP* backing) {
+  HBITMAP backing_handle =
+      static_cast<HBITMAP>(GetCurrentObject(context, OBJ_BITMAP));
+  const size_t backing_size = sizeof(*backing);
+  if (GetObject(backing_handle, backing_size, backing) != backing_size)
+    return SkImageInfo();
+  return SkImageInfo::MakeN32Premul(backing->bmWidth, backing->bmHeight);

[Peter Kasting, 2016/06/22 02:23:34]

Nit: Fractionally less verbose; up to you

  return (GetObject(backing_handle, backing_size, backing) == backing_size)
             ? SkImageInfo::MakeN32Premul(backing->bmWidth, backing->bmHeight)
             : SkImageInfo();

[tomhudson, 2016/06/22 21:29:58]

Done.

+}
+
+sk_sp<SkSurface> MapPlatformSurface(HDC context) {
+  BITMAP backing;
+  const SkImageInfo size(PrepareAllocation(context, &backing));
+  if (size.isEmpty())
+    return nullptr;
+  return SkSurface::MakeRasterDirect(size, backing.bmBits,
+                                     backing.bmWidthBytes);

[Peter Kasting, 2016/06/22 02:23:33]

Nit: Likewise:

  return size.IsEmpty() ? nullptr
                        : SkSurface::MakeRasterDirect(size, backing.bmBits,
                                                      backing.bmWidthBytes);

[tomhudson, 2016/06/22 21:29:58]

Done.

+}
+
+SkBitmap MapPlatformBitmap(HDC context) {
+  BITMAP backing;
+  const SkImageInfo size(PrepareAllocation(context, &backing));
+  SkBitmap bitmap;
+  if (!size.isEmpty())
+    bitmap.installPixels(size, backing.bmBits, size.minRowBytes());
+  return bitmap;
+}
+
+HDC CreateOffscreenSurface(int width, int height) {
+  HBITMAP bitmap = nullptr;
+
+  // If this process doesn't have access to GDI, we'll have to use a shared
+  // memory segment instead.
+  if (!base::win::IsUser32AndGdi32Available())
+    return nullptr;
+
+  bitmap = CreateHBitmap(width, height);
+  if (!bitmap)
+    return nullptr;
+
+  HDC hdc = CreateCompatibleDC(nullptr);
+  SkASSERT(hdc);

[Peter Kasting, 2016/06/22 02:23:34]

Do we really want to assert here instead of just returning null on failure? 
Same below.

[tomhudson, 2016/06/22 21:29:58]

This code was taken from code that asserted, but since we return null if bitmap
creation fails we should be able to do the same here, although we need to at
least clean up the HDC...

Done, with header file comment update.

[Peter Kasting, 2016/06/22 23:43:51]

I don't see any header comment update for this function?

+  InitializeDC(hdc);
+  HRGN clip = CreateRectRgn(0, 0, width, height);
+  int result = SelectClipRgn(hdc, clip);
+  SkASSERT(result != ERROR);
+  result = DeleteObject(clip);
+  SkASSERT(result);
+
+  SelectObject(hdc, bitmap);
+
+  // The caller must call DeleteDC(hdc) on this object once done with it.

[Peter Kasting, 2016/06/22 02:23:34]

It's too bad we can't return a ScopedCreateDC here to enforce this.  I wonder if
it would be worth modifying that type to support moves like a real unique_ptr
does.

[tomhudson, 2016/06/22 21:29:58]

Filed crbug.com/622442.

+  return hdc;
+}
+
 }  // namespace skia