[webkit-reviews] review denied: [Bug 104360] Coordinated Graphics: Separate LayerTreeCoordinator into LayerTreeCoordinator and CompositingCoordinator : [Attachment 178863] Rebase after r137293

[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 178863: Rebase after r137293
https://bugs.webkit.org/attachment.cgi?id=178863&action=review

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


This change takes the code in a direction I don't agree with.
r- until we reach a concensus.

>
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinatorCom
mands.h:82
> +class CompositingCoordinatorCommands {
> +public:
> +    virtual void setRootCompositingLayer(CoordinatedLayerID) = 0;
> +    virtual void setCompositingLayerState(CoordinatedLayerID, const
CoordinatedLayerInfo&) = 0;
> +    virtual void setCompositingLayerChildren(CoordinatedLayerID, const
Vector<CoordinatedLayerID>&) = 0;
> +#if ENABLE(CSS_FILTERS)
> +    virtual void setCompositingLayerFilters(CoordinatedLayerID, const
WebCore::FilterOperations&) = 0;
> +#endif
> +    virtual void deleteCompositingLayer(CoordinatedLayerID) = 0;
> +
> +    virtual void createTileForLayer(CoordinatedLayerID, uint32_t tileID,
const WebCore::IntRect&, const SurfaceUpdateInfo&) = 0;
> +    virtual void updateTileForLayer(CoordinatedLayerID, uint32_t tileID,
const WebCore::IntRect&, const SurfaceUpdateInfo&) = 0;
> +    virtual void removeTileForLayer(CoordinatedLayerID, uint32_t tileID) =
0;
> +
> +    virtual void createUpdateAtlas(uint32_t atlasID, const
WebCoordinatedSurface::Handle&) = 0;
> +    virtual void removeUpdateAtlas(uint32_t atlasID) = 0;
> +
> +    virtual void createImageBacking(CoordinatedImageBackingID) = 0;
> +    virtual void updateImageBacking(CoordinatedImageBackingID, const
WebCoordinatedSurface::Handle&) = 0;
> +    virtual void clearImageBackingContents(CoordinatedImageBackingID) = 0;
> +    virtual void removeImageBacking(CoordinatedImageBackingID) = 0;
> +
> +    virtual void didRenderFrame(const WebCore::IntSize& contentsSize, const
WebCore::IntRect& coveredRect) = 0;
> +    virtual void didChangeScrollPosition(const WebCore::FloatPoint&
position) = 0;
> +
> +#if USE(GRAPHICS_SURFACE)
> +    virtual void createCanvas(CoordinatedLayerID, const WebCore::IntSize&,
const WebCore::GraphicsSurfaceToken&) = 0;
> +    virtual void syncCanvas(CoordinatedLayerID, uint32_t frontBuffer) = 0;
> +    virtual void destroyCanvas(CoordinatedLayerID) = 0;
> +#endif
> +
> +    virtual void setLayerAnimations(CoordinatedLayerID, const
WebCore::GraphicsLayerAnimations&) = 0;
> +    virtual void setAnimationsLocked(bool) = 0;
> +};

I'd rather have these as operations; e.g.
CompositingOperations::RemoveTileForLayer::create(CoordinatedLayerID, uint32_t)

with a deepCopy function for threading.

and then 
WebCompositingOperations::encode/decode