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

Account for WHEEL_DELTA when dispatching VT mouse wheel events #6843

Merged
3 commits merged into from
Jul 9, 2020

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Jul 8, 2020

By storing up the accumulated delta in the mouse input handler, we can
enlighten both conhost and terminal about wheel events that are less
than one line in size. Previously, we had a workaround in conhost that
clamped small scroll deltas to a whole line, which made trackpad
scrolling unimaginably fast. Terminal didn't make this mistake, but it
also didn't handle delta accumulation . . . which resulted in the same
behavior.

MouseInput will now wait until it's received WHEEL_DELTA (well-known
constant, value 120) worth of scrolling delta before it dispatches a
single scroll event.

Future considerations may include sending multiple wheel button events
for every multiple of WHEEL_DELTA, but that would be a slightly larger
refactoring that I'm not yet ready to undertake.

There's a chance that we should be dividing WHEEL_DELTA by the system's
"number of lines to scroll at once" setting, because on trackpads
conhost now scrolls a little slow. I think the only way to determine
whether this is palatable is to just ship it.

Fixes #6184.

By storing up the accumulated delta in the mouse input handler, we can
enlighten both conhost and terminal about wheel events that are less
than one line in size. Previously, we had a workaround in conhost that
clamped small scroll deltas to a whole line, which made trackpad
scrolling unimaginably fast. Terminal didn't make this mistake, but it
also didn't handle delta accumulation . . . which resulted in the same
behavior.

MouseInput will now wait until it's received WHEEL_DELTA (well-known
constant, value 120) worth of scrolling delta before it dispatches a
single scroll event.

Future considerations may include sending multiple wheel button events
for every *multiple* of WHEEL_DELTA, but that would be a slightly larger
refactoring that I'm not yet ready to undertake.

There's a chance that we should be dividing WHEEL_DELTA by the system's
"number of lines to scroll at once" setting, because on trackpads
conhost now scrolls a little _slow_. I think the only way to determine
whether this is palatable is to just ship it.

Fixes #6184.
@ghost ghost added Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jul 8, 2020
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 9, 2020
@ghost
Copy link

ghost commented Jul 9, 2020

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@DHowett
Copy link
Member Author

DHowett commented Jul 9, 2020

Pull request !175984 to OpenConsole states:

Trackpad events scroll properly now
WM_MOUSEWHEEL events use a wheel delta to tell an app how much they scrolled, with WHEEL_DELTA representing "one" scroll event. Trackpads however, frequently scroll in increments less than that. So now each scroll event gets a VT mode scroll event. Windows apps won't see a change, but VT mousemode apps will get many more scroll events on trackpads than a usual app, but it works now.

VT mousemode apps will get many more scroll events ... than a usual app

@zadjii-msft since you originally wrote the +1/-1 wheel delta workaround in windowio.cpp (input.cpp), do you additionally sign off on this being the right solution to that problem from 2016? 😉

@zadjii-msft
Copy link
Member

Lol yea, this is way better. Thanks for finally getting around to it 😄

@miniksa
Copy link
Member

miniksa commented Jul 9, 2020

Pull request !175984 to OpenConsole states:

Trackpad events scroll properly now
WM_MOUSEWHEEL events use a wheel delta to tell an app how much they scrolled, with WHEEL_DELTA representing "one" scroll event. Trackpads however, frequently scroll in increments less than that. So now each scroll event gets a VT mode scroll event. Windows apps won't see a change, but VT mousemode apps will get many more scroll events on trackpads than a usual app, but it works now.

VT mousemode apps will get many more scroll events ... than a usual app

@zadjii-msft since you originally wrote the +1/-1 wheel delta workaround in windowio.cpp (input.cpp), do you additionally sign off on this being the right solution to that problem from 2016? 😉

@DHowett, thanks for checking that.

@miniksa
Copy link
Member

miniksa commented Jul 9, 2020

@DHowett where's the test for your new Utility function? :P

@ghost ghost merged commit fc08329 into master Jul 9, 2020
@ghost ghost deleted the dev/duhowett/wheel_delta branch July 9, 2020 23:24
DHowett added a commit that referenced this pull request Jul 17, 2020
By storing up the accumulated delta in the mouse input handler, we can
enlighten both conhost and terminal about wheel events that are less
than one line in size. Previously, we had a workaround in conhost that
clamped small scroll deltas to a whole line, which made trackpad
scrolling unimaginably fast. Terminal didn't make this mistake, but it
also didn't handle delta accumulation . . . which resulted in the same
behavior.

MouseInput will now wait until it's received WHEEL_DELTA (well-known
constant, value 120) worth of scrolling delta before it dispatches a
single scroll event.

Future considerations may include sending multiple wheel button events
for every *multiple* of WHEEL_DELTA, but that would be a slightly larger
refactoring that I'm not yet ready to undertake.

There's a chance that we should be dividing WHEEL_DELTA by the system's
"number of lines to scroll at once" setting, because on trackpads
conhost now scrolls a little _slow_. I think the only way to determine
whether this is palatable is to just ship it.

Fixes #6184.

(cherry picked from commit fc08329)
@ghost
Copy link

ghost commented Jul 21, 2020

🎉Windows Terminal v1.1.2021.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal v1.1.2021.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal Preview v1.2.2022.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vim/Neovim/tmux scroll way too fast with trackpads
4 participants