Summary: | [Win][Direct2D] Implement ImageBufferData::getData. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||
Component: | WebCore Misc. | Assignee: | Per Arne Vollan <pvollan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, bfulgham, commit-queue | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Per Arne Vollan
2016-10-19 10:11:04 PDT
Created attachment 292078 [details]
Patch
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.
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? (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! Committed r207591: <http://trac.webkit.org/changeset/207591> |