[webkit-reviews] review denied: [Bug 57409] [WinCairo] Implement Missing drawWindowsBitmap : [Attachment 87646] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Mar 30 18:24:48 PDT 2011
Adam Roben (:aroben) <aroben at apple.com> has denied Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 57409: [WinCairo] Implement Missing drawWindowsBitmap
https://bugs.webkit.org/show_bug.cgi?id=57409
Attachment 87646: Patch
https://bugs.webkit.org/attachment.cgi?id=87646&action=review
------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=87646&action=review
> Source/WebCore/platform/graphics/GraphicsContext.h:460
> + IntSize size() const { return m_bitmapBits.size(); }
Should be a const IntSize&.
> Source/WebCore/platform/graphics/win/GraphicsContextWin.cpp:91
> +GraphicsContext::WindowsBitmap::WindowsBitmap(HDC hdc,
GraphicsContext::WindowsBitmap::FromActiveBitmapType)
> + : m_hdc(hdc)
> +{
> + m_bitmap = static_cast<HBITMAP>(GetCurrentObject(hdc, OBJ_BITMAP));
> + m_bitmapBits.initializeFromHBitmap(m_bitmap);
> +}
This will still result in the HDC and HBITMAP being destroyed when the
WindowsBitmap is destroyed. That seems undesirable and wrong.
> Source/WebCore/platform/win/WindowsDIB.cpp:31
> +WindowsDIB::WindowsDIB(const HBITMAP& bitmap)
No need for the const&. HBITMAP is just a pointer.
Seems like this class should be in platform/graphics/win.
> Source/WebCore/platform/win/WindowsDIB.cpp:46
> +void WindowsDIB::initializeFromHBitmap(const HBITMAP& bitmap)
> +{
> + BITMAP bmpInfo;
> + GetObject(bitmap, sizeof(bmpInfo), &bmpInfo);
> +
> + m_bitmapBuffer = reinterpret_cast<UInt8*>(bmpInfo.bmBits);
> + m_bitmapBufferLength = bmpInfo.bmWidthBytes * bmpInfo.bmHeight;
> + m_size = IntSize(bmpInfo.bmWidth, bmpInfo.bmHeight);
> + m_bytesPerRow = bmpInfo.bmWidthBytes;
> + m_bitsPerPixel = bmpInfo.bmBitsPixel;
> +}
Why not just do this in the constructor?
No need for the const&. HBITMAP is just a pointer.
> Source/WebCore/platform/win/WindowsDIB.h:54
> + IntSize size() const { return m_size; }
Should be a const IntSize&.
More information about the webkit-reviews
mailing list