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 mouse coordinates when viewport is scrolled for all events, not just pressed #11290

Merged
1 commit merged into from
Sep 22, 2021

Conversation

PankajBhojwani
Copy link
Contributor

@PankajBhojwani PankajBhojwani commented Sep 20, 2021

Does the mouse coordinate adjustment added in #10642 for all the other mouse events as well (moved, released, wheel)

Closes #10190

@ghost ghost added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Sep 20, 2021
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Wow, i'm surprised we did not have such a helper before!

@DHowett
Copy link
Member

DHowett commented Sep 22, 2021

@msftbot merge this in 1 minute

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Sep 22, 2021
@ghost
Copy link

ghost commented Sep 22, 2021

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Wed, 22 Sep 2021 19:39:35 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@ghost ghost merged commit f04fd08 into main Sep 22, 2021
@ghost ghost deleted the dev/pabhoj/mouse_viewport branch September 22, 2021 19:41
miniksa pushed a commit that referenced this pull request Sep 27, 2021
…ust pressed (#11290)

Does the mouse coordinate adjustment added in #10642 for all the other mouse events as well (moved, released, wheel)

Closes #10190
miniksa pushed a commit that referenced this pull request Sep 27, 2021
…ust pressed (#11290)

Does the mouse coordinate adjustment added in #10642 for all the other mouse events as well (moved, released, wheel)

Closes #10190
@ghost
Copy link

ghost commented Oct 4, 2021

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

Handy links:

@ghost
Copy link

ghost commented Oct 4, 2021

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

Handy links:

@ghost
Copy link

ghost commented Oct 20, 2021

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

Handy links:

DHowett added a commit that referenced this pull request Aug 23, 2024
PR #10642 and #11290 introduced an adjustment for the cursor position
used to generate VT mouse mode events.

One of the decisions made in those PRs was to only send coordinates
where Y was >= 0, so if you were off the top of the screen you wouldn't
get any events. However, terminal emulators are expected to send
_clamped_ events when the mouse is off the screen. This decision broke
clamping Y to 0 when the mouse was above the screen.

The other decision was to only adjust the Y coordinate if the core's
`ScrollOffset` was greater than 0. It turns out that `ScrollOffset` _is
0_ when you are scrolled all the way back in teh buffer. With this
check, we would clamp coordinates properly _until the top line of the
scrollback was visible_, at which point we would send those coordinates
over directly. This resulted in the same weird behavior as observed in
#10190.

I've fixed both of those things. Core is expected to receive negative
coordinates and clamp them to the viewport. ScrollOffset should never be
below 0, as it refers to the top visible buffer line.

In addition to that, #17744 uncovered that we were allowing
autoscrolling to happen even when VT mouse events were being generated.
I added a way for `ControlInteractivity` to halt further event
processing. It's crude.

Refs #10190
Closes #17744
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.) 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-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Win32 mouse coordinates wrong when viewport scrolled
3 participants