In RenderTableCell::logicalHeightForRowSizing the adjustedLogicalHeight is calculated from the logicalHeight and intrinsic padding and is then returned and floored. This can cause cause the cell to be slightly smaller (0.5px) than the the element it contains.
Downstream chromium bug: https://code.google.com/p/chromium/issues/detail?id=180747
Created attachment 192105 [details] Patch
Comment on attachment 192105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192105&action=review > Source/WebCore/rendering/RenderTableCell.h:93 > LayoutUnit logicalHeightForRowSizing() const Looks like this should be an integer, and that would make using pixelSnappedLogicalHeight make more sense.
Created attachment 192107 [details] Patch
Comment on attachment 192107 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192107&action=review > Source/WebCore/rendering/RenderTableCell.h:101 > - styleLogicalHeight += computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter(); > + styleLogicalHeight += (computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter()).floor(); wouldn't (computedCSSPaddingBefore() + computedCSSPaddingAfter().floor() + borderBefore() + borderAfter() be more efficient?
(In reply to comment #5) > (From update of attachment 192107 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192107&action=review > > > Source/WebCore/rendering/RenderTableCell.h:101 > > - styleLogicalHeight += computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter(); > > + styleLogicalHeight += (computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter()).floor(); > > wouldn't (computedCSSPaddingBefore() + computedCSSPaddingAfter().floor() + borderBefore() + borderAfter() be more efficient? (computedCSSPaddingBefore() + computedCSSPaddingAfter()).floor() rather
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 192107 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=192107&action=review > > > > > Source/WebCore/rendering/RenderTableCell.h:101 > > > - styleLogicalHeight += computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter(); > > > + styleLogicalHeight += (computedCSSPaddingBefore() + computedCSSPaddingAfter() + borderBefore() + borderAfter()).floor(); > > > > wouldn't (computedCSSPaddingBefore() + computedCSSPaddingAfter().floor() + borderBefore() + borderAfter() be more efficient? > > (computedCSSPaddingBefore() + computedCSSPaddingAfter()).floor() rather Good idea.
Created attachment 192112 [details] Patch
Comment on attachment 192112 [details] Patch Thanks for the quick review!
Committed r145242: <http://trac.webkit.org/changeset/145242>
This caused subtle changes in lots of tests of the form: --- /Volumes/data/b/build/slave/WebKit_Mac10_8/build/layout-test-results/css1/basic/inheritance-expected.txt +++ /Volumes/data/b/build/slave/WebKit_Mac10_8/build/layout-test-results/css1/basic/inheritance-actual.txt @@ -1,8 +1,8 @@ -layer at (0,0) size 785x730 +layer at (0,0) size 785x731 RenderView at (0,0) size 785x600 -layer at (0,0) size 785x730 - RenderBlock {HTML} at (0,0) size 785x730 - RenderBody {BODY} at (8,8) size 769x714 [color=#008000] [bgcolor=#CCCCCC] +layer at (0,0) size 785x731 + RenderBlock {HTML} at (0,0) size 785x731 + RenderBody {BODY} at (8,8) size 769x715 [color=#008000] [bgcolor=#CCCCCC] or very small changes in the scrollbar's height. It looks like these are expected for this sort of change. Shall I just rebaseline?
(In reply to comment #11) > or very small changes in the scrollbar's height. It looks like these are expected for this sort of change. Shall I just rebaseline? All expected. I included new linux baselines in the patch and will rebaseline for other ports once the bots cycle.