Bug 82182

Summary:

Revert linesBoundingBox to integers

Product:

WebKit

Reporter:

Levi Weintraub <leviw>

Component:

Layout and Rendering

Assignee:

Levi Weintraub <leviw>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, eae, eric, jchaffraix, vivekjkumar266, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

60318

Attachments:

Description	Flags
Patch	none

Description Levi Weintraub 2012-03-26 03:12:33 PDT

Moving linesBoundingBox back to integers. We don't pixel snap values from the inline box tree, we enclose them. Keeping linesBoundingBox as integers doesn't change its accuracy, and prevents misuse.

Comment 1 Levi Weintraub 2012-03-26 03:22:56 PDT

Created attachment 133763 [details]
Patch

Comment 2 Eric Seidel (no email) 2012-03-26 15:38:08 PDT

Comment on attachment 133763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133763&action=review

> Source/WebCore/ChangeLog:12
> +        LayoutRects are intended to be pixel snapped to determine the ultimate screen
> +        coordinates, but the Inline Box tree is laid out using floats, and pixel snapping
> +        the resulting box from linesBoundingBox would produce a potentially incorrect
> +        rectangle. Keeping this using enclosingIntRect retains the previous accuracy and
> +        prevents misuse.

Clearly you've learned a lot as youv'e gone.  I hope that we've documented this learning for others.  To help make it obvious where LayoutPoint vs. IntPoitn vs. FloatPoint should be used.  It's still not always 100% obvious to me. :(

Comment 3 Levi Weintraub 2012-03-27 02:40:30 PDT

Comment on attachment 133763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133763&action=review

>> Source/WebCore/ChangeLog:12
>> +        prevents misuse.
> 
> Clearly you've learned a lot as youv'e gone.  I hope that we've documented this learning for others.  To help make it obvious where LayoutPoint vs. IntPoitn vs. FloatPoint should be used.  It's still not always 100% obvious to me. :(

Looks like I've got a few things to clear up on the Wiki. I'll give it some more love today. I appreciate the feedback.

Comment 4 WebKit Review Bot 2012-03-27 03:07:07 PDT

Comment on attachment 133763 [details]
Patch

Clearing flags on attachment: 133763

Committed r112237: <http://trac.webkit.org/changeset/112237>

Comment 5 WebKit Review Bot 2012-03-27 03:07:12 PDT

All reviewed patches have been landed.  Closing bug.