[webkit-reviews] review granted: [Bug 88813] Change RenderTableSection::calcRowLogicalHeight to round rather than floor height : [Attachment 146932] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Emil A Eklund
<eae at chromium.org>'s request for review:
Bug 88813: Change RenderTableSection::calcRowLogicalHeight to round rather than
floor height
https://bugs.webkit.org/show_bug.cgi?id=88813

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146932&action=review


> LayoutTests/ChangeLog:8
> +	   Add new test ensuring that rows are painted with the desired height
when

It's not really 'painted' more 'laid out'.

> LayoutTests/fast/sub-pixel/table-rows-have-stable-height.html:45
> +	   </p>

Nit: I like to have the bug number in the output.

>>> LayoutTests/fast/sub-pixel/table-rows-have-stable-height.html:65
>>> +			 var height = r((20 + i) * 0.93 + i);
>> 
>> Where did you find this formula? The whole test cases is based on this being
right, yet it's super cryptic.
> 
> The formula isn't critical at all, it is just a way to have each row be a
different height with subpixel precision.

I think a comment on why we are doing that would be welcome. Something like:

// Set the size to a non-pixel bound value. The exact formula isn't important,
what matters is that each rows has a different size.