86665 2012-05-16 12:33:08 -0700 Use pixelSnappedLogicalHeight for TableSection layout 2012-05-16 20:36:44 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 85532 1 leviw leviw eae eric webkit.review.bot oldest_to_newest 625745 0 leviw 2012-05-16 12:33:08 -0700 Previously, we were using logicalHeight but storing the value as an int in RenderTableSection. We *want* to use ints, but we need to pixel snap the value. 625780 1 142329 leviw 2012-05-16 13:16:26 -0700 Created attachment 142329 Patch 626105 2 142329 eric 2012-05-16 18:27:31 -0700 Comment on attachment 142329 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142329&action=review > Source/WebCore/rendering/RenderBox.h:77 > + int pixelSnappedLogicalHeight() const { return style()->isHorizontalWritingMode() ? pixelSnappedHeight() : pixelSnappedWidth(); } > + int pixelSnappedLogicalWidth() const { return style()->isHorizontalWritingMode() ? pixelSnappedWidth() : pixelSnappedHeight(); } So sad that we have all these redundant accessors. One would think that a logicalSize() might make more sense. :) 626112 3 142329 leviw 2012-05-16 18:29:07 -0700 Comment on attachment 142329 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142329&action=review >> Source/WebCore/rendering/RenderBox.h:77 >> + int pixelSnappedLogicalWidth() const { return style()->isHorizontalWritingMode() ? pixelSnappedWidth() : pixelSnappedHeight(); } > > So sad that we have all these redundant accessors. One would think that a logicalSize() might make more sense. :) Thanks for the review! I smell some potential cleanup :) 626199 4 142329 webkit.review.bot 2012-05-16 20:36:39 -0700 Comment on attachment 142329 Patch Clearing flags on attachment: 142329 Committed r117388: <http://trac.webkit.org/changeset/117388> 626200 5 webkit.review.bot 2012-05-16 20:36:44 -0700 All reviewed patches have been landed. Closing bug. 142329 2012-05-16 13:16:26 -0700 2012-05-16 20:36:39 -0700 Patch bug-86665-20120516131625.patch text/plain 2922 leviw SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExNzMyMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI0IEBACisyMDEyLTA1LTE2ICBMZXZpIFdl aW50cmF1YiAgPGxldml3QGNocm9taXVtLm9yZz4KKworICAgICAgICBVc2UgcGl4ZWxTbmFwcGVk TG9naWNhbEhlaWdodCBmb3IgVGFibGVTZWN0aW9uIGxheW91dAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODY2NjUKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZSBkbyB0YWJsZSBsYXlvdXQgdXNpbmcgaW50 ZWdlcnMgdG8gZW5zdXJlIHdlIGZvbGxvdyB0aGUgc3BlYyAoc2VlCisgICAgICAgIGh0dHBzOi8v dHJhYy53ZWJraXQub3JnL3dpa2kvTGF5b3V0VW5pdCBmb3IgZGV0YWlscyksIGJ1dCB3ZXJlIHBy ZXZpb3VzbHkKKyAgICAgICAgZmxvb3JpbmcgdGhlIGxvZ2ljYWwgaGVpZ2h0IHVzZWQgaW4gbGF5 b3V0Um93cy4gVGhpcyBjYXVzZWQgdXMgdG8gbWlzLQorICAgICAgICBjYWxjdWxhdGUgdGhlIGlu dHJpbnNpYyBwYWRkaW5nIGFuZCBncm93IGNlbGxzIGJleW9uZCB0aGVpciBwcm9wZXIgaGVpZ2h0 LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gRnVsbHkgY292ZXJlZCBieSBleGlzdGluZyB0ZXN0 cyB3aXRoIHN1Yi1waXhlbCBlbmFibGVkLgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlckJv eC5oOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyQm94OjpwaXhlbFNuYXBwZWRMb2dpY2FsSGVp Z2h0KTogQWRkZWQgdGhlc2UgY29udmVuaWVuY2UKKyAgICAgICAgbWV0aG9kcy4KKyAgICAgICAg KFdlYkNvcmU6OlJlbmRlckJveDo6cGl4ZWxTbmFwcGVkTG9naWNhbFdpZHRoKToKKyAgICAgICAg KiByZW5kZXJpbmcvUmVuZGVyVGFibGVTZWN0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJl bmRlclRhYmxlU2VjdGlvbjo6bGF5b3V0Um93cyk6CisKIDIwMTItMDUtMTYgIEFuZHJlYXMgS2xp bmcgIDxrbGluZ0B3ZWJraXQub3JnPgogCiAgICAgICAgIEF2b2lkIHJlcGFyc2luZyB0aGUgc3R5 bGUgYXR0cmlidXRlIHdoZW4gY2xvbmluZyBlbGVtZW50cy4KSW5kZXg6IFNvdXJjZS9XZWJDb3Jl L3JlbmRlcmluZy9SZW5kZXJCb3guaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5k ZXJpbmcvUmVuZGVyQm94LmgJKHJldmlzaW9uIDExNzEwNCkKKysrIFNvdXJjZS9XZWJDb3JlL3Jl bmRlcmluZy9SZW5kZXJCb3guaAkod29ya2luZyBjb3B5KQpAQCAtNzMsNiArNzMsOSBAQCBwdWJs aWM6CiAgICAgTGF5b3V0VW5pdCBsb2dpY2FsV2lkdGgoKSBjb25zdCB7IHJldHVybiBzdHlsZSgp LT5pc0hvcml6b250YWxXcml0aW5nTW9kZSgpID8gd2lkdGgoKSA6IGhlaWdodCgpOyB9CiAgICAg TGF5b3V0VW5pdCBsb2dpY2FsSGVpZ2h0KCkgY29uc3QgeyByZXR1cm4gc3R5bGUoKS0+aXNIb3Jp em9udGFsV3JpdGluZ01vZGUoKSA/IGhlaWdodCgpIDogd2lkdGgoKTsgfQogCisgICAgaW50IHBp eGVsU25hcHBlZExvZ2ljYWxIZWlnaHQoKSBjb25zdCB7IHJldHVybiBzdHlsZSgpLT5pc0hvcml6 b250YWxXcml0aW5nTW9kZSgpID8gcGl4ZWxTbmFwcGVkSGVpZ2h0KCkgOiBwaXhlbFNuYXBwZWRX aWR0aCgpOyB9CisgICAgaW50IHBpeGVsU25hcHBlZExvZ2ljYWxXaWR0aCgpIGNvbnN0IHsgcmV0 dXJuIHN0eWxlKCktPmlzSG9yaXpvbnRhbFdyaXRpbmdNb2RlKCkgPyBwaXhlbFNuYXBwZWRXaWR0 aCgpIDogcGl4ZWxTbmFwcGVkSGVpZ2h0KCk7IH0KKwogICAgIHZvaWQgc2V0TG9naWNhbExlZnQo TGF5b3V0VW5pdCBsZWZ0KQogICAgIHsKICAgICAgICAgaWYgKHN0eWxlKCktPmlzSG9yaXpvbnRh bFdyaXRpbmdNb2RlKCkpCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGFi bGVTZWN0aW9uLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVu ZGVyVGFibGVTZWN0aW9uLmNwcAkocmV2aXNpb24gMTE3MTA0KQorKysgU291cmNlL1dlYkNvcmUv cmVuZGVyaW5nL1JlbmRlclRhYmxlU2VjdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTYxOCw3 ICs2MTgsNyBAQCB2b2lkIFJlbmRlclRhYmxlU2VjdGlvbjo6bGF5b3V0Um93cygpCiAKICAgICAg ICAgICAgIGludCBvbGRJbnRyaW5zaWNQYWRkaW5nQmVmb3JlID0gY2VsbC0+aW50cmluc2ljUGFk ZGluZ0JlZm9yZSgpOwogICAgICAgICAgICAgaW50IG9sZEludHJpbnNpY1BhZGRpbmdBZnRlciA9 IGNlbGwtPmludHJpbnNpY1BhZGRpbmdBZnRlcigpOwotICAgICAgICAgICAgaW50IGxvZ2ljYWxI ZWlnaHRXaXRob3V0SW50cmluc2ljUGFkZGluZyA9IGNlbGwtPmxvZ2ljYWxIZWlnaHQoKSAtIG9s ZEludHJpbnNpY1BhZGRpbmdCZWZvcmUgLSBvbGRJbnRyaW5zaWNQYWRkaW5nQWZ0ZXI7CisgICAg ICAgICAgICBpbnQgbG9naWNhbEhlaWdodFdpdGhvdXRJbnRyaW5zaWNQYWRkaW5nID0gY2VsbC0+ cGl4ZWxTbmFwcGVkTG9naWNhbEhlaWdodCgpIC0gb2xkSW50cmluc2ljUGFkZGluZ0JlZm9yZSAt IG9sZEludHJpbnNpY1BhZGRpbmdBZnRlcjsKIAogICAgICAgICAgICAgaW50IGludHJpbnNpY1Bh ZGRpbmdCZWZvcmUgPSAwOwogICAgICAgICAgICAgc3dpdGNoIChjZWxsLT5zdHlsZSgpLT52ZXJ0 aWNhbEFsaWduKCkpIHsK