Very late in the implementation of ChromiumOS trackpad scrolling, we discovered that to also support smooth wheel scrolling Chromium Linux/Windows requires differentiating hasPreciseScrollingDeltas PlatformWheelEvents from !hasPreciseScrollingDeltas wheel events. Consequently, enable the hasPreciseScrollingDelta feature of PlatformWheelEvents for Chromium Linux/Windows.
Created attachment 144557 [details] Patch
jamesr@: could you perhaps review?
Comment on attachment 144557 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144557&action=review > Source/WebCore/platform/PlatformWheelEvent.h:71 > +#if PLATFORM(MAC) || PLATFORM(CHROMIUM) Do these phases make sense for CHROMIUM non-OS X platforms? > Source/WebCore/platform/PlatformWheelEvent.h:93 > +#if PLATFORM(MAC) || PLATFORM(CHROMIUM) I understand that hasPreciseScrollingDeltas makes sense for non-OS X chromium platforms, but do the other bits make any sense? > Source/WebCore/platform/ScrollAnimator.cpp:95 > + ScrollGranularity granularity = e.hasPreciseScrollingDeltas() ? ScrollByPrecisePixel : ScrollByPixel; how is hasPreciseScrollingDeltas supposed to interact with PlatformWheelEvent::granularity() ?
(In reply to comment #3) > (From update of attachment 144557 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144557&action=review > > > Source/WebCore/platform/PlatformWheelEvent.h:71 > > +#if PLATFORM(MAC) || PLATFORM(CHROMIUM) > > Do these phases make sense for CHROMIUM non-OS X platforms? Not at present. Something similar would probably be necessary when we go about writing support for inertial scroll. I couldn't decide here which would be better: more convoluted #ifs or just including the unused fields. I thought that doing it this way made the patch simpler. I'll do it the other way in p2. > > > Source/WebCore/platform/PlatformWheelEvent.h:93 > > +#if PLATFORM(MAC) || PLATFORM(CHROMIUM) > > I understand that hasPreciseScrollingDeltas makes sense for non-OS X chromium platforms, but do the other bits make any sense? > > > Source/WebCore/platform/ScrollAnimator.cpp:95 > > + ScrollGranularity granularity = e.hasPreciseScrollingDeltas() ? ScrollByPrecisePixel : ScrollByPixel; > > how is hasPreciseScrollingDeltas supposed to interact with PlatformWheelEvent::granularity() ? hasPreciseScrollingDeltas is ignored when granularity() != ScrollByPixelWheelEvent. If granularity == ScrollByPixelWheelEvent and hasPreciseScrollingDeltas is false, ScrollAnimatorNone should animate the scroll. Otherwise, ScrollAnimatorNone should simply scroll by the specified amount.
Created attachment 144823 [details] Patch
Note in passing: the gtk build fail would not seem to be caused by this change.
Comment on attachment 144823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144823&action=review R=me. The ifdefs are pretty unfortunate, but I think they reflect fairly accurately the complexities of the different bits :( > Source/WebCore/platform/ScrollAnimatorNone.cpp:416 > + spurious newline
Comment on attachment 144823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144823&action=review >> Source/WebCore/platform/ScrollAnimatorNone.cpp:416 >> + > > spurious newline fixed in p3.
Created attachment 146135 [details] Patch
Comment on attachment 146135 [details] Patch Clearing flags on attachment: 146135 Committed r119684: <http://trac.webkit.org/changeset/119684>
All reviewed patches have been landed. Closing bug.