[webkit-reviews] review granted: [Bug 93475] Floored and truncated rounding confused : [Attachment 157211] Patch

[bugzilla-daemon at webkit.org]

Levi Weintraub <leviw at chromium.org> has granted Allan Sandfeld Jensen
<allan.jensen at nokia.com>'s request for review:
Bug 93475: Floored and truncated rounding confused
https://bugs.webkit.org/show_bug.cgi?id=93475

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

------- Additional Comments from Levi Weintraub <leviw at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=157211&action=review


> Source/WebCore/ChangeLog:12
> +	   This patch changes several instances of floored rounding to do
actual floored
> +	   rounding, and a fixes and cleans up several places using floored
rounding.

What are you saying this changes? I'm not sure what "actual" means here.

> Source/WebCore/platform/graphics/FractionalLayoutRect.cpp:134
> -    float x = floorf(rect.x().toFloat());
> -    float y = floorf(rect.y().toFloat());
> -    float width = ceilf(rect.maxX().toFloat()) - x;
> -    float height = ceilf(rect.maxY().toFloat()) - y;
> +    IntPoint location = flooredIntPoint(rect.minXMinYCorner());
> +    IntPoint maxPoint = ceiledIntPoint(rect.maxXMaxYCorner());
>  
> -    return IntRect(clampToInteger(x), clampToInteger(y), 
> -		      clampToInteger(width), clampToInteger(height));
> +    return IntRect(location, maxPoint - location);

So much cleaner, well done!

> Source/WebCore/rendering/RenderReplaced.cpp:291
> -	       m_intrinsicSize = flooredIntSize(intrinsicSize); // FIXME: This
introduces precision errors. We should convert m_intrinsicSize to be a float.
> +	       m_intrinsicSize = IntSize(intrinsicSize.width(),
intrinsicSize.height()); // FIXME: This introduces precision errors. We should
convert m_intrinsicSize to be a float.

Why this change is correct isn't obvious to me. Since intrinsic size is never
negative this doesn't seem to change any behavior. How about an explanation in
the changelog?