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

[1.19+] Too many scrollbar marks slow down scrolling significantly #15955

Closed
Tracked by #15057
DHowett opened this issue Sep 11, 2023 · 2 comments · Fixed by #16006
Closed
Tracked by #15057

[1.19+] Too many scrollbar marks slow down scrolling significantly #15955

DHowett opened this issue Sep 11, 2023 · 2 comments · Fixed by #16006
Labels
Area-Performance Performance-related issue Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal.
Milestone
Terminal v1.20

Comments

@DHowett
Copy link
Member

DHowett commented Sep 11, 2023

I think this is what's happening. Fill a buffer and then search for a really short needle like e.

Scroll.

It will be really unusually slow.

It gets better if the needle is longer.
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 11, 2023
@DHowett
Copy link
Member Author

DHowett commented Sep 11, 2023

FWIW: The viewport has no bearing on what content exists for search :)

@lhecker
Copy link
Member

lhecker commented Sep 13, 2023

This doesn't happen due to searching the buffer again, but rather because WinUI struggles with the number of layers in the scrollbar canvas. I prototyped a solution based on my earlier bitmap proposal here: https://github.com/microsoft/terminal/compare/dev/lhecker/15955-optimize-scrollbar-marks
(It also includes a bug fix for ControlCore. The branch works but is still missing: A code cleanup, Shortening the canvas area by 16px on the top and bottom, The std::clamp code seems kinda bad)

@lhecker lhecker changed the title [1.19+] Search re-searches every time the viewport scrolls [1.19+] Too many scrollbar marks slow down scrolling significantly Sep 13, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.19 milestone Sep 18, 2023
@zadjii-msft zadjii-msft added Area-Performance Performance-related issue Issue-Bug It either shouldn't be doing this or needs an investigation. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Sep 18, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 18, 2023
@zadjii-msft zadjii-msft mentioned this issue Sep 19, 2023
Closed
@lhecker lhecker mentioned this issue Sep 20, 2023
Merged
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Sep 20, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue Sep 20, 2023
@lhecker
Improve performance of scrollbar marks (#16006)
b5333f6 
This replaces the use of a `<Canvas>` with an `<Image>` for drawing
scrollbar marks. Otherwise, WinUI struggles with the up to ~9000 UI
elements as they get dirtied every time the scrollbar moves.
(FWIW 9000 is not a lot and it should not struggle with that.)

The `<Image>` element has the benefit that we can get hold of a CPU-side
bitmap which we can manually draw our marks into and then swap them into
the UI tree. It draws the same 9000 elements, but now WinUI doesn't
struggle anymore because only 1 element gets invalidated every time.

Closes #15955

## Validation Steps Performed
* Fill the buffer with "e"
* Searching for "e" fills the entire thumb range with white ✅
* ...doesn't lag when scrolling around ✅
* ...updates quickly when adding newlines at the end ✅
* Marks sort of align with their scroll position ✅
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Performance Performance-related issue Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Terminal v1.20
Development

Successfully merging a pull request may close this issue.

Improve performance of scrollbar marks microsoft/terminal
3 participants
@DHowett @lhecker @zadjii-msft