Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for precision trackpad / mouse scrolling in windows. #8960

Closed
xaviergonz opened this issue Mar 18, 2017 · 34 comments

Comments

Projects
None yet
@xaviergonz
Copy link
Contributor

commented Mar 18, 2017

  • Electron version: 1.6.3
  • Operating system: Windows 10

Expected behavior

When scrolling using a precision trackpad / mouse it should be able to react to WM_MOUSEWHEEL / WM_MOUSEHWHEEL events with deltas < 120, like the ones generated by precision trackpads such as the one from surface pro and others.
This is properly supported by chrome and other browsers.

Actual behavior

Using such devices to scroll results in a laggy and wrong behavior, making scrolling almost unusable.

How to reproduce

Just use one of such devices and try to scroll a scrollable page.

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2017

PS: I'd give it a go, but I'd need some pointer on where to start looking.

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2017

I just noticed something weird, if the window is resized then scrolling works properly. Seems to be related to this issue:
microsoft/vscode#13612

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2017

I found a workaround about this. Not sure why but if the window is initially hidden and then shown only once the webpage is loaded the scroll works ok:

    const shouldShow = true;
    mainWindow = new BrowserWindow({
      width: 1000,
      height: 600,
      show: false
    });

    // workaround to fix weird scroll behavior
    mainWindow.on('ready-to-show', () => {
      if (shouldShow) {
        mainWindow.show();
      }
    });
@timfish

This comment has been minimized.

Copy link
Member

commented Mar 21, 2017

Is the electron behaviour of WM_MOUSEWHEEL / WM_MOUSEHWHEEL events with deltas < 120 different to Chrome? If not, it's unlikely to be fixed only in electron.

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2017

Funnily enough I just had the very same issue in chrome itself as well when the chrome process is closed and then you open as the very first (and only) tab a scrollable page. Resizing the window fixed it.

For example, just close the chrome process and then on a command line do "start https://github.com/electron/electron/issues/8960#issuecomment-288228600".

@timfish

This comment has been minimized.

Copy link
Member

commented Mar 21, 2017

If it's a reproducible chrome bug it needs reporting there and when it's fixed it will finally find it's way into electron.

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2017

Agreed, I just filled a bug report to the Chrome team about it.

@astoilkov

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2017

@xaviergonz Can you share the link to the Chrome bug you created so I could star it and follow its progress?

@xaviergonz

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2017

@astoilkov I submitted it via chrome menu - help - report an issue, so there's no ticket (AFAIK)

@zcbenz

This comment has been minimized.

Copy link
Member

commented Jun 14, 2017

Since this is a bug of Chrome, I suggest reporting to Chromium's issues list instead, the Chrome developers would have a much better opinion on this issue.

I'm closing this since we are unable to fix nor debug this issue on Electron's side.

@wizarrc

This comment has been minimized.

Copy link

commented Oct 18, 2017

I found an issue that seems to reference the issue in Chromium's issue list.

https://bugs.chromium.org/p/chromium/issues/detail?id=713907

It looks like the developers cannot replicate the problem. Everyone feel free to upvote it to get this thing fixed.

@CoenraadS

This comment has been minimized.

Copy link

commented Oct 28, 2017

