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.

Sign up for GitHub

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

Jump to bottom

differential: selection gets struck multiple times while scrolling #5471

Closed
DHowett-MSFT opened this issue Apr 22, 2020 · 1 comment · Fixed by #5551
Closed

differential: selection gets struck multiple times while scrolling #5471

DHowett-MSFT opened this issue Apr 22, 2020 · 1 comment · Fixed by #5551
Assignees
@miniksa
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@DHowett-MSFT
Copy link
Contributor

image

Sometimes when scrolling the region covered by the selection also doesn't get redrawn -- sounds racey.
@DHowett-MSFT DHowett-MSFT added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Priority-2 A description (P2) labels Apr 22, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 22, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Apr 22, 2020
@miniksa miniksa self-assigned this Apr 22, 2020
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 22, 2020
@miniksa miniksa mentioned this issue Apr 24, 2020
Merged
8 tasks
@ghost ghost added the In-PR This issue has a related PR label Apr 24, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Apr 24, 2020
DHowett-MSFT pushed a commit that referenced this issue Apr 24, 2020
@miniksa
Lock when changing selection endpoint on wheel/auto-scroll (#5551)
98ac196 
Takes the lock inside two routines in `TermControl` that were changing
the selection endpoint while a rendering frame was still drawing,
resulting in several variants of graphical glitches from double-struck
selection boxes to duplicated line text.

## References
- Introduced with #5185

## PR Checklist
* [x] Closes #5471 
* [x] Already signed life away to company.
* [x] Manual tests passed since it's visual.
* [x] No extra doc besides the comments.
* [x] Am core contributor: Roar.

The renderer base and specific renderer engine do a lot of work to
remember the previous selection and compensate for scrolling regions and
deltas between frames. However, all that work doesn't quite match up
when the endpoints are changed out from under it. Unfortunately,
`TermControl` doesn't have a robust history of locking correctly in step
with the renderer nor does the renderer's `IRenderData` currently
provide any way of 'snapping' state at the beginning of a frame so it
could work without a full lock. So the solution for now is for the
methods that scroll the display in `TermControl` to take the lock that
is shared with the renderer's frame painter so they can't change out of
sync.

## Validation Steps Performed
- Opened terminal with Powershell core.
  Did ls a bunch of times.
  Clicked to make selection and held mouse button while wheeling around.
- Opened terminal with Powershell core.;
  Did ls a bunch of times.
  Clicked to make selection and dragged mouse outside the window to make
  auto scroll happen.
- Opened terminal with Powershell core.
  Did ls a bunch of times.
  Clicked to make selection and released. Wheeled around like a crazy
  person to make sure I didn't regress that.
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 24, 2020
@ghost
Copy link

ghost commented May 5, 2020

🎉This issue was addressed in #5551, which has now been successfully released as Windows Terminal Release Candidate v0.11.1251.0 (1.0rc1).:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@miniksa miniksa

Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Lock when changing selection endpoint on wheel/auto-scroll microsoft/terminal
2 participants
@DHowett-MSFT @miniksa