103984 2012-12-04 01:32:37 -0800 IntSize::scale() should return rounded value 2012-12-04 10:46:04 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified REOPENED P2 Normal --- 104015 1 mikhail.pozdnyakov mikhail.pozdnyakov darin d-r eae kenneth tmpsantos webkit.review.bot oldest_to_newest 782365 0 mikhail.pozdnyakov 2012-12-04 01:32:37 -0800 Currently IntSize::scale() just casts 'float' result to 'int' loosing all the data after '.' instead of rounding the float value to the nearest integer. For example scaling (10, 10) by 0.999999 will result (9,9). Also it's causing flakiness of css3/device-adapt/viewport-width-not-affecting-next-page.html on WK2 EFL. 782373 1 177447 mikhail.pozdnyakov 2012-12-04 01:38:56 -0800 Created attachment 177447 patch 782379 2 177447 kenneth 2012-12-04 01:45:15 -0800 Comment on attachment 177447 patch I assume you ran layout tests and checked other call sites 782413 3 d-r 2012-12-04 02:35:28 -0800 (In reply to comment #0) > Currently IntSize::scale() just casts 'float' result to 'int' loosing all the data after '.' instead of > rounding the float value to the nearest integer. For example scaling (10, 10) by 0.999999 will result (9,9). > Also it's causing flakiness of css3/device-adapt/viewport-width-not-affecting-next-page.html on WK2 EFL. Should the viewport code maybe use LayoutUnits instead? (without knowing much about it). 782447 4 mikhail.pozdnyakov 2012-12-04 03:28:44 -0800 (In reply to comment #2) > (From update of attachment 177447 [details]) > I assume you ran layout tests and checked other call sites Some EWS bots (chromium at least) check layout tests as well. Anyway I've just run the tests locally and did not discover regressions. 782535 5 177447 webkit.review.bot 2012-12-04 07:42:39 -0800 Comment on attachment 177447 patch Clearing flags on attachment: 177447 Committed r136509: <http://trac.webkit.org/changeset/136509> 782536 6 webkit.review.bot 2012-12-04 07:42:42 -0800 All reviewed patches have been landed. Closing bug. 782603 7 eae 2012-12-04 09:27:47 -0800 This change broke at least 12 tests. Most of them seem harmless but this one is worrying as it changes how the image is aligned: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fbackgrounds%2Fsize%2Fcontain-and-cover-zoomed.html 782629 8 darin 2012-12-04 09:45:35 -0800 This is a really low level change to make. We have to look at every call site and make sure that rounding is desired. It wasn’t an accident before that this function truncated; how did we determine call sites did not depend on that? 782630 9 darin 2012-12-04 09:46:34 -0800 A much safer change would be to rename scale to describe the rounding vs. truncating behavior and possibly splitting it into more than one function if callers need more than one kind of behavior. If there are many tests broken, this should be rolled out and the change should be redone more carefully. 782642 10 eae 2012-12-04 09:58:21 -0800 I'm with Darin on this one. Unless I hear back from either of you in the next hour or so I'll rollout this change. 782689 11 kenneth 2012-12-04 10:41:41 -0800 Fine with me, please roll out. As I wrote before "I assume you ran layout tests and checked other call sites" 782695 12 webkit.review.bot 2012-12-04 10:46:04 -0800 Re-opened since this is blocked by bug 104015 177447 2012-12-04 01:38:56 -0800 2012-12-04 07:42:39 -0800 patch bug103984 text/plain 3355 mikhail.pozdnyakov Y29tbWl0IDYyYWVkYjI3ZDk5OGRmZDZkNTY1ZmRmODU3NTU3ZWZkZWQxOWNjYWUKQXV0aG9yOiBN aWtoYWlsIFBvemRueWFrb3YgPG1pa2hhaWwucG96ZG55YWtvdkBpbnRlbC5jb20+CkRhdGU6ICAg VHVlIERlYyA0IDExOjM1OjI1IDIwMTIgKzAyMDAKCiAgICBidWcxMDM5ODQKCmRpZmYgLS1naXQg YS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNDgz OTE3ZS4uMGNiNTU1ZSAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xh eW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEyLTEyLTA0ICBNaWtoYWls IFBvemRueWFrb3YgIDxtaWtoYWlsLnBvemRueWFrb3ZAaW50ZWwuY29tPgorCisgICAgICAgIElu dFNpemU6OnNjYWxlKCkgc2hvdWxkIHJldHVybiByb3VuZGVkIHZhbHVlCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDM5ODQKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUZXN0IGNzczMvZGV2aWNlLWFkYXB0 L3ZpZXdwb3J0LXdpZHRoLW5vdC1hZmZlY3RpbmctbmV4dC1wYWdlLmh0bWwgaXMgbm90IGZsYWt5 IGFueSBtb3JlLgorCisgICAgICAgICogcGxhdGZvcm0vZWZsLXdrMi9UZXN0RXhwZWN0YXRpb25z OgorCiAyMDEyLTEyLTAzICBOYXRlIENoYXBpbiAgPGphcGhldEBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgc3ZnL2ZvcmVpZ25PYmplY3Qvdmlld3BvcnQtZm9yZWlnbk9iamVjdC1jcmFzaC5odG1s IHJlY3Vyc2VzIGluZmluaXRlbHkgc29tZXRpbWVzCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9w bGF0Zm9ybS9lZmwtd2syL1Rlc3RFeHBlY3RhdGlvbnMgYi9MYXlvdXRUZXN0cy9wbGF0Zm9ybS9l Zmwtd2syL1Rlc3RFeHBlY3RhdGlvbnMKaW5kZXggODQwYzlmMy4uZjcyMWUxNiAxMDA2NDQKLS0t IGEvTGF5b3V0VGVzdHMvcGxhdGZvcm0vZWZsLXdrMi9UZXN0RXhwZWN0YXRpb25zCisrKyBiL0xh eW91dFRlc3RzL3BsYXRmb3JtL2VmbC13azIvVGVzdEV4cGVjdGF0aW9ucwpAQCAtMjAyLDkgKzIw Miw2IEBAIHdlYmtpdC5vcmcvYi8xMDMzOTkgWyBEZWJ1ZyBdIGZhc3QvZm9ybXMvYmFzaWMtYnV0 dG9ucy5odG1sIFsgRmFpbHVyZSBQYXNzIF0KIHdlYmtpdC5vcmcvYi8xMDM0MDAgWyBEZWJ1ZyBd IGNvbXBvc2l0aW5nL292ZXJmbG93L2NvbnRlbnQtZ2FpbnMtc2Nyb2xsYmFycy5odG1sIFsgQ3Jh c2ggUGFzcyBdCiB3ZWJraXQub3JnL2IvMTAzNDAwIFsgRGVidWcgXSBjb21wb3NpdGluZy9vdmVy Zmxvdy9jb250ZW50LWxvc2VzLXNjcm9sbGJhcnMuaHRtbCBbIENyYXNoIFBhc3MgXQogCi0jIEVh c2lseSByZXByb2R1Y2FibGUgZmxha2luZXNzIG9uIEVGTCBXSzIgb25seQotd2Via2l0Lm9yZy9i LzEwMzg4MiBjc3MzL2RldmljZS1hZGFwdC92aWV3cG9ydC13aWR0aC1ub3QtYWZmZWN0aW5nLW5l eHQtcGFnZS5odG1sIFsgRmFpbHVyZSBQYXNzIF0KLQogd2Via2l0Lm9yZy9iLzEwMzg4MyBjc3Mz L3NlbGVjdG9yczMvaHRtbC9jc3MzLW1vZHNlbC0xLmh0bWwgWyBGYWlsdXJlIFBhc3MgXQogd2Vi a2l0Lm9yZy9iLzEwMzg4MyBjc3MzL3NlbGVjdG9yczMveG1sL2NzczMtbW9kc2VsLTEueG1sIFsg RmFpbHVyZSBQYXNzIF0KIHdlYmtpdC5vcmcvYi8xMDM4ODMgY3NzMy9zZWxlY3RvcnMzL3hodG1s L2NzczMtbW9kc2VsLTEueG1sIFsgRmFpbHVyZSBQYXNzIF0KZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBiN2JiMGJm Li40YzUyMzZhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTItMTItMDQgIE1pa2hh aWwgUG96ZG55YWtvdiAgPG1pa2hhaWwucG96ZG55YWtvdkBpbnRlbC5jb20+CisKKyAgICAgICAg SW50U2l6ZTo6c2NhbGUoKSBzaG91bGQgcmV0dXJuIHJvdW5kZWQgdmFsdWUKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwMzk4NAorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEludFNpemU6OnNjYWxlKCkgaXMg bm93IHJvdW5kaW5nIGZsb2F0IHZhbHVlIHRvIHRoZSBuZWFyZXN0IGludGVnZXIgaW5zdGVhZCBv ZiBibGluZCB0eXBlIGNhc3RpbmcuCisKKyAgICAgICAgVGVzdDogY3NzMy9kZXZpY2UtYWRhcHQv dmlld3BvcnQtd2lkdGgtbm90LWFmZmVjdGluZy1uZXh0LXBhZ2UuaHRtbAorCisgICAgICAgICog cGxhdGZvcm0vZ3JhcGhpY3MvSW50U2l6ZS5oOgorICAgICAgICAoV2ViQ29yZTo6SW50U2l6ZTo6 c2NhbGUpOgorCiAyMDEyLTEyLTAzICBLZW50YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9y Zz4KIAogICAgICAgICBbVjhdIEVudGVyIGFuIElzb2xhdGUgYmVmb3JlIGluaXRpYWxpemluZyBQ ZXJJc29sYXRlRGF0YQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvSW50U2l6ZS5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvSW50U2l6ZS5o CmluZGV4IGM4YTU0OWQuLjE4MTQxYzMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL0ludFNpemUuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9JbnRTaXplLmgKQEAgLTI2LDYgKzI2LDggQEAKICNpZm5kZWYgSW50U2l6ZV9oCiAjZGVm aW5lIEludFNpemVfaAogCisjaW5jbHVkZSA8d3RmL01hdGhFeHRyYXMuaD4KKwogI2lmIFVTRShD RykgfHwgVVNFKFNLSUFfT05fTUFDX0NIUk9NSVVNKQogdHlwZWRlZiBzdHJ1Y3QgQ0dTaXplIENH U2l6ZTsKICNlbmRpZgpAQCAtODMsOCArODUsOCBAQCBwdWJsaWM6CiAKICAgICB2b2lkIHNjYWxl KGZsb2F0IHdpZHRoU2NhbGUsIGZsb2F0IGhlaWdodFNjYWxlKQogICAgIHsKLSAgICAgICAgbV93 aWR0aCA9IHN0YXRpY19jYXN0PGludD4oc3RhdGljX2Nhc3Q8ZmxvYXQ+KG1fd2lkdGgpICogd2lk dGhTY2FsZSk7Ci0gICAgICAgIG1faGVpZ2h0ID0gc3RhdGljX2Nhc3Q8aW50PihzdGF0aWNfY2Fz dDxmbG9hdD4obV9oZWlnaHQpICogaGVpZ2h0U2NhbGUpOworICAgICAgICBtX3dpZHRoID0gbHJv dW5kZihzdGF0aWNfY2FzdDxmbG9hdD4obV93aWR0aCkgKiB3aWR0aFNjYWxlKTsKKyAgICAgICAg bV9oZWlnaHQgPSBscm91bmRmKHN0YXRpY19jYXN0PGZsb2F0PihtX2hlaWdodCkgKiBoZWlnaHRT Y2FsZSk7CiAgICAgfQogICAgIAogICAgIHZvaWQgc2NhbGUoZmxvYXQgc2NhbGUpCg==