Is starring the same as upvoting? The recent steps posted (https://bugs.chromium.org/p/chromium/issues/detail?id=713907#c10) make it so easy to reproduce, need Chrome team to look at this issue again.

Maybe the Electron team has direct contact with Chrome team to speed up the process, since this affects all electron apps also?

@chaopeng

This comment has been minimized.

Copy link

commented Oct 29, 2017

Hi, I am a developer from Chrome/input-dev currently working on support Windows Precision Touchpad. Just create an issue for it. I already start coding, hope it can fix this issue.

https://bugs.chromium.org/p/chromium/issues/detail?id=779372

@sywesk

This comment has been minimized.

Copy link

commented Dec 13, 2017

@chaopeng

This comment has been minimized.

Copy link

commented Dec 13, 2017

The fix can apply to any version after m51. But we are monitoring it to make sure it does not hang/crash GPU process.

@chaopeng

This comment has been minimized.

Copy link

commented Jan 30, 2018

Hi folks, I have a new fix for this issue. Since the previous has regression on Print page. Does electron want to port it?

https://bugs.chromium.org/p/chromium/issues/detail?id=713907#c36

The new one, I think is pretty stable, we already monitor it for a week.
For porting the fix to electron, you may want to check https://github.com/electron/electron/blob/master/atom/browser/native_window_views.cc#L981-L989

@alexeykuzmin

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2018

The fix from electron/libchromiumcontent#453 also has to be backported to libcc's electron-2-0-x branch. Reopen.

@alexeykuzmin alexeykuzmin reopened this Mar 7, 2018

@alexeykuzmin

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2018

The fix from electron/libchromiumcontent#453 has also be backported to libcc's electron-2-0-x branch.

electron/libchromiumcontent#472

@orchidalloy

This comment has been minimized.

Copy link

commented Jun 7, 2018

It's still very much a thing, seen it in Atom, VS Code, Discord, Whatsapp... the list goes on. Incredibly annoying. It happens very often.

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2018

This issue should be reopened, it is not fixed in any version of Electron for us (VSCode).

@Tyriar Tyriar reopened this Jun 14, 2018

@rossnichols

This comment has been minimized.

Copy link

commented Jun 14, 2018

I'm a developer at Microsoft, on the input stack of the Windows OS, and I looked into this issue more by debugging Visual Studio Code with and without their workaround (setting: window.smoothScrollingWorkaround).

In the “good” setup (workaround active), there are three HWNDs:

+ Toplevel: class=Chrome_WidgetWin_1, name is based on the tab’s file
+- Child 1: class=Chrome_RenderWidgetHostHWND, name=Chrome Legacy Window
+- Child 2: class=Intermediate D3D Window, no name

In the “bad” setup (laggy touchpad), Child 1 doesn’t exist.

The toplevel window and Child 1 are on the same thread; Child 2 is in a separate process (though they all seem to share an input queue, via AttachThreadInput). When the OS hit-tests an input payload, it finds one of these children. When it finds Child 2 (as is the case when Child 1 doesn't exist), the input is first delivered to Child 2's thread before getting rerouted to the toplevel window's thread. It is the interaction between these two threads that is causing the issue.

The underlying issue is actually an OS bug, and I'm continuing the investigation more on our side. However, Electron can work around the issue by ensuring that Child 2 doesn't initially receive the input. This can be accomplished in two ways:

  1. Ensure that Child 1 always exists - it takes priority over Child 2 in the hit-test. VSCode's workaround seems to do this.
  2. Prevent Child 2 from getting hit-tested - if it's not a hit-test candidate, then the hit-test will find the toplevel window if Child 1 doesn't exist.

Option 2 can be accomplished by hiding the window, setting its client rect to an empty rect, adding WS_EX_TRANSPARENT, or perhaps other ways - I'm not an expert in Win32 programming, so I don't have any authoritative advice. I don't know how Electron uses this window, either, so I don't have a strong recommendation. I just know that hit-testing to Child 2 is what is causing the issue.

If you'd like to chat more in-depth, feel free to email me at rnichols@microsoft.com.

@rossnichols

This comment has been minimized.

Copy link

commented Jun 14, 2018

@chaopeng FYI - see my previous comment.

@chaopeng

This comment has been minimized.

Copy link

commented Jun 14, 2018

Hi Ross, I have fixed the laggy issue on Chromium and patch to electron electron/libchromiumcontent#472

But it seems not working for vscode. I saw it fixed on a beta release atom. I suggest you test it on a plain electorn app (with open source electron release include my patch) see it can still reproduce first.

The root cause I found is the "Intermediate D3D Window" in front of "Chrome_RenderWidgetHostHWND".

The issue you saw seems related to chromium's aura window manager. When window (RenderWidgetHostHWND) hide we will move it to a hidden window (hidden Chrome_WidgetWin).Then add it back to the showing Chrome_WidgetWin when it show. In the case you seeing it seems the add to showing Chrome_WidgetWin skipped.

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2018

It looks like the new Windows 10 Insider Build 17751 fixes this issue for VSCode users (see microsoft/vscode#13612 (comment) and onwards).

@chaopeng

This comment has been minimized.

Copy link

commented Oct 27, 2018

Yes, that change maybe help for electron app too. Chromium CL is https://chromium-review.googlesource.com/c/chromium/src/+/1299342. The idea of that CL is changing D3D window to be a transparent layered window which can prevent the event hit test to D3D window. After this CL, we have known issue is: Min/Max/Close does not handle touch event properly https://bugs.chromium.org/p/chromium/issues/detail?id=897662. It should not effect electron.

Thank you Ross.

@chaopeng

This comment has been minimized.

Copy link

commented Oct 28, 2018

Forgot to mention, for fixing this issue only need the window style change. other changes only for cleanup. Check the patchset 1 for backport. https://chromium-review.googlesource.com/c/chromium/src/+/1299342/1

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

It would be great to get this fix into Electron 2.0.x 👍

@chaopeng

This comment has been minimized.

Copy link

commented Oct 29, 2018

The fix still not in chrome canary. I think we should we wait for a while see if any regression.

@ckerr

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

@chaopeng thanks for linking to this!

I'd like to see this in Canary first for exactly the reason you describe, but on first read this looks like a good backport candidate.

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2018

@chaopeng any update on how reliable this fix was? did it land in beta?

@chaopeng

This comment has been minimized.

Copy link

commented Nov 19, 2018

The fix landed in Chrome 72.0.3595.0. Currently in Canary channel, no regression for 25 days, will beta around mid Dec.

@chaopeng

This comment has been minimized.

Copy link

commented Nov 20, 2018

Bruce received a new reply from Microsoft.

The workaround from Chrome’s end : https://chromium-review.googlesource.com/c/chromium/src/+/1299342 is recommended for now in the existing OS.

We are introducing changes in the next version of Windows (19H1), which will make sure the issue does not happen. For RS4 and RS5, a change is still under review because, with the workaround (https://chromium-review.googlesource.com/c/chromium/src/+/1299342), the issue would not happen in those versions and implementing any OS-wide changes under these existing OS requires additional care so as to not break the existing functionalities.

So, in conclusion, we have a fix for 19H1 but in the existing RS4 and RS5 OS, we recommend the workaround from Chrome’s end.

https://bugs.chromium.org/p/chromium/issues/detail?id=871257#c85

@ThePlasmaRailgun

This comment has been minimized.

Copy link

commented Dec 14, 2018

I'm still observing this problem in Discord, there have been issues in a ton of different electron apps, (VS Code, Discord, CaretEditor). Is this a problem on each app's code or is it still in Electron itself?

@guilala

This comment has been minimized.

Copy link

commented Jan 16, 2019

Logitech Options driver / control panel software tries to use precision mouse scrolling for supported hardware. The control panel has an application- level "Smooth Scrolling" option. That option is enabled for Google Chrome 7x, but disabled for Visual Studio Code, to date. This would suggest that it's a feature / api that needs to be available for a running application. The difference also suggests that it's fixed in recent versions of Google Chrome. But what is it exactly, that feature / api? And in what version of Chrome (Chromium i hope) is it added. When someone can pinpoint that, instead of al those other things that have been merged, then we know when it would land in Electron. Am i wrong? I guess i am?..

edit: i think this issue should not have been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.