Summary: | Mock scrollbars differ by 1px in DRT vs. WKTR | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | rniwa, simon.fraser, zimmermann | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Tim Horton
2012-03-12 13:42:07 PDT
Created attachment 131397 [details]
difference image
Created attachment 131398 [details]
actual image
Created attachment 131399 [details]
expected image
Hmm, i wonder if the total scrollbar length is different by a pixel or two. (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? (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? Rounding probably makes more sense. Created attachment 145639 [details]
patch
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. (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... (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. Anyway, I've reverted the work around and rebaselined all the tests. Took me forever to finish with all other test & build failures :( |