[Webkit-unassigned] [Bug 103704] [EFL][WK2] Implement Accelerated2DCanvas on WK2 Efl port
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Nov 29 22:50:00 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=103704
Noam Rosenthal <noam at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #176902|review?, commit-queue? |review-, commit-queue-
Flag| |
--- Comment #7 from Noam Rosenthal <noam at webkit.org> 2012-11-29 22:52:15 PST ---
(From update of attachment 176902)
View in context: https://bugs.webkit.org/attachment.cgi?id=176902&action=review
This is a good first attempt :)
However a lot of the choices in this patch don't explain themselves, and there are no comments in the changelog or in the code to explain them...
> Source/WebCore/ChangeLog:10
> + Implement accelerated 2D canvas using Coordinated Graphics on WK2 Efl port.
> + This implementation is based on COORDINATED_GRAPHICS.
> +
Needs more information.
> Source/WebCore/platform/graphics/efl/Canvas2DLayerEfl.h:51
> +#else
> + void paintContents(GraphicsContext&, const IntRect&);
> +#endif
This shouldn't really be inside the #else clause.
There could be a situation where graphics surfaces are compiled in, but allocating a graphisc surface didn't succeed, in which case this layer should still paint with software.
> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:50
> +#if ENABLE(ACCELERATED_2D_CANVAS)
> + virtual void paintContents(GraphicsContext&, const IntRect&) { }
> + virtual bool is2D() { return false; }
> +#endif
This is a bit confusing.
Perhaps hasGraphicsSurface() would be a better choice, that always returns false if ENABLE(GRAPHICS_SURFACE) is false.
> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:682
> +#if ENABLE(ACCELERATED_2D_CANVAS) && !USE(GRAPHICS_SURFACE)
> + if (m_canvasPlatformLayer && m_canvasPlatformLayer->is2D()) {
> + m_canvasPlatformLayer->paintContents(*context, rect);
> + return;
> + }
> +#endif
How about
if (m_canvasPlatformLayer && !m_canvasPlatformLayer->hasGraphicsSurface())
... paintContents() ...
> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:766
> +#if ENABLE(ACCELERATED_2D_CANVAS) && !USE(GRAPHICS_SURFACE)
> + if (m_canvasPlatformLayer && m_canvasPlatformLayer->is2D())
> + setDrawsContent(true);
> +#endif
This is quite strange.
Hard for me to understand why you would want an accelerated 2d canvas layer with no graphics surface; which would essentially mean it's not accelerated.
I'm willing to listen to reason but there is no explanation in the Changelog :)
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list