Thanks to abarth for bringing this to my attention. We incorrectly started ceiling instead of truncating some values when using integer layout. The fix is easy but unfortunate... I'll upload momentarily.
Created attachment 158432 [details] Patch
Comment on attachment 158432 [details] Patch This doesn't make sense to me. Why do we truncate in one case and ceil in teh other?
(In reply to comment #2) > (From update of attachment 158432 [details]) > This doesn't make sense to me. Why do we truncate in one case and ceil in teh other? Because we're actually ceiling to the next LayoutUnit when sub-pixel layout is enabled, which ensures that we have enough room for margins for example when actually laying out the lines in the block. When we don't have sub-pixel layout enabled, we truncate in both cases.
That explanation still doesn't make sense to me. With layout units, we have to be careful when to ciel vs. when to floor. Why is the old code, which always operates on integers correct to floor here?
(In reply to comment #4) > That explanation still doesn't make sense to me. With layout units, we have to be careful when to ciel vs. when to floor. Why is the old code, which always operates on integers correct to floor here? It comes down to safely mirroring what we do at layout time. When we layout with sub-pixel on, we need to make sure we've set preferred widths that contain the error of the content, hence the ceiling. When we layout without sub-pixel, we truncate. Hence, we have to truncate here as well.
So what you're saying is that in the non-subpixel case, the rule is to *always* truncate, everywhere. Is that correct? But with subpixel enabled, things are slightly more tricky. If so, we should update the comment to explain why "adjust" ciels in one case and truncates in the other. Similarly, since subpixel has to be careful when to ciel vs. truncate, we should probably explain when it's OK to use that "adjust" method.
(In reply to comment #6) > So what you're saying is that in the non-subpixel case, the rule is to *always* truncate, everywhere. Is that correct? But with subpixel enabled, things are slightly more tricky. > > If so, we should update the comment to explain why "adjust" ciels in one case and truncates in the other. Similarly, since subpixel has to be careful when to ciel vs. truncate, we should probably explain when it's OK to use that "adjust" method. Naming is hard. It's very duplicitous behavior, given it's flooring in one case, and finding a LayoutUnit that's at least as big as the float in another. I'd love a suggestion, if you have one, for how to name this thing. We don't always truncate everywhere. We ceil the width of lines to contain text for blocks (see updatePreferredWidth), but not for Lengths, even when those lengths are used in the line box tree.
Created attachment 158460 [details] Patch
Comment on attachment 158460 [details] Patch OK.
Comment on attachment 158460 [details] Patch Clearing flags on attachment: 158460 Committed r125632: <http://trac.webkit.org/changeset/125632>
All reviewed patches have been landed. Closing bug.