[webkit-reviews] review denied: [Bug 103704] [EFL][WK2] Implement Accelerated2DCanvas on WK2 Efl port : [Attachment 177177] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Dec 2 22:12:41 PST 2012
Noam Rosenthal <noam at webkit.org> has denied Kyungjin Kim
<gen.kim at samsung.com>'s request for review:
Bug 103704: [EFL][WK2] Implement Accelerated2DCanvas on WK2 Efl port
https://bugs.webkit.org/show_bug.cgi?id=103704
Attachment 177177: Patch
https://bugs.webkit.org/attachment.cgi?id=177177&action=review
------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=177177&action=review
> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:45
> + virtual bool hasGraphicsSurface() const { return true; }
I think it's better if this function returned true when GRAPHICS_SURFACE is on,
and false when off, like what you did inside Canvas2DLayerEfl.cpp.
Maybe Zeno has a different idea though :)
>
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.
cpp:678
> + m_canvasPlatformLayer->paintContents(*context, rect);
This is not right;
You should paint the canvas' content after painting the regular content, and
adjust it to contentsRect().
Otherwise the canvas' background and borders would disappear.
>
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.
cpp:763
> + if (m_canvasPlatformLayer &&
!m_canvasPlatformLayer->hasGraphicsSurface())
> + setDrawsContent(true);
I don't like setting drawsContent from within CoordinatedGraphicsLayer.
Better to refactor the functions that use it, like shouldHaveBackingStore()
More information about the webkit-reviews
mailing list