[webkit-reviews] review requested: [Bug 35833] Clean IntRect : [Attachment 50172] Inline IntRect::intersects() and the constructor taking a FloatRect

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin.poulain at nokia.com> has asked  for review:
Bug 35833: Clean IntRect
https://bugs.webkit.org/show_bug.cgi?id=35833

Attachment 50172: Inline IntRect::intersects() and the constructor taking a
FloatRect
https://bugs.webkit.org/attachment.cgi?id=50172&action=review

------- Additional Comments from Benjamin Poulain <benjamin.poulain at nokia.com>
(In reply to comment #5)
> What is the rationale for making the intersects function inline? Does it make

> the code size smaller or speed some operation up? Did you measure the
speedup?

Make the code smaller I don't think so, but it speed up the rendering a bit on
ARM.


> It is not appropriate to put a function like IntRect::scale inside an #ifdef.

> This is complicating, not simplifying, the IntRect class and the platform
> directory for little gain. It's a sort of "layering violation" to have these
> sorts of ifdefs and we should include them only if there is a massive benefit

> to doing so.
> 
> "I am tired of people complaining of the size of Webkit" is not sufficient
> reason to make a change like this that has a negligible effect on the size of

> WebKit.

Yep, totally, tiredness got me yesterday, that was a stupid change.
As I said, I am really only interested in the inlining. :)