Summary: | [Chromium] Re-enable handling of smooth scrolling on Chromium Linux/Windows | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Kroeger <rjkroege> | ||||||||
Component: | UI Events | Assignee: | Robert Kroeger <rjkroege> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | jamesr, rjkroege, scottbyer, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Robert Kroeger
2012-05-25 14:26:40 PDT
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. |