83017 2012-04-03 04:45:29 -0700 Switch baseline values to LayoutUnits in RenderTableSection. 2012-04-04 03:39:10 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 60318 1 leviw leviw eae eric jchaffraix oldest_to_newest 594373 0 leviw 2012-04-03 04:45:29 -0700 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. 594376 1 leviw 2012-04-03 04:54:21 -0700 Updating the bug title since there's only one class this actually needs to happen in. 594380 2 135312 leviw 2012-04-03 05:01:12 -0700 Created attachment 135312 Patch 594869 3 135312 jchaffraix 2012-04-03 14:08:17 -0700 Comment on attachment 135312 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. 594945 4 eae 2012-04-03 15:16:43 -0700 (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. 594951 5 135312 jchaffraix 2012-04-03 15:24:56 -0700 Comment on attachment 135312 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. 595381 6 135312 leviw 2012-04-04 03:38:27 -0700 Comment on attachment 135312 Patch Landed in http://trac.webkit.org/changeset/113162 135312 2012-04-03 05:01:12 -0700 2012-04-04 03:38:27 -0700 Patch bug-83017-20120403140110.patch text/plain 4345 leviw SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExMzAwOSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBACisyMDEyLTA0LTAzICBMZXZpIFdl aW50cmF1YiAgPGxldml3QGNocm9taXVtLm9yZz4KKworICAgICAgICBTd2l0Y2ggYmFzZWxpbmUg dmFsdWVzIHRvIExheW91dFVuaXRzIGluIFJlbmRlclRhYmxlU2VjdGlvbi4KKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgzMDE3CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2UgaW5pdGlhbGx5IGludGVuZCB0 byBrZWVwIHRhYmxlIGxheW91dCBvbiBpbnRlZ2VycyBwb3N0IHN3aXRjaGluZyB0aGUgcmVuZGVy IHRyZWUgdG8gcHJpbWFyaWx5CisgICAgICAgIHVzZSBzdWItcGl4ZWwgcHJlY2lzaW9uLiBCYXNl bGluZSBwb3NpdGlvbnMgc3dpdGNoIHRvIHN1Yi1waXhlbCB1bml0cywgYW5kIGluIHRhYmxlIGxh eW91dCBjb2RlIGFyZQorICAgICAgICBjb21iaW5lZCB3aXRoIHBhZGRpbmcgKGFsc28gc3ViLXBp eGVsKSwgd2hpY2ggbWVhbnMgd2UgaWRlYWxseSB3b24ndCB0cnVuY2F0ZSB0aGVpciB2YWx1ZXMg dW50aWwKKyAgICAgICAgYWZ0ZXIgdGhleSdyZSBhZGRlZCB0b2dldGhlci4gQ29udmVydGluZyBi YXNlbGluZSB2YWx1ZXMgaW4gUmVuZGVyVGFibGVTZWN0aW9uIHRvIExheW91dFVuaXRzLgorCisg ICAgICAgIE5vIG5ldyB0ZXN0cy4gTm8gY2hhbmdlIGluIGJlaGF2aW9yLgorCisgICAgICAgICog cmVuZGVyaW5nL1JlbmRlclRhYmxlU2VjdGlvbi5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5k ZXJUYWJsZVNlY3Rpb246OmNhbGNSb3dMb2dpY2FsSGVpZ2h0KToKKyAgICAgICAgKFdlYkNvcmU6 OlJlbmRlclRhYmxlU2VjdGlvbjo6bGF5b3V0Um93cyk6CisgICAgICAgICogcmVuZGVyaW5nL1Jl bmRlclRhYmxlU2VjdGlvbi5oOgorICAgICAgICAoUm93U3RydWN0KToKKwogMjAxMi0wNC0wMyAg Tmlrb2xhcyBaaW1tZXJtYW5uICA8bnppbW1lcm1hbm5AcmltLmNvbT4KIAogICAgICAgICBFbmFi bGUgYW5pbVZhbCBzdXBwb3J0IGZvciBTVkdBbmltYXRlZEVudW1lcmF0aW9uCkluZGV4OiBTb3Vy Y2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGFibGVTZWN0aW9uLmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGFibGVTZWN0aW9uLmNwcAkocmV2aXNp b24gMTEyODQ4KQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRhYmxlU2VjdGlv bi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMzNiw3ICszMzYsNyBAQCBpbnQgUmVuZGVyVGFibGVT ZWN0aW9uOjpjYWxjUm93TG9naWNhbEhlCiAKICAgICBmb3IgKHVuc2lnbmVkIHIgPSAwOyByIDwg bV9ncmlkLnNpemUoKTsgcisrKSB7CiAgICAgICAgIG1fZ3JpZFtyXS5iYXNlbGluZSA9IDA7Ci0g ICAgICAgIGludCBiYXNlbGluZURlc2NlbnQgPSAwOworICAgICAgICBMYXlvdXRVbml0IGJhc2Vs aW5lRGVzY2VudCA9IDA7CiAKICAgICAgICAgLy8gT3VyIGJhc2Ugc2l6ZSBpcyB0aGUgYmlnZ2Vz dCBsb2dpY2FsIGhlaWdodCBmcm9tIG91ciBjZWxscycgc3R5bGVzIChleGNsdWRpbmcgcm93IHNw YW5uaW5nIGNlbGxzKS4KICAgICAgICAgbV9yb3dQb3NbciArIDFdID0gbWF4KG1fcm93UG9zW3Jd ICsgbWluaW11bVZhbHVlRm9yTGVuZ3RoKG1fZ3JpZFtyXS5sb2dpY2FsSGVpZ2h0LCAwLCB2aWV3 UmVuZGVyZXIpLCAwKTsKQEAgLTM3Nyw3ICszNzcsNyBAQCBpbnQgUmVuZGVyVGFibGVTZWN0aW9u OjpjYWxjUm93TG9naWNhbEhlCiAgICAgICAgICAgICAvLyBmaW5kIG91dCB0aGUgYmFzZWxpbmUK ICAgICAgICAgICAgIEVWZXJ0aWNhbEFsaWduIHZhID0gY2VsbC0+c3R5bGUoKS0+dmVydGljYWxB bGlnbigpOwogICAgICAgICAgICAgaWYgKHZhID09IEJBU0VMSU5FIHx8IHZhID09IFRFWFRfQk9U VE9NIHx8IHZhID09IFRFWFRfVE9QIHx8IHZhID09IFNVUEVSIHx8IHZhID09IFNVQikgewotICAg ICAgICAgICAgICAgIGludCBiYXNlbGluZVBvc2l0aW9uID0gY2VsbC0+Y2VsbEJhc2VsaW5lUG9z aXRpb24oKTsKKyAgICAgICAgICAgICAgICBMYXlvdXRVbml0IGJhc2VsaW5lUG9zaXRpb24gPSBj ZWxsLT5jZWxsQmFzZWxpbmVQb3NpdGlvbigpOwogICAgICAgICAgICAgICAgIGlmIChiYXNlbGlu ZVBvc2l0aW9uID4gY2VsbC0+Ym9yZGVyQmVmb3JlKCkgKyBjZWxsLT5wYWRkaW5nQmVmb3JlKCkp IHsKICAgICAgICAgICAgICAgICAgICAgbV9ncmlkW3JdLmJhc2VsaW5lID0gbWF4KG1fZ3JpZFty XS5iYXNlbGluZSwgYmFzZWxpbmVQb3NpdGlvbiAtIGNlbGwtPmludHJpbnNpY1BhZGRpbmdCZWZv cmUoKSk7CiAgICAgICAgICAgICAgICAgICAgIGJhc2VsaW5lRGVzY2VudCA9IG1heChiYXNlbGlu ZURlc2NlbnQsIG1fcm93UG9zW2NlbGxTdGFydFJvd10gKyBjZWxsTG9naWNhbEhlaWdodCAtIChi YXNlbGluZVBvc2l0aW9uIC0gY2VsbC0+aW50cmluc2ljUGFkZGluZ0JlZm9yZSgpKSk7CkBAIC0z ODgsNyArMzg4LDcgQEAgaW50IFJlbmRlclRhYmxlU2VjdGlvbjo6Y2FsY1Jvd0xvZ2ljYWxIZQog ICAgICAgICAvLyBkbyB3ZSBoYXZlIGJhc2VsaW5lIGFsaWduZWQgZWxlbWVudHM/CiAgICAgICAg IGlmIChtX2dyaWRbcl0uYmFzZWxpbmUpCiAgICAgICAgICAgICAvLyBpbmNyZWFzZSByb3doZWln aHQgaWYgYmFzZWxpbmUgcmVxdWlyZXMKLSAgICAgICAgICAgIG1fcm93UG9zW3IgKyAxXSA9IG1h eChtX3Jvd1Bvc1tyICsgMV0sIG1fZ3JpZFtyXS5iYXNlbGluZSArIGJhc2VsaW5lRGVzY2VudCk7 CisgICAgICAgICAgICBtX3Jvd1Bvc1tyICsgMV0gPSBtYXg8aW50PihtX3Jvd1Bvc1tyICsgMV0s IG1fZ3JpZFtyXS5iYXNlbGluZSArIGJhc2VsaW5lRGVzY2VudCk7CiAKICAgICAgICAgLy8gQWRk IHRoZSBib3JkZXItc3BhY2luZyB0byBvdXIgZmluYWwgcG9zaXRpb24uCiAgICAgICAgIG1fcm93 UG9zW3IgKyAxXSArPSBtX2dyaWRbcl0ucm93UmVuZGVyZXIgPyBzcGFjaW5nIDogMDsKQEAgLTYy MCw3ICs2MjAsNyBAQCB2b2lkIFJlbmRlclRhYmxlU2VjdGlvbjo6bGF5b3V0Um93cygpCiAgICAg ICAgICAgICAgICAgLy8gSWYgdGhlIGJhc2VsaW5lIG1vdmVkLCB3ZSBtYXkgaGF2ZSB0byB1cGRh dGUgdGhlIGRhdGEgZm9yIG91ciByb3cuIEZpbmQgb3V0IHRoZSBuZXcgYmFzZWxpbmUuCiAgICAg ICAgICAgICAgICAgRVZlcnRpY2FsQWxpZ24gdmEgPSBjZWxsLT5zdHlsZSgpLT52ZXJ0aWNhbEFs aWduKCk7CiAgICAgICAgICAgICAgICAgaWYgKHZhID09IEJBU0VMSU5FIHx8IHZhID09IFRFWFRf Qk9UVE9NIHx8IHZhID09IFRFWFRfVE9QIHx8IHZhID09IFNVUEVSIHx8IHZhID09IFNVQikgewot ICAgICAgICAgICAgICAgICAgICBpbnQgYmFzZWxpbmUgPSBjZWxsLT5jZWxsQmFzZWxpbmVQb3Np dGlvbigpOworICAgICAgICAgICAgICAgICAgICBMYXlvdXRVbml0IGJhc2VsaW5lID0gY2VsbC0+ Y2VsbEJhc2VsaW5lUG9zaXRpb24oKTsKICAgICAgICAgICAgICAgICAgICAgaWYgKGJhc2VsaW5l ID4gY2VsbC0+Ym9yZGVyQmVmb3JlKCkgKyBjZWxsLT5wYWRkaW5nQmVmb3JlKCkpCiAgICAgICAg ICAgICAgICAgICAgICAgICBtX2dyaWRbcl0uYmFzZWxpbmUgPSBtYXgobV9ncmlkW3JdLmJhc2Vs aW5lLCBiYXNlbGluZSk7CiAgICAgICAgICAgICAgICAgfQpJbmRleDogU291cmNlL1dlYkNvcmUv cmVuZGVyaW5nL1JlbmRlclRhYmxlU2VjdGlvbi5oCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL3JlbmRlcmluZy9SZW5kZXJUYWJsZVNlY3Rpb24uaAkocmV2aXNpb24gMTEyODQ4KQorKysg U291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRhYmxlU2VjdGlvbi5oCSh3b3JraW5nIGNv cHkpCkBAIC0xMTAsNyArMTEwLDcgQEAgcHVibGljOgogCiAgICAgICAgIFJvdyByb3c7CiAgICAg ICAgIFJlbmRlclRhYmxlUm93KiByb3dSZW5kZXJlcjsKLSAgICAgICAgaW50IGJhc2VsaW5lOwor ICAgICAgICBMYXlvdXRVbml0IGJhc2VsaW5lOwogICAgICAgICBMZW5ndGggbG9naWNhbEhlaWdo dDsKICAgICB9OwogCg==