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.
Created attachment 177447 [details] patch
Comment on attachment 177447 [details] patch I assume you ran layout tests and checked other call sites
(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).
(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.
Comment on attachment 177447 [details] patch Clearing flags on attachment: 177447 Committed r136509: <http://trac.webkit.org/changeset/136509>
All reviewed patches have been landed. Closing bug.
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
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?
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.
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.
Fine with me, please roll out. As I wrote before "I assume you ran layout tests and checked other call sites"
Re-opened since this is blocked by bug 104015