Bug 88918 – Cast paddings to int in RenderTableCell

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Emil A Eklund]

Table layout uses integers throughout (as documented on https://trac.webkit.org/wiki/LayoutUnit) yet the TableCell paddingLeft/Right/Top/Bottom methods returns LayoutUnits. This causes inconsistent rounding as some call sites cast the numbers to ints before doing computation and others do computation before casting. By changing the methods to always cast the padding values to int we ensure consistent padding calculations. Ideally we'd change the type of the return value for the methods but as they are overriden that would likely cause more confusion.

[Emil A Eklund]

Created attachment 147165 [details]
Patch

[Emil A Eklund]

FYI: The chromium test expectation changes here (apart from the new test) reflects us going back to rendering that more closely matches the other platforms (and in most cases matches how they looked in chromium before we turned on subpixel layout).

[Levi Weintraub]

Comment on attachment 147165 [details]
Patch

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

You need to also update the test expectations files to prevent the bots from going red on non-linux platforms. With that r=me.

> Source/WebCore/rendering/RenderTableCell.cpp:228
> -    return computedCSSPaddingBefore() + intrinsicPaddingBefore();
> +    return static_cast<int>(computedCSSPaddingBefore() + intrinsicPaddingBefore());

Only casting the computedCSSPaddingBefore would save unnecessarily promoting intrinsicPaddingBefore to a LayoutUnit.

[Emil A Eklund]

(In reply to comment #3)
> (From update of attachment 147165 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147165&action=review
> 
> You need to also update the test expectations files to prevent the bots from going red on non-linux platforms. With that r=me.

Will do, I really wish there was a better way to do this.

> 
> > Source/WebCore/rendering/RenderTableCell.cpp:228
> > -    return computedCSSPaddingBefore() + intrinsicPaddingBefore();
> > +    return static_cast<int>(computedCSSPaddingBefore() + intrinsicPaddingBefore());
> 
> Only casting the computedCSSPaddingBefore would save unnecessarily promoting intrinsicPaddingBefore to a LayoutUnit.

Ah, good idea.

[Emil A Eklund]

Committed r120403: <http://trac.webkit.org/changeset/120403>