Bug 103984 – IntSize::scale() should return rounded value

• Status: REOPENED
• Component: WebCore Misc.

[Mikhail Pozdnyakov]

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.

[Mikhail Pozdnyakov]

Created attachment 177447 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 177447 [details]
patch

I assume you ran layout tests and checked other call sites

[Dominik Röttsches (drott)]

(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).

[Mikhail Pozdnyakov]

(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.

[WebKit Review Bot]

Comment on attachment 177447 [details]
patch

Clearing flags on attachment: 177447

Committed r136509: <http://trac.webkit.org/changeset/136509>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Emil A Eklund]

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

[Darin Adler]

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?

[Darin Adler]

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.

[Emil A Eklund]

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.

[Kenneth Rohde Christiansen]

Fine with me, please roll out. As I wrote before "I assume you ran layout tests and checked other call sites"

[WebKit Review Bot]

Re-opened since this is blocked by bug 104015