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

UIA: delay when reaching prompt with word navigation #5243

Closed
codeofdusk opened this issue Apr 5, 2020 · 13 comments
Closed

UIA: delay when reaching prompt with word navigation #5243

codeofdusk opened this issue Apr 5, 2020 · 13 comments
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conhost For issues in the Console codebase
Milestone
Windows vNext

Comments

@codeofdusk
Copy link
Contributor

Environment

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd] 1909 (18363.752)
Windows Terminal version (if applicable): Open Console (4f8acb4)

Any other software? NVDA, Narrator

Steps to reproduce

  1. Open Open Console.
  2. With either NVDA or Narrator, move to the start of the console and begin navigation by word.
  3. After reaching the word "reserved" (of the copyright text), move to the next word.

Expected behaviour

The prompt is quickly read.

Actual behaviour

A lag is observed before the prompt is read. A reduced lag (or none at all) is observed in Windows Terminal.
@ghost ghost 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 Apr 5, 2020
@codeofdusk
Copy link
Contributor Author

Cc @carlos-zamora.

@DHowett-MSFT DHowett-MSFT added the Area-Accessibility Issues related to accessibility label Apr 5, 2020
@DHowett-MSFT
Copy link
Contributor

This could be us scanning the entire 9001-line buffer for words. @codeofdusk If you change the buffer size from 120x9001 to 120x50 (in the legacy preferences panel), does the slowdown disappear?

@codeofdusk
Copy link
Contributor Author

This could be us scanning the entire 9001-line buffer for words. @codeofdusk If you change the buffer size from 120x9001 to 120x50 (in the legacy preferences panel), does the slowdown disappear?

Enabling "use legacy console" in properties completely disables UIA. The "buffer size" is already set to 50, should it be something else?

@codeofdusk
Copy link
Contributor Author

This could be us scanning the entire 9001-line buffer for words. @codeofdusk If you change the buffer size from 120x9001 to 120x50 (in the legacy preferences panel), does the slowdown disappear?

Enabling "use legacy console" in properties completely disables UIA. The "buffer size" is already set to 50, should it be something else?

Changing the buffer size from 50 to 500 has no effect.

@DHowett-MSFT
Copy link
Contributor

Ah, sorry, I meant the "traditional" console properties sheet. No need to actually check the "legacy" checkbox. That one switches back to the console host from Windows 8.1 before we renewed our work on it 😄

Just to confirm, you've got 50 or 500 in the box where I've got 9001:?

image

@codeofdusk
Copy link
Contributor Author

Ah, sorry, I meant the "traditional" console properties sheet. No need to actually check the "legacy" checkbox. That one switches back to the console host from Windows 8.1 before we renewed our work on it 😄

Just to confirm, you've got 50 or 500 in the box where I've got 9001:?

image

It looks like you've got an image with the alt text "image", what is this?

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Apr 5, 2020 •
edited
Loading

Sorry about that. It is a screenshot of the console properties page. The window is titled, "C:\windows\system32\cmd.exe properties"; there's a tab control with "Layout" selected as the active tab. There are three frames inside the tab control:

  • Screen Buffer Size
  • Window Size
  • Window Position

The Screen Buffer Size frame contains the label "Width" and a disabled spinner box containing the number 111; the label "Height" and an enabled spinner box containing the number 9001; and a ticked checkbox labelled "Wrap text output on resize."

There's another similar frame below Screen Buffer Size which is titled "Window Size". Since it's not the frame I am interested in, its contents are not meaningful right now.

(This is a great learning experience for me, and I apologize for providing inaccessible images. 😄)

@codeofdusk
Copy link
Contributor Author

Sorry about that. It is a screenshot of the console properties page. The window is titled, "C:\windows\system32\cmd.exe properties"; there's a tab control with "Layout" selected as the active tab. There are three frames inside the tab control:

  • Screen Buffer Size
  • Window Size
  • Window Position

The Screen Buffer Size frame contains the label "Width" and a disabled spinner box containing the number 111; the label "Height" and an enabled spinner box containing the number 9001; and a ticked checkbox labelled "Wrap text output on resize."

There's another similar frame below Screen Buffer Size which is titled "Window Size". Since it's not the frame I am interested in, its contents are not meaningful right now.

(This is a great learning experience for me, and I apologize for providing inaccessible images. 😄)

Oh I see now. Yeah I have 9001 there. Testing.

@codeofdusk
Copy link
Contributor Author

Sorry about that. It is a screenshot of the console properties page. The window is titled, "C:\windows\system32\cmd.exe properties"; there's a tab control with "Layout" selected as the active tab. There are three frames inside the tab control:

  • Screen Buffer Size
  • Window Size
  • Window Position

