[webkit-reviews] review granted: [Bug 60409] Convert x, y and width, height pairs to IntPoint and IntSize for RenderLayer : [Attachment 92670] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Emil A Eklund <eae at chromium.org>'s
request for review:
Bug 60409: Convert x,y and width,height pairs to IntPoint and IntSize for
RenderLayer
https://bugs.webkit.org/show_bug.cgi?id=60409

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92670&action=review

Looks great to me!  But I think we should leave Simon (or someone else familiar
with layers) a day to comment if they care.

> Source/WebCore/rendering/RenderLayer.cpp:722
> +	   m_relativeOffset.setWidth(0);
> +	   m_relativeOffset.setHeight(0);

You could just do m_relativeOffset = IntSize(), no?  Or maybe intSize() has a
clear() method?  I figure this can be expressed in one line regardless. :)

> Source/WebCore/rendering/RenderLayer.h:215
> +    int x() const { return m_topLeft.x(); }
> +    int y() const { return m_topLeft.y(); }

I feel like there was some concern brought up about using "topLeft" in the
InlineBox patch I wrote like this... something about how things might not
always be "top" or "left".   But that may not apply to layers... I think it was
relating to vertical text.

> Source/WebCore/rendering/RenderLayer.h:219
> +	   m_topLeft.setX(x);
> +	   m_topLeft.setY(y);

I suspect there is a one-line equivalent.  m_topLeft = IntPoint(x, y); would be
one way. :)