[webkit-reviews] review granted: [Bug 78054] Add pixelSnappedIntRect method : [Attachment 126387] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Levi Weintraub <leviw at chromium.org>'s
request for review:
Bug 78054: Add pixelSnappedIntRect method
https://bugs.webkit.org/show_bug.cgi?id=78054

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

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


I'm willing to aprove this and let you all move foward.  But I think you/we
shoudl focus on making the boundaries crystal clear.  It's less important that
we get each spot right, more than that it's easy to tell when a spot is right
or wrong.  Right now it's very difficult to tell what should be a layout rect
vs. an IntRect because the rules are not clear (at least to me).  We need to
make them clear.  NOt just for me, but for all the others who are going to
touch this code and be confused as to what type of rect to use where.

> Source/WebCore/page/FrameView.cpp:1777
> +		   unionedRect.unite(pixelSnappedIntRect(m_repaintRects[i]));

Isn't a repaint rect a pixel rect by definition already?  And thus an IntRect
and not a LayoutRect?

> Source/WebCore/rendering/RenderObject.cpp:968
> -		   graphicsContext->drawRect(IntRect(x1, y1, x2 - x1, y2 -
y1));
> +		   graphicsContext->drawRect(pixelSnappedIntRect(LayoutRect(x1,
y1, x2 - x1, y2 - y1)));

Feels a little odd that we convert from int to layout to int here...