Add optimization to get a11y next word #7789
Conversation
carlos-zamora
added
Area-Accessibility
|
Our tests have missed big issues before -- is there a way we can build trust in this? @codeofdusk I know you tested the combination of #7770 and #7789 before; were there any concerns? |
ghost
closed this
carlos-zamora
changed the base branch from
dev/cazamor/a11y/move-by-word-bugfix
to
master
carlos-zamora
force-pushed
the
dev/cazamor/a11y/next-word-opt
branch
from
b6fb987
to
3248d1b
Compare
|
Nope, works great! |
|
I recall word navigation feeling very sluggish. If this is noticeably better (like: is it usable???), I can easily sell it 😄 |
Certainly better, although please look into #5243.
To whom? |
As we get later in the Windows development cycle, we transition from feature work to bug work. After we move to bugs only, we start having to justify the fixes and their risks so that we're not introducing unnecessary/unhelpful changes as we get closer to release. 😄 |
| const auto bufferSize = GetSize(); | ||
| // move to the beginning of the next word | ||
| // NOTE: _GetWordEnd...() returns the exclusive position of the "end of the word" | ||
| // This is also the inclusive start of the next word. |
|
@msftbot merge this in 5 minutes |
ghost
added
the
AutoMerge
|
Hello @carlos-zamora! 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:
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
merged commit
|
This fix was just released as part of Windows in Insider Build 20236! |
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)
|
🎉 Handy links: |
|
🎉 Handy links: |
This performs a minor refactor on
TextBuffer::MoveToNextWordthatrelies more heavily on
TextBuffer::GetWordEnd. Now, the logic issimplified and looks more like
MoveToPreviousWord.This refactor required me to move the
lastCharPosoptimization down toGetWordEnd. 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.
UIA:MoveThere is an improvement of about 5% in a release build of ConHost.
NOTE:
UIA::Movealready callsExpandafter a move operation isperformed. I'm using this data to represent a performance improvement
across both functions.
Contributes to #5243