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

Improve performance of scrollbar marks #16006

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Sep 20, 2023

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 ✅

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Performance Performance-related issue Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Sep 20, 2023
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

neat

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Sep 20, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit b5333f6 into main Sep 20, 2023
17 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/lhecker/15955-optimize-scrollbar-marks branch September 20, 2023 15:03
carlos-zamora pushed a commit that referenced this pull request Sep 22, 2023
When launching a debug Terminal, `_initializedTerminal` might still be false and the scrollbar might still be 0px tall. This causes the `assert(false)` condition within `_throttledUpdateScrollbar` to be hit.

Regressed in #16006
DHowett pushed a commit that referenced this pull request Oct 17, 2023
Guess what _doesn't_ have the same layout as a bitmap? A `til::color`.

Noticed in 1.19.

Regressed in #16006
DHowett pushed a commit that referenced this pull request Oct 26, 2023
Guess what _doesn't_ have the same layout as a bitmap? A `til::color`.

Noticed in 1.19.

Regressed in #16006

(cherry picked from commit 1745857)
Service-Card-Id: 90758500
Service-Version: 1.19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Performance Performance-related issue 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. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.19+] Too many scrollbar marks slow down scrolling significantly
3 participants