[webkit-reviews] review granted: [Bug 61818] Switch RenderLayer::convertToLayerCoords to use IntPoint : [Attachment 95505] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Emil A Eklund
<eae at chromium.org>'s request for review:
Bug 61818: Switch RenderLayer::convertToLayerCoords to use IntPoint
https://bugs.webkit.org/show_bug.cgi?id=61818

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

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95505&action=review

This is a great change.  It really cleans up a lot of little annoying
expressions.

Please consider the various comments before landing.

> Source/WebCore/rendering/RenderLayer.cpp:920
> +	   transform.translate(delta.x(), delta.y());

It's too bad that translate doesn't understand IntPoint or IntSize...

> Source/WebCore/rendering/RenderLayer.cpp:1118
> +	   location += IntSize((int)absPos.x(), (int)absPos.y());

I wonder if creating some kind of toSize adaptor would be clearer here?

> Source/WebCore/rendering/RenderLayer.cpp:2565
> +	   transform.translateRight(delta.x(), delta.y());

Again, maybe transform.translateRight should have an IntPoint overload...

> Source/WebCore/rendering/RenderLayer.cpp:2915
> +	   transformState->translate(offset.x(), offset.y(),
HitTestingTransformState::AccumulateTransform);

Ditto translate functions and IntPoint.

> Source/WebCore/rendering/RenderLayerBacking.cpp:381
> +	   IntSize rendererOffset(parentClipRect.location().x() - delta.x(),
parentClipRect.location().y() - delta.y());

Isn't the parentClipRect.location() returning an IntPoint as well?  Maybe this
could be:

IntSize renderOffset = toSize(parentClipRect.location() - delta);

?