Bug 163668 – [Win][Direct2D] Implement ImageBufferData::getData.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Per Arne Vollan]

This method is not yet implemented for Direct2D.

[Per Arne Vollan]

Created attachment 292078 [details]
Patch

[WebKit Commit Bot]

Attachment 292078 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:78:  Use the class HWndDC instead of calling GetDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Comment on attachment 292078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292078&action=review

Thanks for getting this to work! I had a few minor comments I'd like you to address when landing.

> Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:50
> +    area *= rect.height();

You should use the "area()" method:  area = 4 * rect.area();

> Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:71
> +    hr = platformContext->QueryInterface(__uuidof(ID2D1GdiInteropRenderTarget), (void**)&gdiRenderTarget);

I think we should retain this in the GraphicsContextPlatformPrivate, since there will be other times we want to get a GDI HBITMAP or similar out of our D2D context.

You can do this as a separate patch.

> Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:89
> +    memcpy(result->data(), pixels, 4 * rect.width() * rect.height());

Can we avoid this extra copy by somehow using result->data() as the pixel source?

[Per Arne Vollan]

(In reply to comment #3)
> Comment on attachment 292078 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=292078&action=review
> 
> Thanks for getting this to work! I had a few minor comments I'd like you to
> address when landing.
> 
> > Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:50
> > +    area *= rect.height();
> 
> You should use the "area()" method:  area = 4 * rect.area();
> 
> > Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:71
> > +    hr = platformContext->QueryInterface(__uuidof(ID2D1GdiInteropRenderTarget), (void**)&gdiRenderTarget);
> 
> I think we should retain this in the GraphicsContextPlatformPrivate, since
> there will be other times we want to get a GDI HBITMAP or similar out of our
> D2D context.
> 
> You can do this as a separate patch.
> 
> > Source/WebCore/platform/graphics/win/ImageBufferDataDirect2D.cpp:89
> > +    memcpy(result->data(), pixels, 4 * rect.width() * rect.height());
> 
> Can we avoid this extra copy by somehow using result->data() as the pixel
> source?

I haven't found a way to do this yet, since the system is allocating the bitmap pixel data for us.

Thanks for reviewing!

[Per Arne Vollan]

Committed r207591: <http://trac.webkit.org/changeset/207591>