[webkit-reviews] review granted: [Bug 180141] [CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::PaintingEngine : [Attachment 327856] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 180141: [CoordGraphics] Move CoordinatedGraphicsLayer painting behind
Nicosia::PaintingEngine
https://bugs.webkit.org/show_bug.cgi?id=180141

Attachment 327856: Patch

https://bugs.webkit.org/attachment.cgi?id=327856&action=review




--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 327856
  --> https://bugs.webkit.org/attachment.cgi?id=327856
Patch

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

> Source/WebCore/ChangeLog:25
> +	   The Nicosia::PaintingEngine::create() function returns a new
> +	   PaintingEngine object. For now it defaults to PaintingEngineBasic,
but
> +	   it could be configured at configure-time or even runtime in the
future
> +	   to return a more advanced engine implementation.

If possible I would make this configurable at runtime, I'm sure we will want to
compare output between engines at some point.

> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngine.h:44
> +class PaintingEngine {
> +public:

WTF_MAKE_FAST_ALLOCATED?

> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngineBasic.cpp:62
> +    return true;

Why is it bool if it always returns true? Do you expect other implementations
might fail somehow? If that's the case I would probably change it when needed
eventually.

>
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
56
> +    virtual Nicosia::PaintingEngine& getPaintingEngine() = 0;

getPaintingEngine -> paintingEngine