Bug 80879 – Mock scrollbars differ by 1px in DRT vs. WKTR

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Tim Horton]

Running SVG pixel tests with tolerance=0 between DRT and WKTR, I noticed that in every case there's a scrollbar in the test, the thumb is 1px longer in DRT. (I assume, but have not tested, that this affects non-SVG tests as well). This causes the tests to fail.

Example: svg/dom/SVGLengthList-basics-diffs.html

Attaching two images from mac-lion.

[Tim Horton]

Created attachment 131397 [details]
difference image

[Tim Horton]

Created attachment 131398 [details]
actual image

[Tim Horton]

Created attachment 131399 [details]
expected image

[Simon Fraser (smfr)]

Hmm, i wonder if the total scrollbar length is different by a pixel or two.

[Tim Horton]

(In reply to comment #4)
> Hmm, i wonder if the total scrollbar length is different by a pixel or two.

Nope:


WKTR:

6/4/12 1:37:25.526 PM WebProcess[76497]: trackRect height: 600
6/4/12 1:37:25.527 PM WebProcess[76497]: thumbRect height: 158

DRT:

6/4/12 1:41:44.910 PM DumpRenderTree[77380]: knobRect height: 159.000000 bounds height: 600.000000


I'm guessing rounding?

[Tim Horton]

(In reply to comment #5)
> I'm guessing rounding?

Yep, WebCore::ScrollbarThemeComposite truncates, DRTMockScroller rounds. (the result of proportion * total track length).

Which do you think we should change, Simon?

[Simon Fraser (smfr)]

Rounding probably makes more sense.

[Tim Horton]

Created attachment 145639 [details]
patch

[Tim Horton]

Thanks!

http://trac.webkit.org/changeset/119431

[Darin Adler]

Comment on attachment 145639 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145639&action=review

> Source/WebCore/platform/ScrollbarThemeComposite.cpp:245
> +    int length = round(proportion * trackLen);

Since we are rounding a float and want the result to be an int, the best function is lroundf.

By using round, we are converting from float to double, then rounding to another double, then converting to an int.

If we used lroundf, we would pass in a float, then get a long as a result, and then we would be good, since long and int are the same type on all the compilers that are used to compile WebKit.

[Ryosuke Niwa]

(In reply to comment #10)
> (From update of attachment 145639 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145639&action=review
> 
> > Source/WebCore/platform/ScrollbarThemeComposite.cpp:245
> > +    int length = round(proportion * trackLen);
> 
> Since we are rounding a float and want the result to be an int, the best function is lroundf.
> 
> By using round, we are converting from float to double, then rounding to another double, then converting to an int.
> 
> If we used lroundf, we would pass in a float, then get a long as a result, and then we would be good, since long and int are the same type on all the compilers that are used to compile WebKit.

I wonder if this contributed to the large number of test failures in Chromium...

[Tim Horton]

(In reply to comment #11)
> I wonder if this contributed to the large number of test failures in Chromium...

Seems unlikely, it is inefficient but shouldn't be wrong. The test failures are surely because this change actually changes the rounding... as it was intended to.

[Ryosuke Niwa]

Anyway, I've reverted the work around and rebaselined all the tests. Took me forever to finish with all other test & build failures :(