Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(91)

Issue 1283913002: Improve scroll performance on Windows 10 with high precision touchpads. (Closed)

Created:
5 years, 4 months ago by ananta
Modified:
5 years, 4 months ago
Reviewers:
Reid Kleckner, sky, hans
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Improve scroll performance on Windows 10 with high precision touchpads. Chrome scrolling performance on Windows 10 with high precision touch pads, is very janky and jumpy. Windows 10 introduces a new API called Direct Manipulation which allows consumers to support smooth scrolling, panning etc. Mouse wheel messages are dispatched differently based on whether the window is a Direct manipulation consumer. Microsoft recommends supporting Direct Manipulation on Windows 10 and above. This is an intial patch to do the bare minimum to register Chrome as a Direct Manipulation consumer. The associated functionality is provided by the newly added DirectManipulationHelper class which lives as a member of the LegacyRenderWidgetHostHWND class which ensures that these changes are isolated cleanly. This does smoothen scrolling on these devices. However there is still work to be done BUG=517183, 517187 Committed: https://crrev.com/d53bb2ab620b6dc5e17da85dcf1b83b516c8c212 Cr-Commit-Position: refs/heads/master@{#342962}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address review comments #

Patch Set 3 : Don't pass wheel messages to DirectManipulation from views if it is coming from the child #

Total comments: 13

Patch Set 4 : #

Patch Set 5 : Fixed comment #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+250 lines, -1 line) Patch
M content/browser/renderer_host/legacy_render_widget_host_win.h View 1 2 3 2 chunks +12 lines, -0 lines 0 comments Download
M content/browser/renderer_host/legacy_render_widget_host_win.cc View 1 2 3 5 chunks +18 lines, -0 lines 0 comments Download
M ui/gfx/BUILD.gn View 1 1 chunk +2 lines, -0 lines 0 comments Download
M ui/gfx/gfx.gyp View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A ui/gfx/win/direct_manipulation.h View 1 2 3 4 1 chunk +78 lines, -0 lines 1 comment Download
A ui/gfx/win/direct_manipulation.cc View 1 2 3 1 chunk +109 lines, -0 lines 0 comments Download
M ui/views/win/hwnd_message_handler.h View 1 2 3 2 chunks +9 lines, -1 line 1 comment Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 5 chunks +20 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 15 (5 generated)
ananta
5 years, 4 months ago (2015-08-11 00:32:33 UTC) #2
sky
https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_host/legacy_render_widget_host_win.cc File content/browser/renderer_host/legacy_render_widget_host_win.cc (right): https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_host/legacy_render_widget_host_win.cc#newcode50 content/browser/renderer_host/legacy_render_widget_host_win.cc:50: class DirectManipulationHelper { Would it make more sense to ...
5 years, 4 months ago (2015-08-11 15:43:12 UTC) #3
ananta
https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_host/legacy_render_widget_host_win.cc File content/browser/renderer_host/legacy_render_widget_host_win.cc (right): https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_host/legacy_render_widget_host_win.cc#newcode50 content/browser/renderer_host/legacy_render_widget_host_win.cc:50: class DirectManipulationHelper { On 2015/08/11 15:43:12, sky wrote: > ...
5 years, 4 months ago (2015-08-11 20:52:08 UTC) #4
sky
LGTM https://codereview.chromium.org/1283913002/diff/40001/content/browser/renderer_host/legacy_render_widget_host_win.h File content/browser/renderer_host/legacy_render_widget_host_win.h (right): https://codereview.chromium.org/1283913002/diff/40001/content/browser/renderer_host/legacy_render_widget_host_win.h#newcode18 content/browser/renderer_host/legacy_render_widget_host_win.h:18: #include "ui/gfx/win/direct_manipulation.h" forward declare and include in .cc. ...
5 years, 4 months ago (2015-08-11 23:45:02 UTC) #5
ananta
https://codereview.chromium.org/1283913002/diff/40001/content/browser/renderer_host/legacy_render_widget_host_win.h File content/browser/renderer_host/legacy_render_widget_host_win.h (right): https://codereview.chromium.org/1283913002/diff/40001/content/browser/renderer_host/legacy_render_widget_host_win.h#newcode18 content/browser/renderer_host/legacy_render_widget_host_win.h:18: #include "ui/gfx/win/direct_manipulation.h" On 2015/08/11 23:45:02, sky wrote: > forward ...
5 years, 4 months ago (2015-08-12 00:41:58 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1283913002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1283913002/80001
5 years, 4 months ago (2015-08-12 00:47:41 UTC) #9
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 4 months ago (2015-08-12 02:04:23 UTC) #10
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/d53bb2ab620b6dc5e17da85dcf1b83b516c8c212 Cr-Commit-Position: refs/heads/master@{#342962}
5 years, 4 months ago (2015-08-12 02:05:10 UTC) #11
Reid Kleckner
I'll try to write a quick fix. https://codereview.chromium.org/1283913002/diff/80001/ui/gfx/win/direct_manipulation.h File ui/gfx/win/direct_manipulation.h (right): https://codereview.chromium.org/1283913002/diff/80001/ui/gfx/win/direct_manipulation.h#newcode40 ui/gfx/win/direct_manipulation.h:40: class GFX_EXPORT ...
5 years, 4 months ago (2015-08-12 15:26:11 UTC) #13
hans
5 years, 4 months ago (2015-08-12 20:24:03 UTC) #15
Message was sent while issue was closed.
https://codereview.chromium.org/1283913002/diff/80001/ui/views/win/hwnd_messa...
File ui/views/win/hwnd_message_handler.h (right):

https://codereview.chromium.org/1283913002/diff/80001/ui/views/win/hwnd_messa...
ui/views/win/hwnd_message_handler.h:594:
base::WeakPtrFactory<HWNDMessageHandler> autohide_factory_;
Another Win/Clang style plugin error:

In file included from ..\..\ui\views\win\hwnd_message_handler.cc:5:
..\..\ui/views/win/hwnd_message_handler.h(594,44) :  error: [chromium-style]
WeakPtrFactory members which refer to their outer class must be the last member
in the outer class definition.
  base::WeakPtrFactory<HWNDMessageHandler> autohide_factory_;
                                           ^

I'll prepare a patch.

Powered by Google App Engine
This is Rietveld 408576698