The Screen Buffer Size frame contains the label "Width" and a disabled spinner box containing the number 111; the label "Height" and an enabled spinner box containing the number 9001; and a ticked checkbox labelled "Wrap text output on resize."
There's another similar frame below Screen Buffer Size which is titled "Window Size". Since it's not the frame I am interested in, its contents are not meaningful right now.
(This is a great learning experience for me, and I apologize for providing inaccessible images. 😄)

Oh I see now. Yeah I have 9001 there. Testing.

Yes slowdown is gone when I change it to 50.

@DHowett-MSFT
Copy link
Contributor

This is excellent news. Thank you. @carlos-zamora, I believe we chatted about this. Because conhost doesn't know where the "actual" bottom of the text is, it scans forward almost 120x9001 characters to find out that the buffer consists of nothing but spaces. This is the optimization we talked about with Michael where totally empty rows could be skipped as containing "only spaces".

@DHowett-MSFT DHowett-MSFT added Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conhost For issues in the Console codebase labels Apr 5, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 5, 2020
@DHowett-MSFT
Copy link
Contributor

(The reason conhost doesn't know where the actual bottom of the buffer is is because a Win32 application can "address" the entire 9001 lines, even if they are not visible. It was a huge mistake to give win32 applications control over the entire buffer outside of the viewport, but it is a burden we need to live with.)

@carlos-zamora
Copy link
Member

Makes sense. Aside from that change, I’d be happy to do some more WPR traces on ConHost’s word navigation model. It’d be pretty fun actually haha

@DHowett-MSFT DHowett-MSFT added this to the 21H1 milestone Apr 7, 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 7, 2020
@carlos-zamora carlos-zamora self-assigned this Apr 13, 2020
@carlos-zamora carlos-zamora mentioned this issue Apr 18, 2020
Closed
1 task
@ghost ghost added the In-PR This issue has a related PR label Apr 18, 2020
@codeofdusk codeofdusk mentioned this issue May 21, 2020
Merged
@carlos-zamora carlos-zamora mentioned this issue Jul 20, 2020
Closed
This was referenced Sep 28, 2020
Merged
Merged
ghost pushed a commit that referenced this issue Sep 30, 2020
@carlos-zamora
Add optimization to get a11y next word (#7789)
386ae04 
This performs a minor refactor on `TextBuffer::MoveToNextWord` that
relies more heavily on `TextBuffer::GetWordEnd`. Now, the logic is
simplified and looks more like `MoveToPreviousWord`.

This refactor required me to move the `lastCharPos` optimization down to
`GetWordEnd`. So word expansion gets this optimization for free now.

### WPR Traces
The percentages below represent the weight that a function call had. The
test scenario included moving by word on the CMD welcome message until
the last word was reached. Inspect.exe was used to limit any additional
calls that are generally performed by a screen reader.

| function   | current | branch |
| --         | --      | --     |
| `UIA:Move` | 34.55%  | 29.52% |

There is an improvement of about 5% in a release build of ConHost.

NOTE: `UIA::Move` already calls `Expand` after a move operation is
performed. I'm using this data to represent a performance improvement
across both functions.

Contributes to #5243
DHowett pushed a commit that referenced this issue Oct 19, 2020
@carlos-zamora @DHowett
Add optimization to get a11y next word (#7789)
c66f8fe 
This performs a minor refactor on `TextBuffer::MoveToNextWord` that
relies more heavily on `TextBuffer::GetWordEnd`. Now, the logic is
simplified and looks more like `MoveToPreviousWord`.

This refactor required me to move the `lastCharPos` optimization down to
`GetWordEnd`. So word expansion gets this optimization for free now.

### WPR Traces
The percentages below represent the weight that a function call had. The
test scenario included moving by word on the CMD welcome message until
the last word was reached. Inspect.exe was used to limit any additional
calls that are generally performed by a screen reader.

| function   | current | branch |
| --         | --      | --     |
| `UIA:Move` | 34.55%  | 29.52% |

There is an improvement of about 5% in a release build of ConHost.

NOTE: `UIA::Move` already calls `Expand` after a move operation is
performed. I'm using this data to represent a performance improvement
across both functions.

Contributes to #5243

(cherry picked from commit 386ae04)
@carlos-zamora carlos-zamora mentioned this issue Mar 29, 2021
Open
@ghost ghost removed the In-PR This issue has a related PR label Mar 29, 2021
@carlos-zamora
Copy link
Member

Closing as a part of #7000 (particularly closed by #6986)

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Sep 17, 2021
@ABuffEr ABuffEr mentioned this issue Mar 7, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conhost For issues in the Console codebase
Projects
None yet
Milestone
Windows vNext
Development

Successfully merging a pull request may close this issue.

Constrain a11y word nav to the virtual bottom microsoft/terminal
3 participants
@codeofdusk @carlos-zamora @DHowett-MSFT