[webkit-reviews] review granted: [Bug 63847] [chromium] Externalize layer visibility calculation : [Attachment 99526] Patch

[bugzilla-daemon at webkit.org]

Stephen White <senorblanco at chromium.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 63847: [chromium] Externalize layer visibility calculation
https://bugs.webkit.org/show_bug.cgi?id=63847

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

------- Additional Comments from Stephen White <senorblanco at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99526&action=review

I leave it up to you to decide if my comments are relevant here.

Looks good.  r=me.

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:156
> +    transform.scaleNonUniform(bounds().width() /
static_cast<double>(contentBounds().width()),
> +				 bounds().height() /
static_cast<double>(contentBounds().height()));

This could be a div-zero if contentBounds() has zero width or height.  Dunno if
that's an issue here.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:370
> +    transform.scaleNonUniform(bounds.width() /
static_cast<double>(contentBounds.width()),
> +				 bounds.height() /
static_cast<double>(contentBounds.height()));

Same as above; potential div-zero.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:449
> +		       maskLayer->setVisibleLayerRect(IntRect(IntPoint(),
maskLayer->contentBounds()));
> +		       maskLayer->paintContentsIfDirty();

This function-pair is called four times here; could potentially be refactored.