While we eventually layout table parts using integers, we should still keep track of baseline positions (which are combined with sub-pixel padding values) in sub-pixel units.
Updating the bug title since there's only one class this actually needs to happen in.
Created attachment 135312 [details] Patch
Comment on attachment 135312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135312&action=review > Source/WebCore/rendering/RenderTableSection.cpp:391 > + m_rowPos[r + 1] = max<int>(m_rowPos[r + 1], m_grid[r].baseline + baselineDescent); It looks unfortunate that m_rowPos is actually using an integer here.
(In reply to comment #3) > (From update of attachment 135312 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135312&action=review > > > Source/WebCore/rendering/RenderTableSection.cpp:391 > > + m_rowPos[r + 1] = max<int>(m_rowPos[r + 1], m_grid[r].baseline + baselineDescent); > > It looks unfortunate that m_rowPos is actually using an integer here. If it makes the code simpler and it reduces the number of conversions we could change m_rowPos to a LayoutUnit. It'll always hold an integer value but we could use an ASSERT (or even the test results I suppose, this code has good coverage) to ensure that.
Comment on attachment 135312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135312&action=review >>> Source/WebCore/rendering/RenderTableSection.cpp:391 >>> + m_rowPos[r + 1] = max<int>(m_rowPos[r + 1], m_grid[r].baseline + baselineDescent); >> >> It looks unfortunate that m_rowPos is actually using an integer here. > > If it makes the code simpler and it reduces the number of conversions we could change m_rowPos to a LayoutUnit. It'll always hold an integer value but we could use an ASSERT (or even the test results I suppose, this code has good coverage) to ensure that. I don't think that's worth it for now, considering that the rest of the table code is using int. However it sounds like we should be considering switching table layout to LayoutUnit at some point in the future.
Comment on attachment 135312 [details] Patch Landed in http://trac.webkit.org/changeset/113162