|
|
Created:
7 years, 11 months ago by jamesr Modified:
7 years, 11 months ago Reviewers:
Vangelis Kokkevis CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, ccameron Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionDisable compositor thread input handling on windows
BUG=160122
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177369
Patch Set 1 #
Messages
Total messages: 17 (0 generated)
This is completely untested, but should disable compositor thread input handling for windows.
On 2013/01/16 18:01:11, jamesr wrote: > This is completely untested, but should disable compositor thread input handling > for windows. Thanks, James. I tried it locally and it seems to do the trick. I'll test a bit more.
On 2013/01/16 18:21:21, vangelis wrote: > On 2013/01/16 18:01:11, jamesr wrote: > > This is completely untested, but should disable compositor thread input > handling > > for windows. > > Thanks, James. I tried it locally and it seems to do the trick. I'll test a bit > more. The downside of this patch is it'll lose us touch/gesture input handling on win8. Another approach would be to #ifdef out the handling just for mousewheel. This means we'd do a thread bounce for all input events enough though the only ones we could potentially handle are touch events, which only a small set of users will see. I think I'd prefer turning off all input handling (as this patch does) for 24 since it's the simplest + lowest risk solution.
On 2013/01/17 00:04:11, jamesr wrote: > On 2013/01/16 18:21:21, vangelis wrote: > > On 2013/01/16 18:01:11, jamesr wrote: > > > This is completely untested, but should disable compositor thread input > > handling > > > for windows. > > > > Thanks, James. I tried it locally and it seems to do the trick. I'll test a > bit > > more. > > The downside of this patch is it'll lose us touch/gesture input handling on > win8. > > Another approach would be to #ifdef out the handling just for mousewheel. This > means we'd do a thread bounce for all input events enough though the only ones > we could potentially handle are touch events, which only a small set of users > will see. I think I'd prefer turning off all input handling (as this patch > does) for 24 since it's the simplest + lowest risk solution. I tested this on OS X, and the bugs listed in issue 138003 (rubber-band bounce, navigation swipe, two-finger place, etc), all behave correctly. Want to change this to a "|| !defined(OS_MACOSX)" as well?
On 2013/01/17 00:48:21, ccameron1 wrote: > On 2013/01/17 00:04:11, jamesr wrote: > > On 2013/01/16 18:21:21, vangelis wrote: > > > On 2013/01/16 18:01:11, jamesr wrote: > > > > This is completely untested, but should disable compositor thread input > > > handling > > > > for windows. > > > > > > Thanks, James. I tried it locally and it seems to do the trick. I'll test a > > bit > > > more. > > > > The downside of this patch is it'll lose us touch/gesture input handling on > > win8. > > > > Another approach would be to #ifdef out the handling just for mousewheel. > This > > means we'd do a thread bounce for all input events enough though the only ones > > we could potentially handle are touch events, which only a small set of users > > will see. I think I'd prefer turning off all input handling (as this patch > > does) for 24 since it's the simplest + lowest risk solution. > > I tested this on OS X, and the bugs listed in issue 138003 (rubber-band bounce, > navigation swipe, two-finger place, etc), all behave correctly. Want to change > this to a "|| !defined(OS_MACOSX)" as well? The issues on mac and windows are pretty different, so I'd prefer having separate #if lines citing different crbugs
On 2013/01/17 00:04:11, jamesr wrote: > On 2013/01/16 18:21:21, vangelis wrote: > > On 2013/01/16 18:01:11, jamesr wrote: > > > This is completely untested, but should disable compositor thread input > > handling > > > for windows. > > > > Thanks, James. I tried it locally and it seems to do the trick. I'll test a > bit > > more. > > The downside of this patch is it'll lose us touch/gesture input handling on > win8. What touch / gesture events does the thread currently handle on windows? Do we support fling? I know pinch-zoom isn't there quite yet. > > Another approach would be to #ifdef out the handling just for mousewheel. This > means we'd do a thread bounce for all input events enough though the only ones > we could potentially handle are touch events, which only a small set of users > will see. I think I'd prefer turning off all input handling (as this patch > does) for 24 since it's the simplest + lowest risk solution.
Scroll and fling are supported, but they're honestly pretty busted.
On 2013/01/17 00:59:01, jamesr wrote: > Scroll and fling are supported, but they're honestly pretty busted. LGTM to get this in as a stopgap.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/11946019/1
Retried try job too often on linux_chromeos for step(s) unit_tests
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/11946019/1
Retried try job too often on linux_chromeos for step(s) unit_tests
On 2013/01/17 05:31:53, I haz the power (commit-bot) wrote: > Retried try job too often on linux_chromeos for step(s) unit_tests Pretty sure I'm not breaking NetworkMenuIconTest.CellularIcon on linux_chromeos...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/11946019/1
Retried try job too often on linux_chromeos for step(s) unit_tests
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/11946019/1
Message was sent while issue was closed.
Change committed as 177369 |