[webkit-reviews] review denied: [Bug 62313] RenderEmbeddedObject::getReplacementTextGeometry : [Attachment 96484] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Emil A Eklund <eae at chromium.org>'s
request for review:
Bug 62313: RenderEmbeddedObject::getReplacementTextGeometry
https://bugs.webkit.org/show_bug.cgi?id=62313

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

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

Otherwise seems OK.

> Source/WebCore/platform/graphics/FloatRect.h:111
> +    void moveBy(const IntPoint& delta) { m_location.move(delta.x(),
delta.y()); }

You should put a comment next to the IntPoint moveby as to why it's desired.

Something like
// We have an IntPoint version of moveBy to avoid unecessary float->int
conversion when moving by an IntPoint.

Actually, I don't see how your version helps at all?

We already do the float to int conversion in the move( ) call, why not just do
it once with the INtPoint->FloatPoint implicit converstion.

I think we should remove your moveBy(IntPoint).