[webkit-reviews] review denied: [Bug 104360] Coordinated Graphics: Separate LayerTreeCoordinator into LayerTreeCoordinator and CompositingCoordinator : [Attachment 203920] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied Gwang Yoon Hwang
<ryumiel at company100.net>'s request for review:
Bug 104360: Coordinated Graphics: Separate LayerTreeCoordinator into
LayerTreeCoordinator and CompositingCoordinator
https://bugs.webkit.org/show_bug.cgi?id=104360

Attachment 203920: Patch
https://bugs.webkit.org/attachment.cgi?id=203920&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=203920&action=review


>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
100
> +    m_client->flushCompositingStatePeripheralLayers();

This should be called something like didFlushRootLayer(); The peripheral layers
stuff should be known only to CoordinatedLTH.

>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
176
> +    m_client->prepareCustomFilterProxiesIfNeeded(state);

This should be called didSyncLayerState. The client can decide whether this is
about custom filters or something else.

>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.h:70

> +    class Client {
> +    public:
> +	   virtual void flushCompositingStatePeripheralLayers() = 0;
> +	   virtual void scheduleLayerFlushRequired() = 0;
> +	   virtual void
commitCoordinatedGraphicsState(CoordinatedGraphicsState&) = 0;
> +	   virtual void paintContents(const GraphicsLayer*, GraphicsContext&,
const IntRect& clipRect) = 0;
> +#if ENABLE(CSS_SHADERS)
> +	   virtual void
prepareCustomFilterProxiesIfNeeded(CoordinatedGraphicsLayerState&) = 0;
> +#endif
> +    };

This client is too specific.
The CompositingCoordinator::Client class should respond to events that
compositing coordinator knows about, otherwise it looks like an artificial
separation of classes...