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

Manually pass mouse wheel messages to TermControls #5131

Merged
20 commits merged into from
Apr 1, 2020

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Mar 26, 2020

Summary of the Pull Request

As we've learned in #979, not all touchpads are created equal. Some of them have bad drivers that makes scrolling inactive windows not work. For whatever reason, these devices think the Terminal is all one giant inactive window, so we don't get the mouse wheel events through the XAML stack. We do however get the event as a WM_MOUSEWHEEL on those devices (a message we don't get on devices with normally functioning trackpads).

This PR attempts to take that WM_MOUSEWHEEL and manually dispatch it to the TermControl, so we can at least scroll the terminal content.

Unfortunately, this solution is not very general purpose. This only works to scroll controls that manually implement our own IMouseWheelListener interface. As we add more controls, we'll need to continue manually implementing this interface, until the underlying XAML Islands bug is fixed. I don't love this. I'd rather have a better solution, but it seems that we can't synthesize a more general-purpose PointerWheeled event that could get routed through the XAML tree as normal.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

I've also added a til::point conversion to winrt::Windows::Foundation::Point, and some scaling operators for point

Validation Steps Performed

  • It works on my HP Spectre 2017 with a synaptics trackpad
    • I also made sure to test that tmux works in panes on this laptop
  • It works on my slaptop, and DOESN'T follow this hack codepath on this machine.

zadjii and others added 9 commits March 24, 2020 11:12
  Just ask for the control underneath the mouse where it scrolled, and dispatch a scroll event to it if it matches the given interface.
…MMENSE PAIN trying to work on this project.

  This doesn't work, but it's taking me like 5x as long to do anything on this laptop
  Also for good measure, let's link this to #979
src/inc/til/point.h Outdated Show resolved Hide resolved
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally ok with where this is going. We need a 1.0 solution to this and it's not coming from the platform. It's not the worst, in my opinion, to be grabbing an extra message and sending it down. And I don't see us implementing that many controls on the main window that will need to implement your new interface to get the scroll. Not to mention... many common controls like a textbox don't really have any scrolling behavior so forgetting the interface on them won't be a big deal. I'd say finish this up and let's get it in.

Comment on lines +7 to +9
// This interface is a hack for GH#979. Controls should implement this
// interface to be able to be notified of mousewheel events even on devices
// who's trackpads won't scroll inactive windows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we file a CodeHealth issue to replace this when the XAML Islands bug gets fixed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just filed microsoft/microsoft-ui-xaml#2172 upstream to track them, I'll add another on us.

@zadjii-msft zadjii-msft marked this pull request as ready for review March 27, 2020 17:54
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/IslandWindow.cpp Show resolved Hide resolved
src/cascadia/WindowsTerminal/pch.h Show resolved Hide resolved
src/inc/til/point.h Show resolved Hide resolved
…-touchBAD

# Conflicts:
#	src/inc/til/point.h
#	src/til/ut_til/PointTests.cpp
@zadjii-msft zadjii-msft requested a review from miniksa April 1, 2020 13:03
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 1, 2020
@ghost
Copy link

ghost commented Apr 1, 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.

@ghost ghost merged commit a12a628 into master Apr 1, 2020
@ghost ghost deleted the dev/migrie/b/979-hp-touchBAD branch April 1, 2020 16:58
DHowett-MSFT pushed a commit that referenced this pull request Apr 21, 2020
Improve wide glyph support in UIA (GH-4946)
Add enhanced key support for ConPty (GH-5021)
Set DxRenderer non-text alias mode (GH-5149)
Reduce CursorChanged Events for Accessibility (GH-5196)
Add more object ID tracing for Accessibility (GH-5215)
Add SS3 cursor key encoding to ConPty (GH-5383)
UIA: Prevent crash from invalid UTR endpoint comparison (GH-5399)
Make CodepointWidthDetector::GetWidth faster (CC-3727)
add til::math, use it for float conversions to point, size (GH-5150)
Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI (GH-5353)
Fix a deadlock and a bounding rects issue in UIA (GH-5385)
Don't duplicate spaces from potentially-wrapped EOL-deferred lines (GH-5398)
Reimplement the VT tab stop functionality (CC-5173)
Clamp parameter values to a maximum of 32767. (CC-5200)
Prevent the cursor type being reset when changing the visibility (CC-5251)
Make RIS switch back to the main buffer (CC-5248)
Add support for the DSR-OS operating status report (CC-5300)
Update the virtual bottom location if the cursor moves below it (CC-5317)
ci: run spell check in CI, fix remaining issues (CC-4799) (CC-5352)
Set Cascadia Code as default font (GH-5121)
Show a double width cursor for double width characters (GH-5319)
Delegate all character input to the character event handler (CC-4192)
Update til::bitmap to use dynamic_bitset<> + libpopcnt (GH-5092)
Merged PR 4465022: [Git2Git] Merged PR 4464559: Console: Ingest OSS changes up to e055079
Correct scrolling invalidation region for tmux in pty w/ bitmap (GH-5122)
Render row-by-row instead of invalidating entire screen (GH-5185)
Make conechokey use ReadConsoleInputW by default (GH-5148)
Manually pass mouse wheel messages to TermControls (GH-5131)
This fixes C-M-space for WSL but not for Win32, but I'm not sure there's a problem in Win32 quite yet. (GH-5208)
Fix copying wrapped lines by implementing better scrolling (GH-5181)
Emit lines wrapped due to spaces at the end correctly (GH-5294)
Remove unneeded whitespace (CC-5162)
@ghost
Copy link

ghost commented Apr 22, 2020

🎉Windows Terminal Preview v0.11.1121.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Apr 22, 2020
@msbrunoalves
Copy link

Just tried the release, it's fixed on my HP laptop with Synaptics drivers, thank you!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Touchpad scrolling not possible
6 participants