91061 2012-07-12 02:17:02 -0700 Incorrect rounding in ceiledLayoutUnit and roundedLayoutUnit 2012-07-25 09:45:43 -0700 1 1 1 Unclassified WebKit Layout and Rendering 420+ Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 allan.jensen allan.jensen abarth eae eric leviw webkit.review.bot oldest_to_newest 667548 0 allan.jensen 2012-07-12 02:17:02 -0700 When subpixel layout is enabled, the two functions ceiledLayoutUnit and roundedLayoutUnit both returns the flooredLayoutUnit instead of ceiled or rounded values. The function FractionalUnit::fromFloatCeil was recently added to solve the same problem in updatePreferredWidth in RenderBlock.cpp, and can be reused to fix ceiledLayoutUnit. 667554 1 151892 allan.jensen 2012-07-12 02:26:25 -0700 Created attachment 151892 Patch 668004 2 151892 eae 2012-07-12 11:10:19 -0700 Comment on attachment 151892 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151892&action=review Good catch, thank you! > Source/WebCore/platform/FractionalLayoutUnit.h:97 > + static FractionalLayoutUnit fromFloatRound(float value) We probably want to assert that the value is in bounds here too. 668035 3 allan.jensen 2012-07-12 11:37:51 -0700 (In reply to comment #2) > (From update of attachment 151892 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151892&action=review > > Good catch, thank you! > > > Source/WebCore/platform/FractionalLayoutUnit.h:97 > > + static FractionalLayoutUnit fromFloatRound(float value) > > We probably want to assert that the value is in bounds here too. I believe in this case the values are asserted to be in bounds in the constructor, so I don't think we need to assert it in the fromFloatRound function. 670604 4 eae 2012-07-16 13:21:47 -0700 (In reply to comment #3) > I believe in this case the values are asserted to be in bounds in the constructor, so I don't think we need to assert it in the fromFloatRound function. You are right, that should be enough. Thanks. 678104 5 151892 abarth 2012-07-25 08:46:41 -0700 Comment on attachment 151892 Patch rs=me based on eae's unofficial review. 678159 6 151892 webkit.review.bot 2012-07-25 09:45:39 -0700 Comment on attachment 151892 Patch Clearing flags on attachment: 151892 Committed r123626: <http://trac.webkit.org/changeset/123626> 678160 7 webkit.review.bot 2012-07-25 09:45:43 -0700 All reviewed patches have been landed. Closing bug. 151892 2012-07-12 02:26:25 -0700 2012-07-25 09:45:39 -0700 Patch bug-91061-20120712112628.patch text/plain 3705 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTIyNDMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzIwZGY2YjQwZTAzOTYw Zjk5ZTEwMGI0MDhmZDBlYTIwMDI1M2FhNi4uZjQ4MTIxZmI0ODU1MDNmYzFjMzU4NTBjNzNkNDlm ZTVjMDBkMGJlOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI0IEBACisyMDEyLTA3LTEyICBBbGxh biBTYW5kZmVsZCBKZW5zZW4gIDxhbGxhbi5qZW5zZW5Abm9raWEuY29tPgorCisgICAgICAgIElu Y29ycmVjdCByb3VuZGluZyBpbiBjZWlsZWRMYXlvdXRVbml0IGFuZCByb3VuZGVkTGF5b3V0VW5p dC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkxMDYx CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIHR3 byBmdW5jdGlvbnMgYXJlIGN1cnJlbnRseSBib3RoIHJldHVybmluZyB0aGUgZmxvb3JlZExheW91 dFVuaXQgd2hlbiBzdWJwaXhlbAorICAgICAgICBsYXlvdXQgaXMgZW5hYmxlZC4gVGhpcyBwYXRj aCBjaGFuZ2VzIHRoZW0gdG8gcmV0dXJuIHRoZSBjb3JyZWN0IGZyYWN0aW9uYWwgcm91bmRlZAor ICAgICAgICB2YWx1ZS4KKworICAgICAgICAqIHBsYXRmb3JtL0ZyYWN0aW9uYWxMYXlvdXRVbml0 Lmg6CisgICAgICAgIChXZWJDb3JlOjpGcmFjdGlvbmFsTGF5b3V0VW5pdDo6ZnJvbUZsb2F0Q2Vp bCk6CisgICAgICAgIChGcmFjdGlvbmFsTGF5b3V0VW5pdCk6CisgICAgICAgIChXZWJDb3JlOjpG cmFjdGlvbmFsTGF5b3V0VW5pdDo6ZnJvbUZsb2F0Um91bmQpOgorICAgICAgICAqIHJlbmRlcmlu Zy9MYXlvdXRUeXBlcy5oOgorICAgICAgICAoV2ViQ29yZTo6cm91bmRlZExheW91dFVuaXQpOgor ICAgICAgICAoV2ViQ29yZTo6Y2VpbGVkTGF5b3V0VW5pdCk6CisgICAgICAgICogcmVuZGVyaW5n L1JlbmRlckJsb2NrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnVwZGF0ZVByZWZlcnJlZFdpZHRo KToKKwogMjAxMi0wNy0xMiAgTU9SSVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KIAog ICAgICAgICBbU2hhZG93IERPTV0gPHZpZGVvPiB3aXRoIDxzaGFkb3c+IGNyYXNoZXMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL0ZyYWN0aW9uYWxMYXlvdXRVbml0LmggYi9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9GcmFjdGlvbmFsTGF5b3V0VW5pdC5oCmluZGV4IDIxYzUy NjQxMDk4MzM3MGFmM2VmZGUwYmUzMGZjYjY4MjJjOTczYmIuLmI4OTA5MTY3YjVlOTgzMzFkODFk YWFiNDkwNGExNjg3Y2FlNDU0YTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L0ZyYWN0aW9uYWxMYXlvdXRVbml0LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vRnJh Y3Rpb25hbExheW91dFVuaXQuaApAQCAtODgsMTAgKzg4LDE3IEBAIHB1YmxpYzoKIAogICAgIHN0 YXRpYyBGcmFjdGlvbmFsTGF5b3V0VW5pdCBmcm9tRmxvYXRDZWlsKGZsb2F0IHZhbHVlKQogICAg IHsKLSAgICAgIFJFUE9SVF9PVkVSRkxPVyhpc0luQm91bmRzKHZhbHVlKSk7Ci0gICAgICBGcmFj dGlvbmFsTGF5b3V0VW5pdCB2OwotICAgICAgdi5tX3ZhbHVlID0gY2VpbGYodmFsdWUgKiBrRml4 ZWRQb2ludERlbm9taW5hdG9yKTsKLSAgICAgIHJldHVybiB2OworICAgICAgICBSRVBPUlRfT1ZF UkZMT1coaXNJbkJvdW5kcyh2YWx1ZSkpOworICAgICAgICBGcmFjdGlvbmFsTGF5b3V0VW5pdCB2 OworICAgICAgICB2Lm1fdmFsdWUgPSBjZWlsZih2YWx1ZSAqIGtGaXhlZFBvaW50RGVub21pbmF0 b3IpOworICAgICAgICByZXR1cm4gdjsKKyAgICB9CisKKyAgICBzdGF0aWMgRnJhY3Rpb25hbExh eW91dFVuaXQgZnJvbUZsb2F0Um91bmQoZmxvYXQgdmFsdWUpCisgICAgeworICAgICAgICBpZiAo dmFsdWUgPj0gMCkKKyAgICAgICAgICAgIHJldHVybiBGcmFjdGlvbmFsTGF5b3V0VW5pdCh2YWx1 ZSArIGVwc2lsb24oKSAvIDIuMGYpOworICAgICAgICByZXR1cm4gRnJhY3Rpb25hbExheW91dFVu aXQodmFsdWUgLSBlcHNpbG9uKCkgLyAyLjBmKTsKICAgICB9CiAKICNpZiBFTkFCTEUoU1VCUElY RUxfTEFZT1VUKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0xheW91dFR5 cGVzLmggYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvTGF5b3V0VHlwZXMuaAppbmRleCA3MGEy YWEwZDc3YjQ3Nzg0YmFiMDkwMmRkNzk5MTRkN2ExNjczMzdiLi45NTA1NWY4ZWIyMDBmOTQ2YWNm NzM3ZmE1ZDhjZjVlYTQ1ZDE1Y2YyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJp bmcvTGF5b3V0VHlwZXMuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvTGF5b3V0VHlw ZXMuaApAQCAtMTE3LDcgKzExNyw3IEBAIGlubGluZSBpbnQgZmxvb3JUb0ludChMYXlvdXRVbml0 IHZhbHVlKQogaW5saW5lIExheW91dFVuaXQgcm91bmRlZExheW91dFVuaXQoZmxvYXQgdmFsdWUp CiB7CiAjaWYgRU5BQkxFKFNVQlBJWEVMX0xBWU9VVCkKLSAgICByZXR1cm4gRnJhY3Rpb25hbExh eW91dFVuaXQodmFsdWUpOworICAgIHJldHVybiBGcmFjdGlvbmFsTGF5b3V0VW5pdDo6ZnJvbUZs b2F0Um91bmQodmFsdWUpOwogI2Vsc2UKICAgICByZXR1cm4gc3RhdGljX2Nhc3Q8aW50Pihscm91 bmRmKHZhbHVlKSk7CiAjZW5kaWYKQEAgLTEyNiw3ICsxMjYsNyBAQCBpbmxpbmUgTGF5b3V0VW5p dCByb3VuZGVkTGF5b3V0VW5pdChmbG9hdCB2YWx1ZSkKIGlubGluZSBMYXlvdXRVbml0IGNlaWxl ZExheW91dFVuaXQoZmxvYXQgdmFsdWUpCiB7CiAjaWYgRU5BQkxFKFNVQlBJWEVMX0xBWU9VVCkK LSAgICByZXR1cm4gRnJhY3Rpb25hbExheW91dFVuaXQodmFsdWUpOworICAgIHJldHVybiBGcmFj dGlvbmFsTGF5b3V0VW5pdDo6ZnJvbUZsb2F0Q2VpbCh2YWx1ZSk7CiAjZWxzZQogICAgIHJldHVy biBjZWlsZih2YWx1ZSk7CiAjZW5kaWYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJCbG9jay5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxv Y2suY3BwCmluZGV4IDAzMGIxODcxMTU3ZGE3MWJkODVhOWMxNjMzNTVlNjUwZjkwZDVjYzUuLmYw YmM3NjU4N2RjNzI4MzUxZjhlZTg1MDhmNTNlOWFjNjE3YTYwMWEgMTAwNzU1Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJCbG9jay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv cmVuZGVyaW5nL1JlbmRlckJsb2NrLmNwcApAQCAtNTU2Niw3ICs1NTY2LDcgQEAgc3RhdGljIGlu bGluZSB2b2lkIHN0cmlwVHJhaWxpbmdTcGFjZShmbG9hdCYgaW5saW5lTWF4LCBmbG9hdCYgaW5s aW5lTWluLAogCiBzdGF0aWMgaW5saW5lIHZvaWQgdXBkYXRlUHJlZmVycmVkV2lkdGgoTGF5b3V0 VW5pdCYgcHJlZmVycmVkV2lkdGgsIGZsb2F0JiByZXN1bHQpCiB7Ci0gICAgTGF5b3V0VW5pdCBz bmFwcGVkUmVzdWx0ID0gTGF5b3V0VW5pdDo6ZnJvbUZsb2F0Q2VpbChyZXN1bHQpOworICAgIExh eW91dFVuaXQgc25hcHBlZFJlc3VsdCA9IGNlaWxlZExheW91dFVuaXQocmVzdWx0KTsKICAgICBw cmVmZXJyZWRXaWR0aCA9IG1heChzbmFwcGVkUmVzdWx0LCBwcmVmZXJyZWRXaWR0aCk7CiB9CiAK