[webkit-reviews] review granted: [Bug 39461] Need to implement Tiling for large compositing layers : [Attachment 56766] Replacement patch with more issues addressed

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 39461: Need to implement Tiling for large compositing layers
https://bugs.webkit.org/show_bug.cgi?id=39461

Attachment 56766: Replacement patch with more issues addressed
https://bugs.webkit.org/attachment.cgi?id=56766&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/platform/graphics/win/WKCACFLayer.h
> ===================================================================

> +    void setNeedsDisplay(const CGRect* dirtyRect)
> +    {
> +	   internalSetNeedsDisplay(dirtyRect);
> +	   setNeedsCommit();
> +    }
> +
> +    void setNeedsDisplay() { setNeedsDisplay(0); }

Why not just void setNeedsDisplay(const CGRect* dirtyRect = 0)? No need for
another method.

> Index: WebCore/platform/graphics/win/WebTiledLayer.cpp
> ===================================================================

> +WebTiledLayer::WebTiledLayer(const CGSize& tileSize, GraphicsLayer* owner)
> +    : WebLayer(WKCACFLayer::Layer, owner)
> +{
> +    // Tiled layers are placed in a child layer that is always the first
child of the TiledLayer
> +    RetainPtr<CACFLayerRef> tileParent(AdoptCF,
CACFLayerCreate(kCACFLayer));
> +    CACFLayerInsertSublayer(layer(), tileParent.get(), 0);
> +
> +    m_tileSize = tileSize;

Use an intializer: m_tileSize(tileSize).

> +CACFLayerRef WebTiledLayer::tileParent() const
> +{
> +    CFArrayRef sublayers = CACFLayerGetSublayers(layer());
> +    ASSERT(sublayers && CFArrayGetCount(sublayers) > 0);
> +    return
static_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sublayers,
0)));
> +}

I think it's worth keeping tileParent in a member variable. Up to you.
r=me