This PR merges the default colors and cursor color into the main color
table, enabling us to simplify the `ConGetSet` and `ITerminalApi`
interfaces, with just two methods required for getting and setting any
form of color palette entry.
The is a follow-up to the color table standardization in #11602, and a
another small step towards de-duplicating `AdaptDispatch` and
`TerminalDispatch` for issue #3849. It should also make it easier to
support color queries (#3718) and a configurable bold color (#5682) in
the future.
On the conhost side, default colors could originally be either indexed
positions in the 16-color table, or separate standalone RGB values. With
the new system, the default colors will always be in the color table, so
we just need to track their index positions.
To make this work, those positions need to be calculated at startup
based on the loaded registry/shortcut settings, and updated when
settings are changed (this is handled in
`CalculateDefaultColorIndices`). But the plus side is that it's now much
easier to lookup the default color values for rendering.
For now the default colors in Windows Terminal use hardcoded positions,
because it doesn't need indexed default colors like conhost. But in the
future I'd like to extend the index handling to both terminals, so we
can eventually support the VT525 indexed color operations.
As for the cursor color, that was previously stored in the `Cursor`
class, which meant that it needed to be copied around in various places
where cursors were being instantiated. Now that it's managed separately
in the color table, a lot of that code is no longer required.
## Validation
Some of the unit test initialization code needed to be updated to setup
the color table and default index values as required for the new system.
There were also some adjustments needed to account for API changes, in
particular for methods that now take index values for the default colors
in place of COLORREFs. But for the most part, the essential behavior of
the tests remains unchanged.
I've also run a variety of manual tests looking at the legacy console
APIs as well as the various VT color sequences, and checking that
everything works as expected when color schemes are changed, both in
Windows Terminal and conhost, and in the latter case with both indexed
colors and RGB values.
Closes#11768
This commit introduces "AtlasEngine", a new text renderer based on DxEngine.
But unlike it, DirectWrite and Direct2D are only used to rasterize glyphs.
Blending and placing these glyphs into the target view is being done using
Direct3D and a simple HLSL shader. Since this new renderer more aggressively
assumes that the text is monospace, it simplifies the implementation:
The viewport is divided into cells, and its data is stored as a simple matrix.
Modifications to this matrix involve only simple pointer arithmetic and is easy
to understand. But just like with DxEngine however, DirectWrite
related code remains extremely complex and hard to understand.
Supported features:
* Basic text rendering with grayscale AA
* Foreground and background colors
* Emojis, including zero width joiners
* Underline, dotted underline, strikethrough
* Custom font axes and features
* Selections
* All cursor styles
* Full alpha support for all colors
* _Should_ work with Windows 7
Unsupported features:
* A more conservative GPU memory usage
The backing texture atlas for glyphs is grow-only and will not shrink.
After 256MB of memory is used up (~20k glyphs) text output
will be broken until the renderer is restarted.
* ClearType
* Remaining gridlines (left, right, top, bottom, double underline)
* Hyperlinks don't get full underlines if hovered in WT
* Softfonts
* Non-default line renditions
Performance:
* Runs at up to native display refresh rate
Unfortunately the frame rate often drops below refresh rate, due us
fighting over the buffer lock with other parts of the application.
* CPU consumption is up to halved compared to DxEngine
AtlasEngine is still highly unoptimized. Glyph hashing
consumes up to a third of the current CPU time.
* No regressions in WT performance
VT parsing and related buffer management takes up most of the CPU time (~85%),
due to which the AtlasEngine can't show any further improvements.
* ~2x improvement in raw text throughput in OpenConsole
compared to DxEngine running at 144 FPS
* ≥10x improvement in colored VT output in WT/OpenConsole
compared to DxEngine running at 144 FPS
## Summary of the Pull Request
In the original implementation, we used two different orderings for the color tables. The WT color table used ANSI order, while the conhost color table used a Windows-specific order. This PR standardizes on the ANSI color order everywhere, so the usage of indexed colors is consistent across both parts of the code base, which will hopefully allow more of the code to be shared one day.
## References
This is another small step towards de-duplicating `AdaptDispatch` and `TerminalDispatch` for issue #3849, and is essentially a followup to the SGR dispatch refactoring in PR #6728.
## PR Checklist
* [x] Closes#11461
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #11461
## Detailed Description of the Pull Request / Additional comments
Conhost still needs to deal with legacy attributes using Windows color order, so those values now need to be transposed to ANSI colors order when creating a `TextAttribute` object. This is done with a simple mapping table, which also handles the translation of the default color entries, so it's actually slightly faster than the original code.
And when converting `TextAttribute` values back to legacy console attributes, we were already using a mapping table to handle the narrowing of 256-color values down to 16 colors, so we just needed to adjust that table to account for the translation from ANSI to Windows, and then could make use of the same table for both 256-color and 16-color values.
There are also a few places in conhost that read from or write to the color tables, and those now need to transpose the index values. I've addressed this by creating separate `SetLegacyColorTableEntry` and `GetLegacyColorTableEntry` methods in the `Settings` class which take care of the mapping, so it's now clearer in which cases the code is dealing with legacy values, and which are ANSI values.
These methods are used in the `SetConsoleScreenBufferInfoEx` and `GetConsoleScreenBufferInfoEx` APIs, as well as a few place where color preferences are handled (the registry, shortcut links, and the properties dialog), none of which are particularly sensitive to performance. However, we also use the legacy table when looking up the default colors for rendering (which happens a lot), so I've refactored that code so the default color calculations now only occur once per frame.
The plus side of all of this is that the VT code doesn't need to do the index translation anymore, so we can finally get rid of all the calls to `XTermToWindowsIndex`, and we no longer need a separate color table initialization method for conhost, so I was able to merge a number of color initialization methods into one. We also no longer need to translate from legacy values to ANSI when generating VT sequences for conpty.
The one exception to that is the 16-color VT renderer, which uses the `TextColor::GetLegacyIndex` method to approximate 16-color equivalents for RGB and 256-color values. Since that method returns a legacy index, it still needs to be translated to ANSI before it can be used in a VT sequence. But this should be no worse than it was before.
One more special case is conhost's secret _Color Selection_ feature. That uses `Ctrl`+Number and `Alt`+Number key sequences to highlight parts of the buffer, and the mapping from number to color is based on the Windows color order. So that mapping now needs to be transposed, but that's also not performance sensitive.
The only thing that I haven't bothered to update is the trace logging code in the `Telemetry` class, which logs the first 16 entries in the color table. Those entries are now going to be in a different order, but I didn't think that would be of great concern to anyone.
## Validation Steps Performed
A lot of unit tests needed to be updated to use ANSI color constants when setting indexed colors, where before they might have been expecting values in Windows order. But this replaced a wild mix of different constants, sometimes having to use bit shifting, as well as values mapped with `XTermToWindowsIndex`, so I think the tests are a whole lot clearer now. Only a few cases have been left with literal numbers where that seemed more appropriate.
In addition to getting the unit tests working, I've also manually tested the behaviour of all the console APIs which I thought could be affected by these changes, and confirmed that they produced the same results in the new code as they did in the original implementation.
This includes:
- `WriteConsoleOutput`
- `ReadConsoleOutput`
- `SetConsoleTextAttribute` with `WriteConsoleOutputCharacter`
- `FillConsoleOutputAttribute` and `FillConsoleOutputCharacter`
- `ScrollConsoleScreenBuffer`
- `GetConsoleScreenBufferInfo`
- `GetConsoleScreenBufferInfoEx`
- `SetConsoleScreenBufferInfoEx`
I've also manually tested changing colors via the console properties menu, the registry, and shortcut links, including setting default colors and popup colors. And I've tested that the "Quirks Mode" is still working as expected in PowerShell.
In terms of performance, I wrote a little test app that filled a 80x9999 buffer with random color combinations using `WriteConsoleOutput`, which I figured was likely to be the most performance sensitive call, and I think it now actually performs slightly better than the original implementation.
I've also tested similar code - just filling the visible window - with SGR VT sequences of various types, and the performance seems about the same as it was before.
This fixes an issue that Touch Keyboard is not invoked when user taps on the PowerShell.
Before this change, it was returning small rectangle on the right of the cursor. Touch Keyboard should be invoked by tapping anywhere inside the console.
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
ITfContextOwner::GetScreenExt is used to define rectangle that can invoke Touch Keyboard.
https://docs.microsoft.com/en-us/windows/win32/api/msctf/nf-msctf-itfcontextowner-getscreenext
## Validation Steps Performed
* [x] Touch keyboard was invoked by tapping inside the Console while Hardware Keyboard was not attached.
* [x] Selecting text worked as expected without invoking touch keyboard.
* [x] Long tapping the console invoked Touch Keyboard. I would like to confirm if this is the expected behavior.
## Summary of the Pull Request
This replaces the `GridLines` enum in the renderers with a `til::enumset` type, avoiding the need for the various `WI_IsFlagSet` macros and flag operators.
## References
This is followup to PR #10492 which introduced the `enumset` class.
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.
## Validation Steps Performed
I've manually confirmed that all the different gridlines are still rendering correctly in both the GDI and DX renderers.
## Summary of the Pull Request
Fixes the 24 failing generated tests. 20 of them were fixed by enforcing the following rule: when moving backwards by word...
- a degenerate range moves to the beginning of the word, then to the word behind it.
- a non-degenerate range outright moves to the word behind it.
The fix was simple: if we're a degenerate range, check if we're at the beginning of the word. If not, move there. Otherwise, move to the word before it. See UiaTextRangeBase.cpp changes for implementation details.
Along the way, several misauthored tests were found:
- 2 generated tests:
- Cause: MS Word considers a line break a word delimiter. We don't use line-wrapping to distinguish two separate words.
- `MovementAtExclusiveEnd` backwards word movement tests:
- `end` will always be `writeTarget` because...
- [degenerate range case] both `start` and `end` are moved to the beginning of the word (`writeTarget`)
- [non-degenerate range case] from the `UiaTextRangeBase` bugfix, we should be moving to the word behind it.
- this misauthored test was explicitly found by fixing the bug first explained here.
## References
#10925 Word navigation testing
## Summary of the Pull Request
Updates our `UiaTextRange` to no longer treat the end of the buffer as the "document end". Instead, we consider the "document end" to be the line beneath the cursor or last legible character (whichever is further down). In the event where the last legible character is on the last line of the buffer, we use the "end exclusive" position (left-most point on a line one past the end of the buffer).
When movement of any kind occurs, we clamp each endpoint to the document end. Since the document end is an actual spot in the buffer (most of the time), this should improve stability because we shouldn't be pointing out-of-bounds anymore.
The biggest benefit is that this significantly improves the performance of word navigation because screen readers no longer have to take into account the whitespace following the end of the prompt.
Word navigation tests were added to the `TestTableWriter` (see #10886). 24 of the 85 tests were failing, however, they don't seem to interact with the document end, so I've marked them as skip and will fix them in a follow-up. This PR is large enough as-is, so I'm hoping I can take time in the follow-up to clean some things on the side (aka `preventBoundary` and `allowBottomExclusive` being used interchangeably).
## References
#7000 - Epic
Closes#6986Closes#10925
## Validation Steps Performed
- [X] Tests pass
- [X] @codeofdusk has been personally testing this build (and others)
## Summary of the Pull Request
Follow-up for #10886. The new UIA movement tests found some failing cases. This PR fixes UiaTextRangeBase to have movement match that of MS Word. In total, this fixes 64 tests.
## PR Checklist
* [X] Closes#10924
* [X] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
Root causes include...
1. if we were a non-degenerate range and we failed to move, we should still expand to enclose the unit
2. non-degenerate ranges are treated as if they already encompassed their given unit.
- this one is a bit difficult to explain. Consider these examples:
1. document movement
- state: you have a 1-cell wide range on the buffer, and you try to move by document
- result: move by 0 (there is no next/prev document), but the range now encompasses the entire document
2. line movement
- state: you have a 1-cell wide range on a line, and you try to move back by a line
- result: you go to the previous line (not the beginning of this line)
- conversely, a degenerate range successfully moves to the beginning/end of the current unit (i.e. document/line)
- this (bizarre) behavior was confirmed using MS Word
As a bonus, occasionally, Narrator would get stuck when navigating by line. This issue now seems to be fixed.
## Updates to existing tests
- `CanMoveByCharacter`
- `can't move backward from (0, 0)` --> misauthored, result should be one character wide.
- `can't move past the last column in the last row` --> misauthored and already covered in generated tests
- `CanMoveByLine`
- `can't move backward from top row` --> misauthored, end should be on next line. Already covered by generated tests
- `can't move forward from bottom row` --> misauthored, end should be on next line
- `can't move backward when part of the top row is in the range` --> misauthored, should expand
- `can't move forward when part of the bottom row is in the range` --> misauthored, degenerate range moves to end of buffer
- `MovementAtExclusiveEnd`
- populate the text buffer _before_ we do a move by word operation
- update to match the now fixed behavior
Introduces a new methodology to maintain tests for UI Automation. This includes...
- `UiaTests.csv`: an excel spreadsheet designed to store UIA movement tests in a compact format
- `GeneratedTests.ps1`: a PowerShell script that imports `UiaTests.csv` and outputs a C++ TEST_METHOD for `UiaTextRangeTests.
This new system can be used to easily add more UIA movement tests.
Read https://github.com/microsoft/terminal/blob/dev/cazamor/a11y-7000/testing/tools/TestTableWriter/README.md for more details.
Follow-up work items:
- #10924 **Failing Tests**: this found some failing tests. We should make them not fail.
- #10925 **Missing Tests: Word navigation**: Word navigation is missing.
- #10926 **MoveEndpoint Tests**: an additional column can be added to the CSV "EndpointTarget", which can be "start", "end", or "both". This will allow us to test `MoveEndpoint` in addition to `Move`.
This PR adds conhost support for downloadable soft fonts - also known as
dynamically redefinable character sets (DRCS) - using the `DECDLD`
escape sequence.
These fonts are typically designed to work on a specific terminal model,
and each model tends to have a different character cell size. So in
order to support as many models as possible, the code attempts to detect
the original target size of the font, and then scale the glyphs to fit
our current cell size.
Once a font has been downloaded to the terminal, it can be designated in
the same way you would a standard character set, using an `SCS` escape
sequence. The identification string for the set is defined by the
`DECDLD` sequence. Internally we map the characters in this set to code
points `U+EF20` to `U+EF7F` in the Unicode private use are (PUA).
Then in the renderer, any characters in that range are split off into
separate runs, which get painted with a special font. The font itself is
dynamically generated as an in-memory resource, constructed from the
downloaded character bitmaps which have been scaled to the appropriate
size.
If no soft fonts are in use, then no mapping of the PUA code points will
take place, so this shouldn't interfere with anyone using those code
points for something else, as along as they aren't also trying to use
soft fonts. I also tried to pick a PUA range that hadn't already been
snatched up by Nerd Fonts, but if we do receive reports of a conflict,
it's easy enough to change.
## Validation Steps Performed
I added an adapter test that runs through a bunch of parameter
variations for the `DECDLD` sequence, to make sure we're correctly
detecting the font sizes for most of the known DEC terminal models.
I've also tested manually on a wide range of existing fonts, of varying
dimensions, and from multiple sources, and made sure they all worked
reasonably well.
Closes#9164
This commit fixes the UseDx mode for conhost.
In order to add support for UseDx without calling `SetWindowSize`,
responsibility for resizing `_invalidMap` has been moved to occur
only when the renderer itself recognizes a new size. Furthermore
`InvalidateAll` is now the central point to invalidate `_invalidMap`.
## Validation Steps Performed
* Enabling `UseDx` enables the DxEngine for conhost ✔️
* Resizing windows in conhost works ✔️
* Resizing windows in WT works ✔️Closes#5455
Visual Studio 2022 Preview recently released the v143 toolchain.
C4189 is now flagging several unused variables, which breaks our build.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* CascadiaPackage builds ✔️
* All tests build ✔️
## Summary of the Pull Request
This implements `GetAttributeValue` and `FindAttribute` for `UiaTextRangeBase` (the shared `ITextRangeProvider` for Conhost and Windows Terminal). This also updates `UiaTracing` to collect more useful information on these function calls.
## References
#7000 - Epic
[Text Attribute Identifiers](https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids)
[ITextRangeProvider::GetAttributeValue](https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-getattributevalue)
[ITextRangeProvider::FindAttribute](https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-findattribute)
## PR Checklist
* [X] Closes#2161
* [X] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
- `TextBuffer`:
- Exposes a new `TextBufferCellIterator` that takes in an end position. This simplifies the logic drastically as we can now use this iterator to navigate through the text buffer. The iterator can also expose the position in the buffer.
- `UiaTextRangeBase`:
- Shared logic & helper functions:
- Most of the text attributes are stored as `TextAttribute`s in the text buffer. To extract them, we generate an attribute verification function via `_getAttrVerificationFn()`, then use that to verify if a given cell has the desired attribute.
- A few attributes are special (i.e. font name, font size, and "is read only"), in that they are (1) acquired differently and (2) consistent across the entire text buffer. These are handled separate from the attribute verification function.
- `GetAttributeValue`: Retrieve the attribute verification of the first cell in the range. Then, verify that the entire range has that attribute by iterating through the text range. If a cell does not have that attribute, return the "reserved mixed attribute value".
- `FindAttribute`: Iterate through the text range and leverage the attribute verification function to find the first contiguous range with that attribute. Then, make the end exclusive and output a `UiaTextRangeBase`. This function must be able to perform a search backwards, so we abstract the "start" and "end" into `resultFirstAnchor` and `resultSecondAnchor`, then perform post processing to output a valid `UiaTextRangeBase`.
- `UiaTracing`:
- `GetAttributeValue`: Log uia text range, desired attribute, resulting attribute metadata, and the type of the result.
- `FindAttribute`: Log uia text range, desired attribute and attribute metadata, if we were searching backwards, the type of the result, and the resulting text range.
- `AttributeType` is a nice way to understand/record if the result was either of the reserved UIA values, a normal result, or an error.
- `UiaTextRangeTests`:
- `GetAttributeValue`:
- verify that we know which attributes we support
- test each of the known text attributes (expecting 100% code coverage for `_getAttrVerificationFn()`)
- `FindAttribute`:
- test each of the known _special_ text attributes
- test `IsItalic`. NOTE: I'm explicitly only testing one of the standard text attributes because the logic is largely the same between all of them and they leverage `_getAttrVerificationFn()`.
## Validation Steps Performed
- @codeofdusk has been testing this Conhost build
- Tests added for Conhost and shared implementation
- Windows Terminal changes were manually verified using accessibility insights and NVDA
Change accessibility notifier creation so we do not create one when we're in PTY mode. (Guard all call sites to skip math/event work when the notifier is null.) MSAA events are legacy events that are registered for globally and used by some screen readers to find content in the conhost window. The PTY mode is not responsible for hosting the display content or input window, so it makes sense for it to not broadcast these events and delegate the accessibility requirement to the connected terminal.
## References
- #10537
## PR Checklist
* [x] Closes#10568
* [x] I work here
* [x] Manual test launches passed.
## Summary of the Pull Request
#7960 was caused by `UiaTextRangeBase::_blockRange` not being initialized, thus pointing to random memory. In most cases, we initialize it properly in `RuntimeClassInitialize`, however, the copying version of `RuntimeClassInitialize` doesn't actually copy it over, resulting in it still containing random memory.
NVDA (and other screen readers) occasionally use `Clone` (really just the copy initializer), resulting in this bug occurring randomly.
## PR Checklist
* [X] Closes#7960
* [X] Tests added/passed
## Validation Steps Performed
Test failed before the change, but passes after the change.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 40df712b33a40e76aeeac87f823bbb028d2e3972
Related work items: MSFT-32007459
Persist inbox conhost; delegate control activities to it via a pipe
## PR Checklist
* [x] Closes#10194 - WSL Debug Tap doesn't work
* [x] Closes#10134 - WSL Parameter is Incorrect
* [x] Closes#10413 - Ctrl+C not passed to client
* [x] Closes#10414 - Leftover processes on abrupt termination
* [x] Might help #10251 - Win+X Powershell sometimes fails to attach
* [x] I work here
* [x] Manually tested with assorted launch scenarios
## Detailed Description of the Pull Request / Additional comments
It turns out that there's a bit of ownership that goes on with the original inbox `conhost.exe` and the operating system/driver. The PID of that original `conhost.exe` is stowed when the initial connection is established and it is verified for several activities. This means that the plan of letting it go completely away and having the `OpenConsole.exe` take over all of its activities must be slightly revised.
I have tested the following two alternatives to keeping `conhost.exe` around and they do not work:
1. Replacing the original owner `conhost.exe` with `OpenConsole.exe` - A.) The driver does not allow this. Once the owner is registered, it cannot be replaced. B.) There's no way of updating this information inside the client process space and it is kept there too in the `kernelbase`/`conclnt` data from its initial connection.
2. Attempting to pick up the first packet (to determine headed/headless and other initial connection information that we use to determine whether handoff is appropriate or not) prior to registering any owner at all. - The driver doesn't allow this either. The owner must be registered prior to a packet coming through.
Put this mental model in your head:
CMD --> Conhost (inbox) --> OpenConsole (WT Package) --> Terminal (WT Package)
So since the `conhost.exe` needs to stick around, here's what I'm doing in this PR:
- `conhost.exe` in the OS will receive back the `OpenConsole.exe` process handle on a successful handoff and is expected to remain alive until the `OpenConsole.exe` exits. It's now waiting on that before it terminates itself.
- `conhost.exe` in the OS will establish a signal channel pipe and listen for control commands from `OpenConsole.exe` in a very similar fashion to how the `ConPTY` signal pipe operates between the Terminal and the PTY (provided by `OpenConsole.exe` in this particular example.) When `OpenConsole.exe` needs to do something that would be verified by the OS and rejected... it will instead signal the original `conhost.exe` to do that thing and it will go through.
- `conhost.exe` will give its own handle through to `OpenConsole.exe` so it can monitor its lifetime and cleanup. If the owner is gone, the session should end.
- Assorted handle cleanup that was leading to improper exits. I was confused between `.reset()` and `.release()` for some of the `wil::unique_any<T>` handling and it lead to leaked handles. The leaked handles meant that threads weren't aware of the other sides collapsing and wouldn't cleanup/terminate appropriately.
How does this fix things?
- For the WSL cases... WSL was specifically looking up the owner PID of the console session from the driver. That was the `conhost.exe` PID. If it exits, that PID isn't valid and is recycled. Thus the parameter is incorrect or other inappropriate WSL setup behaviors.
- Ctrl+C not passed... this is a signal the operating system rejects from a PID that is not the owner. This is now relayed through the original owner and it works.
- Leftover processes... I believe I explained this was both not-enough-monitoring of each others' process lifetimes coupled with mishandling of release/resetting handles and leaking them.
- Powershell sometimes fails to attach... my theory on this one is that it's a race that became upset when the `conhost.exe` disappeared while something about Powershell/.NET was still starting, much like the WSL one. I believe now that it is sticking around, it will be fine.
Also, this WILL require an OS update to complete improvement of functionality and I have revised the interface ID. This is considered an acceptable breaking change with no mitigation because we said this feature was an alpha preview.
## Validation Steps Performed
- Launched WSL with defapp set, it works
- Launched WSL with defapp set and the debug tap on, it works and opens in two tabs
- Launched CMD, ran ping, did Ctrl+C, it now receives it
- Launched Win+X powershell a ton of times. It seems fine now
- Launched cmd, powershell, wsl, etc. Killed assorted processes in the chain (client/conhost/openconsole/windowsterminal) and observed in Process Explorer (with a long delta timer so I could see it) that they all successfully tear down now without leftovers.
This pull request converts four of our existing `#ifdef` (or `#ifndef`)
`INSIDE_WINDOWS` blocks to til::features:
* Attempting to establish a handoff session (inside Windows only)
* The ability to *receive* a handoff session (outside Windows only)
* The DX engine (outside Windows only) and shaders (also outside only)
* Whether we use numpad event synthesis for clipboard/conpty (inside
Windows only)
Most of these are using the preprocessor verison of til::feature, only
because it is more difficult to gate the inclusion of headers on
constant expressions. I'd love to prefer the compile time version.
When we encounter clipboard input that cannot be mapped to a keyboard
key, we try our best to map it to an event. If if is an alphanumeric
character or a wide glyph (per the old width algorithm), we will pass it
through as the UnicodeChar of a key event. If it's *not* wide and *not*
alphanumeric, we will synthesize a set of Alt+NumPad events.
Those events comprise a set containing...
1. Alt Down (modifiers = LAlt, UnicodeChar = 0)
2. Numpad 0 Down/Up (modifiers = LAlt, UnicodeChar = 0)
3. Numpad 1 Down/Up (modifiers = LAlt, UnicodeChar = 0)
4. Numpad 2 Down/Up (modifiers = LAlt, UnicodeChar = 0)
5. Alt Up (modifiers = 0, UnicodeChar = [THE CHARACTER])
Because of event group 5, application developers seem to have taken a
dependency on receiving Alt Up + Character and don't seek to recompose
the original character from its numpad representation.
In pull request GH-8035 (!5394370), we stripped the old width algorithm
out and replaced it with a lookup table (finally!). Unfortunately, that
broke clipboard input for Chinese text as it was no longer considered
"Wide" for the purposes of detecting whether we should use numpad
events.
This commit introduces a version of GetQuickCharWidth that fulfills the
exact contract CharToKeyEvents needs, and doesn't answer for a codepoint
more. We'll use it in Windows to fix MSFT-32901370.
The Terminal analogue of this bug, GH-9052, is fixed by *never emitting
numpad events again.* We think this is okay because it looks like nobody
was ever handling numpad events... and that we were only using them as a
way to communicate within conhost (which we're *also* not using) and any
public exposition of event 5 as a contract was unintended.
VALIDATION
----------
I took this new implementation (with an early return) and the old
implementation and compared whether they would yield a numpad event or a
key event over the entire supported codepoint space [0000..FFFF].
They matched.
Fixes MSFT-32901370
Fixes GH-9052
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 224751bbb061f5f59d794c2c9bdac5a9674ebde6
Add names to threads to make debugging a slight bit easier.
## PR Checklist
* [x] Closes personal todo item.
* [x] I work here.
* [x] Tested manually.
## Detailed Description of the Pull Request / Additional comments
Thread descriptions show up as names in both the Visual Studio debugger, WinDBG debugger, and Windows Performance Analyzer. This makes it faster and easier to identify threads of interest in our processes.
## Validation Steps Performed
* [x] Checked threads were named in OpenConsole.exe running in classic conhost window mode under VS debug
* [x] Checked threads were named in OpenConsole.exe running in conpty mode under VS debug
* [x] Checked threads were named in WindowsTerminal.exe (for a few of the threads around connections)
* [x] Checked that we could also see it in WinDBG
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This change cleans up the Fullscreen implementation for both conhost and Terminal, improving the restore position (where the window goes when exiting fullscreen).
Prior to this change the window wasn't guaranteed to restore somewhere on the window's current monitor when exiting fullscreen. With this change the window will restore always to its current monitor, at a reasonable location (and will 'double restore' (to fullscreen->maximize->restore) after monitor changes while fullscreen, which is the expected user behavior.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#9746
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
A fullscreen window's monitor can change.
- Win+Shift+left/right migrates a window between monitors.
- User could open settings, display, and move the monitor or change its DPI.
- The monitor could be unplugged.
- The session could be remote and be disconnected.
A fullscreen window stores a 'restore position' when entering fullscreen, used to move the window back 'where it was'. BUT, its unexpected for the window to exit fullscreen and jump to another monitor. This means its previous position must be migrated from the old monitor's work area to the new monitor's work area.
If a window is maximized, it is sized to the work area. Like with fullscreen, a maximized window has a 'restore position', though unlike with fullscreen the restore position for maximized is stored by the system itself. Migration in cases where a maximized (or fullscreen) window's monitor changes is also taken care of by the system. To restore 'safely' to maximized (after changing window styles) a window must only `SetWindowPos(SWP_FRAMECHANGED)`. While technically a maximized window that becomes fullscreen 'is still maximized' (from Win32's perspective), its prudent to also `ShowWindow(SW_MAXIMIZED)` prior to `SWP_FRAMECHANGED` (to explicitly make the window maximized).
If not restoring to maximized, the restore position is adjusted by the new/ old work area. Additionally, the new/ old window DPI is used to adjust the size of the window by the DPI change (keeping the window's logical size the same).
- The work area origin is checked first (shifting window rect by the change in origin)
- The DPI is checked next, changing right/ bottom (size only)
- Each edge of the window is compared against the corresponding edge of the work area, nudging the window back on-screen if hanging offscreen. By shifting right before left, bottom before top, the top-left is guaranteed on-screen.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Tried it out. Seemed to work on my machine.
Jk, ran conhost/ terminal on mixed DPI system, max (or not), fullscreen, win+shift+left/ exit fullscreen/ maximize. Monitor unplug, etc.
The PlaySound functions were removed from OneCoreUAP_apiset.Lib in Windows 10 SDK 19041 because they did not actually belong there. Link to WinMM.Lib for PlaySoundW.
### Validation Steps Performed
* Built for x64 from repository root with: `MSBuild.exe -property:TargetPlatformVersion=10.0.19041.0`
* Installed CascadiaPackage_0.0.1.0_x64_Debug.msix and launched on 19042.867
This PR adds support for the VT line rendition attributes, which allow
for double-width and double-height line renditions. These renditions are
enabled with the `DECDWL` (double-width line) and `DECDHL`
(double-height line) escape sequences. Both reset to the default
rendition with the `DECSWL` (single-width line) escape sequence. For now
this functionality is only supported by the GDI renderer in conhost.
There are a lot of changes, so this is just a general overview of the
main areas affected.
Previously it was safe to assume that the screen had a fixed width, at
least for a given point in time. But now we need to deal with the
possibility of different lines have different widths, so all the
functions that are constrained by the right border (text wrapping,
cursor movement operations, and sequences like `EL` and `ICH`) now need
to lookup the width of the active line in order to behave correctly.
Similarly it used to be safe to assume that buffer and screen
coordinates were the same thing, but that is no longer true. Lots of
places now need to translate back and forth between coordinate systems
dependent on the line rendition. This includes clipboard handling, the
conhost color selection and search, accessibility location tracking and
screen reading, IME editor positioning, "snapping" the viewport, and of
course all the rendering calculations.
For the rendering itself, I've had to introduce a new
`PrepareLineTransform` method that the render engines can use to setup
the necessary transform matrix for a given line rendition. This is also
now used to handle the horizontal viewport offset, since that could no
longer be achieved just by changing the target coordinates (on a double
width line, the viewport offset may be halfway through a character).
I've also had to change the renderer's existing `InvalidateCursor`
method to take a `SMALL_RECT` rather than a `COORD`, to allow for the
cursor being a variable width. Technically this was already a problem,
because the cursor could occupy two screen cells when over a
double-width character, but now it can be anything between one and four
screen cells (e.g. a double-width character on the double-width line).
In terms of architectural changes, there is now a new `lineRendition`
field in the `ROW` class that keeps track of the line rendition for each
row, and several new methods in the `ROW` and `TextBuffer` classes for
manipulating that state. This includes a few helper methods for handling
the various issues discussed above, e.g. position clamping and
translating between coordinate systems.
## Validation Steps Performed
I've manually confirmed all the double-width and double-height tests in
_Vttest_ are now working as expected, and the _VT100 Torture Test_ now
renders correctly (at least the line rendition aspects). I've also got
my own test scripts that check many of the line rendition boundary cases
and have confirmed that those are now passing.
I've manually tested as many areas of the conhost UI that I could think
of, that might be affected by line rendition, including things like
searching, selection, copying, and color highlighting. For
accessibility, I've confirmed that the _Magnifier_ and _Narrator_
correctly handle double-width lines. And I've also tested the Japanese
IME, which while not perfect, is at least useable.
Closes#7865
## References
* See also #8617
## PR Checklist
* [x] Supports #3075
* [x] I work here.
* [x] Manual test.
## Detailed Description of the Pull Request / Additional comments
### Window Title Generation
Every time the renderer checks the title, it's doing two bad things that
I've fixed:
1. It's assembling the prefix to the full title doing a concatenation.
No one ever gets just the prefix ever after it is set besides the
concat. So instead of storing prefix and the title, I store the
assembled prefix + title and the bare title.
2. A copy must be made because it was returning `std::wstring` instead
of `std::wstring&`. Now it returns the ref.
### Dirty Area Return
Every time the renderer checks the dirty area, which is sometimes
multiple times per pass (regular text printing, again for selection,
etc.), a vector is created off the heap to return the rectangles. The
consumers only ever iterate this data. Now we return a span over a
rectangle or rectangles that the engine must store itself.
1. For some renderers, it's always a constant 1 element. They update
that 1 element when dirty is queried and return it in the span with a
span size of 1.
2. For other renderers with more complex behavior, they're already
holding a cached vector of rectangles. Now it's effectively giving
out the ref to those in the span for iteration.
### Bitmap Runs
The `til::bitmap` used a `std::optional<std::vector<til::rectangle>>`
inside itself to cache its runs and would clear the optional when the
runs became invalidated. Unfortunately doing `.reset()` to clear the
optional will destroy the underlying vector and have it release its
memory. We know it's about to get reallocated again, so we're just going
to make it a `std::pmr::vector` and give it a memory pool.
The alternative solution here was to use a `bool` and
`std::vector<til::rectangle>` and just flag when the vector was invalid,
but that was honestly more code changes and I love excuses to try out
PMR now.
Also, instead of returning the ref to the vector... I'm just returning a
span now. Everyone just iterates it anyway, may as well not share the
implementation detail.
### UTF-8 conversions
When testing with Terminal and looking at the `conhost.exe`'s PTY
renderer, it spends a TON of allocation time on converting all the
UTF-16 stuff inside to UTF-8 before it sends it out the PTY. This was
because `ConvertToA` was allocating a string inside itself and returning
it just to have it freed after printing and looping back around again...
as a PTY does.
The change here is to use `til::u16u8` that accepts a buffer out
parameter so the caller can just hold onto it.
## Validation Steps Performed
- [x] `big.txt` in conhost.exe (GDI renderer)
- [x] `big.txt` in Terminal (DX, PTY renderer)
- [x] Ensure WDDM and BGFX build under Razzle with this change.
Dustin L. Howett (3)
* Move CharToKeyEvents (and friends) into InteractivityBase (GH-9106)
* Update Cascadia Code to 2102.03 (GH-9088)
* verison: bump to 1.7 on main
Josh Soref (1)
* ci: update to Spell check to 0.0.17a (CC-9014)
Leonard Hecker (3)
* Fixed GH-5205: Ctrl+Alt+2 doesn't send ^[^@ (CC-5272)
* Fix issues in tests.xml and OpenConsole.psm1 (CC-9011)
* Fix GH-8458: Handle all Ctrl-key combinations (CC-8870)
Mike Griese (1)
* Add support for running a commandline in another WT window (GH-8898)
Michael Niksa (1)
* Teach the renderer to keep thread alive if engine requests it (GH-9091)
Lachlan Picking (1)
* Fix shader time input (CC-8994)
PankajBhojwani (1)
* Separate runtime TerminalSettings from profile-TerminalSettings (CC-8602)
Chester Liu (2)
* Add support for paste filtering and bracketed paste mode (CC-9034)
* Add support for chaining OSC 10-12 (CC-8999)
Related work items: MSFT-31692939
These functions have a dependency on the "VT Redirected" versions of
VkKeyScanW, MapVirtualKeyW and GetKeyState. Those implementations depend
on the service locator and therefore the entire interactivity stack.
This meant that anybody depending on just Types had to pull in **the
entire host** worth of dependencies (!).
Since these functions are only used in places where we have or are
testing interactivity, it makes sense to consolidate them here.
(cherry picked from commit 8f73145d9d)
These functions have a dependency on the "VT Redirected" versions of
VkKeyScanW, MapVirtualKeyW and GetKeyState. Those implementations depend
on the service locator and therefore the entire interactivity stack.
This meant that anybody depending on just Types had to pull in **the
entire host** worth of dependencies (!).
Since these functions are only used in places where we have or are
testing interactivity, it makes sense to consolidate them here.
### Plurals and paste tenses
In the past, plurals `foo`+`s` and past tenses `foo`+`ed` were
automatically tolerated. This turned out to be a bad design choice on my
part.
The basic example is that `potatos` would sometimes be treated as a
mistake and sometimes not (depending on the presence of `potato`).
You can see in this PR, that this logic resulted in `Applys` being
accepted as a word along with `AppContainered` -- there's nothing
intrinsically wrong w/ the latter, but unfortunately in order to screen
out the former, my shortcut just couldn't stick around. This means that
the `dictionary`/`expect` files will grow perhaps by a tiny bit, but as
you can see, not really by much.
This is also why `thereses` (a user) was accepted as a word in the past
(therese is in the base dictionary, so `therese` + `s` was acceptable).
### Pull requests
When GitHub initially introduced GitHub Actions, the event for
`pull_request` was created without enough permission for a tool like
this to work properly. I worked around that by using the `schedule`
event. In 2020, they introduced a replacement event
`pull_request_target` which has enough permission. This means that I can
stop relying on the `schedule` event.
### Miscellaneous
* I've folded together some `expect/` files since now is as good a time
as any.
* I've included a hint about `excludes.txt` (I added a similar one for
our primary repo recently, and it came up this week in
`microsoft/terminal` -- @zadjii-msft)
* I've standardized on a default of `.github/actions/spelling` to make
the out of the box experience easier for new adopters, so I'm applying
that change here -- if you're attached to the old directory name,
specifying it is still supported. -- note the directory rename may
cause a merge conflict for people with open PRs and changes to the
contents, this shouldn't be a big problem.
Initialize stack variables. check return code from shell lnk loading (GH-8712)
## References
* Commit f273aa679d4d9fb516678fc7ed5fc6495a8e8532 from os repository.
## Detailed Description of the Pull Request / Additional comments
* We found and fixed this in November 2017, but I fumbled the replication and accidentally overwrote the commit. This digs it up from history and puts it back in our code.
* When the shortcut file cannot be read for whatever reason, we are setting the hotkey value to uninitialized data as we never initialize several members on the stack in this function. It just so happens that the one in the `dwHotkey` field is commonly `0x4c` or the letter `L` which is then sent into the window procedure to tell the OS to capture it as a global hotkey and foreground the `conhost.exe` that was started. We should realize the load failure and not set any hotkey at all and we should initialize the stack variables. This does both.
## Validation Steps Performed
* [x] Manual scenario running LNK file that cannot be loaded from customer report (make LNK to cmd.exe, make it inaccessible to conhost so it can't load/read it.)
* [x] Manual scenario with `mklink`'d link that makes the shortcut parser attempt to load (and fail because it isn't a LNK at all from GH-7650)
Fixes MSFT-31351620
## References
* Commit f273aa679d4d9fb516678fc7ed5fc6495a8e8532 from os repository.
## PR Checklist
* [x] Closes#7650
* [x] I work here.
* [x] Tests passed
## Detailed Description of the Pull Request / Additional comments
* We found and fixed this in November 2017, but I fumbled the replication and accidentally overwrote the commit. This digs it up from history and puts it back in our code.
* When the shortcut file cannot be read for whatever reason, we are setting the hotkey value to uninitialized data as we never initialize several members on the stack in this function. It just so happens that the one in the `dwHotkey` field is commonly `0x4c` or the letter `L` which is then sent into the window procedure to tell the OS to capture it as a global hotkey and foreground the `conhost.exe` that was started. We should realize the load failure and not set any hotkey at all and we should initialize the stack variables. This does both.
## Validation Steps Performed
* [x] Manual scenario running LNK file that cannot be loaded from customer report (make LNK to cmd.exe, make it inaccessible to conhost so it can't load/read it.)
* [x] Manual scenario with `mklink`'d link that makes the shortcut parser attempt to load (and fail because it isn't a LNK at all from #7650)
N
* Change backslashes in include statements to forward slashes (CC-8205)
Dustin L. Howett
* Refactor DEC/ANSI modes to avoid duplication when we add SM/RM (GH-8469)
* Fix the xterm and SGR mouse encodings for CTRL, ALT, SHIFT (GH-8379)
James Holderness
* Fix rendering of DBCS characters when partially off screen (CC-8438)
* Correct horizontal coordinates in viewport overflow test (CC-8456)
* Correct paths in the the runut and runft test scripts (CC-8488)
* Retain horizontal viewport offset when moving to bottom (CC-8434)
* Fix out-of-bounds exceptions in Set...{Buffer,Screen}Size (CC-8309)
PankajBhojwani (1)
* Implement ConEmu's OSC 9;4 to set the taskbar progress indicator (CC-8055)
Chester Liu (1)
* Improve OSC 8 Hyperlink parsing logic (CC-7962)
Related work items: #31090459, #31090805, #31090808, #31090810
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 91dff13f6953441afe9e28bfe3da9d5bff27bb05
Related: MSFT-26196390
Related work items: #26196390
We had the xterm and SGR codings for meta/ctrl backwards. Oops.
This commit also fixes an observed issue in Windows Terminal where we
were passing in a console-style modifiers enum when MouseInput is
expecting MK_ constants.
I decided to unify MouseInput around the console-style modifier
constants because they have support for META (which MK_ does not) and
can differentiate between left/right alt/ctrl.
Our tests are fundamentally flawed here: they use a copy of the
modifier key generating logic _themselves_, so we got a bit of "error
carried forward."
I did not fix the tests to use known-good control sequences, I simply
replaced the character generator with another copy of the modifier code.
I did, however, extend them to test ctrl|meta and left/right modifiers.
Fixes#8291
Many include statements use forward slashes, while others use backwards
slashes. This is inconsistent formatting. For this reason, I changed the
backward slashes to forward slashes since that is the standard.
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/201117-2002 into official/rs_onecore_dep_uxp Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 526043ab6050397506b3cdb77e6a43b5ca9a2e9a
Related work items: MSFT-29990377
`ScrollIntoView` is responsible for scrolling the viewport to include
the UTR's start endpoint. The crash was caused by `start` being at the
exclusive end, and attempting to scroll to it. This is now fixed by
clamping the result to the bottom of the buffer.
Most of the work here is to allow a test for this. `ScrollIntoView`
relied on a virtual `ChangeViewport` function. By making that
non-virtual, the `DummyElementProvider` in the tests can now be a
`ScreenInfoUiaProviderBase`. This opens up the possibility of more
UiaTextRange tests in the future too.
Closes#7839
The `MovementAtExclusiveEnd` test was improperly authored for the
following reasons:
- it should have used `TEST_METHOD_PROPERTY` to cover all of the
TextUnits
- TextUnit::Document (arguably one of the most important) was ommitted
accidentally (`!= TextUnit_Document` was used instead of `<=`)
- The created range was not `EndExclusive`, but rather, the last cell in
the buffer (`EndInclusive`)
The first half of this PR fixes the test.
The second half of this PR expands the test and fixes any related issues
to make the test pass (i.e. #7771):
- `TEST_METHOD_PROPERTY` was added for it to be degenerate (start/end at
`EndExclusive`) or not (last cell of buffer)
- `utr->_start` is now also validated after moving backwards
NOTE: `utr->_start` was not validated when moving forwards because
moving forwards should always fail when at/past the last chell in the
buffer.
Closes#7771
This fixes a bug when moving backwards by word that resulted in #7742.
This also includes...
- a minor refactor that leverages `GetWordStart` in `MoveToPreviousWord`
- additional unit tests for movement by word
- a feature test comprised of the referenced bug report
`MoveToPreviousWord()` would...
- move backwards for each whitespace character
- then, move backwards for each regular character
This would actually result in moving to the beginning of the current "word" (as defined by a11y).
We actually need to do this process twice:
- the first time gets you to the beginning of the current word
- attempt to move back by one character
- the second time gets you to the beginning of the previous word
Rather than implementing 4 while loops, we leverage `GetWordStart()` to
attempt to move to the beginning of the previous word. We call it twice
(as described above). The logic is unchanged, but we instead reuse a
function that has already undergone more testing.
To make sure this works as expected, additional unit tests were
introduced covering "MoveByWord" in the TextBuffer.
## Validation Steps Performed
Added test for repro steps.
Added unit tests for movement by word.
Closes#7742
`EndExclusive` represents the end of the buffer. This is designed to not
point to any data on the buffer. UiaTextRange would point to this
`EndExclusive` and then attempt to move based on it. However, since it
does not point to any data, it could experience undefined behavior or
(inevitably) crash from running out of bounds.
This PR specifically checks for expansion and movement at that point,
and prevents us from moving beyond it. There are plans in the future to
define the "end" as the last character in the buffer. Until then, this
solution will suffice and provide correct behavior that doesn't crash.
## Validation Steps Performed
Performed the referenced bugs' repro steps and added test coverage.
Closes MSFT-20458595
Closes#7663Closes#7664
Conhost expands UIA text ranges when moved. This means that degenerate
ranges become non-degenerate after movement, leading to odd behaviour
from UIA clients. This PR doesn't expand degenerate ranges, but rather
keeps them degenerate by moving `_end` to the newly-changed `_start`.
Tested in the NVDA Python console (cases with `setEndPoint` and
`compareEndPoints` described in #7342). Also ran the logic by
@michaeldcurran.
Closes#7342
Almost definitely addresses nvaccess/nvda#11288 (although I'll need to
test with my Braille display). Also fixes an issue privately reported to
me by @simon818 with copy/paste from review cursor which originally lead
me to believe the issue was with `moveEndPointByRange`.
MouseInput was directly asking user32 about the state of the mouse buttons,
which was somewhat of a layering violation. This commit makes all callers
have to pass the mouse state in themselves.
Closes#4869
Carlos Zamora (1)
* UIA: use full buffer comparison in rects and endpoint setter (GH-6447)
Dan Thompson (2)
* Tweaks: normalize TextAttribute method names (adjective form) (GH-6951)
* Fix 'bcz exclusive' typo (GH-6938)
Dustin L. Howett (4)
* Fix VT mouse capture issues in Terminal and conhost (GH-7166)
* version: bump to 1.3 on master
* Update Cascadia Code to 2007.15 (GH-6958)
* Move to the TerminalDependencies NuGet feed (GH-6954)
James Holderness (3)
* Render the SGR "underlined" attribute in the style of the font (CC-7148)
* Add support for the "crossed-out" graphic rendition attribute (CC-7143)
* Refactor grid line renderers with support for more line types (CC-7107)
Leonard Hecker (1)
* Added til::spsc, a lock-free, single-producer/-consumer FIFO queue (CC-6751)
Michael Niksa (6)
* Update TAEF to 10.57.200731005-develop (GH-7164)
* Skip DX invalidation if we've already scrolled an entire screen worth of height (GH-6922)
* Commit attr runs less frequently by accumulating length of color run (GH-6919)
* Set memory order on slow atomics (GH-6920)
* Cache the viewport to make invalidation faster (GH-6918)
* Correct comment in this SPSC test as a quick follow up to merge.
Related work items: MSFT-28208358
This pull request fixes capture and event generation in VT mouse mode
for both conhost and terminal.
Fixes#6401.
[1/3] Terminal: clamp mouse events to the viewport, don't throw them away
gnome-terminal (at least) sends mouse events whose x/y are at the
extreme ends of the buffer when a drag starts inside the terminal and
then exits it.
We would previously discard any mouse events that exited the borders of
the viewport. Now we will keep emitting events where X/Y=0/w/h.
[2/3] conhost: clamp VT mouse to viewport, capture pointer
This is the same as (1), but for conhost. conhost wasn't already
capturing the pointer when VT mouse mode was in use. By capturing, we
ensure that events that happen outside the screen still result in events
sent to an application (like a release after a drag)
[3/3] wpf: capture the pointer when VT mouse is enabled
This is the same as (2), but for the WPF control. Clamping is handled
in TerminalCore in (1), so we didn't need to do it in WPF.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 29b1a1d663d0047dc0d2125dd05f75959bca27ef
Related work items: MSFT:27866336
We were using std::basic_string_view as a stand-in for std::span so that
we could change over all at once when C++20 dropped with full span
support. That day's not here yet, but as of 54a7fce3e we're using GSL 3,
whose span is C++20-compliant.
This commit replaces every instance of basic_string_view that was not
referring to an actual string with a span of the appropriate type.
I moved the `const` qualifier into span's `T` because while
`basic_string_view.at()` returns `const T&`, `span.at()` returns `T&`
(without the const). I wanted to maintain the invariant that members of
the span were immutable.
* Mechanical Changes
* `sv.at(x)` -> `gsl::at(sp, x)`
* `sv.c{begin,end}` -> `sp.{begin,end}` (span's iterators are const)
I had to replace a `std::basic_string<>` with a `std::vector<>` in
ConImeInfo, and I chose to replace a manual array walk in
ScreenInfoUiaProviderBase with a ranged-for. Please review those
specifically.
This will almost certainly cause a code size regression in Windows
because I'm blowing out all the PGO counts. Whoops.
Related: #3956, #975.
This is a refactoring of the renderer color calculations to simplify the
implementation, and to make it easier to support additional
color-altering rendition attributes in the future (e.g. _faint_ and
_conceal_).
## References
* This is a followup to PRs #3817 and #6809, which introduced additional
complexity in the color calculations, and which suggested the need for
refactoring.
## Detailed Description of the Pull Request / Additional comments
When we added support for `DECSCNM`, that required the foreground and
background color lookup methods to be able to return the opposite of
what was requested when the reversed mode was set. That made those
methods unnecessarily complicated, and I thought we could simplify them
considerably just by combining the calculations into a single method
that derived both colors at the same time.
And since both conhost and Windows Terminal needed to perform the same
calculations, it also made sense to move that functionality into the
`TextAttribute` class, where it could easily be shared.
In general this way of doing things is a bit more efficient. However, it
does result in some unnecessary work when only one of the colors is
required, as is the case for the gridline painter. So to make that less
of an issue, I've reordered the gridline code a bit so it at least
avoids looking up the colors when no gridlines are needed.
## Validation Steps Performed
Because of the API changes, quite a lot of the unit tests had to be
updated. For example instead of verifying colors with two separate calls
to `LookupForegroundColor` and `LookupBackgroundColor`, that's now
achieved with a single `LookupAttributeColors` call, comparing against a
pair of values. The specifics of the tests haven't changed though, and
they're all still working as expected.
I've also manually confirmed that the various color sequences and
rendition attributes are rendering correctly with the new refactoring.
By storing up the accumulated delta in the mouse input handler, we can
enlighten both conhost and terminal about wheel events that are less
than one line in size. Previously, we had a workaround in conhost that
clamped small scroll deltas to a whole line, which made trackpad
scrolling unimaginably fast. Terminal didn't make this mistake, but it
also didn't handle delta accumulation . . . which resulted in the same
behavior.
MouseInput will now wait until it's received WHEEL_DELTA (well-known
constant, value 120) worth of scrolling delta before it dispatches a
single scroll event.
Future considerations may include sending multiple wheel button events
for every *multiple* of WHEEL_DELTA, but that would be a slightly larger
refactoring that I'm not yet ready to undertake.
There's a chance that we should be dividing WHEEL_DELTA by the system's
"number of lines to scroll at once" setting, because on trackpads
conhost now scrolls a little _slow_. I think the only way to determine
whether this is palatable is to just ship it.
Fixes#6184.
