## 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 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 PR adds support for the ANSI _doubly underlined_ graphic rendition
attribute, which is enabled by the `SGR 21` escape sequence.
There was already an `ExtendedAttributes::DoublyUnderlined` flag in the
`TextAttribute` class, but I needed to add `SetDoublyUnderlined` and
`IsDoublyUnderlined` methods to access that flag, and update the
`SetGraphicsRendition` methods of the two dispatchers to set the
attribute on receipt of the `SGR 21` sequence. I also had to update the
existing `SGR 24` handler to reset _DoublyUnderlined_ in addition to
_Underlined_, since they share the same reset sequence.
For the rendering, I've added a new grid line type, which essentially
just draws an additional line with the same thickness as the regular
underline, but slightly below it - I found a gap of around 0.05 "em"
between the lines looked best. If there isn't enough space in the cell
for that gap, the second line will be clamped to overlap the first, so
you then just get a thicker line. If there isn't even enough space below
for a thicker line, we move the offset _above_ the first line, but just
enough to make it thicker.
The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, the two underline attributes share
the same reset sequence, so to forward that state over conpty we require
a slightly more complicated process than with most other attributes
(similar to _Bold_ and _Faint_). We first check whether either underline
attribute needs to be turned off to send the reset sequence, and then
check individually if each of them needs to be turned back on again.
## Validation Steps Performed
For testing, I've extended the existing attribute tests in
`AdapterTest`, `VTRendererTest`, and `ScreenBufferTests`, to make sure
we're covering both the _Underlined_ and _DoublyUnderlined_ attributes.
I've also manually tested the `SGR 21` sequence in conhost and Windows
Terminal, with a variety of fonts and font sizes, to make sure the
rendering was reasonably distinguishable from a single underline.
Closes#2916
## Summary of the Pull Request
Text can have various attributes, such as "bold", "italic", "underlined", etc. The TextAttribute class embodies this. It has methods to set/query these attributes.
This change tweaks a few of the method names to make them match. I.e. for an imaginary text property "Foo", we should have methods along the lines of:
```
IsFoo
SetFoo(bool isFoo)
```
And variations should match: we should have "Foo" and "OverFoo", not "Fooey" and "OverFoo".
I chose to standardize on the adjective form, since that's what we are closest to already. The attributes I attacked here are:
SetItalic**s** --> SetItalic
SetUnderline --> SetUnderline**d**
SetOverline --> SetOverline**d**
("italic" is an adjective; "italics" is a plural noun, representing letters or words in an italic typeface)
And I also added methods for "DoublyUnderlined" for good measure.
I stopped short of renaming the GraphicsOptions enum values to match, too; but I'd be willing to do that in a follow-up change if people wanted it.
## Validation Steps Performed
It builds, and tests still pass.
## Summary of the Pull Request
This PR adds support for the `SGR 2` escape sequence, which enables the
ANSI _faint_ graphic rendition attribute. When a character is output
with this attribute set, it uses a dimmer version of the active
foreground color.
## PR Checklist
* [x] Closes#6703
* [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: #6703
## Detailed Description of the Pull Request / Additional comments
There was already an `ExtendedAttributes::Faint` flag in the
`TextAttribute` class, but I needed to add `SetFaint` and `IsFaint`
methods to access that flag, and update the `SetGraphicsRendition`
methods of the two dispatchers to set the attribute on receipt of the
`SGR 2` sequence. I also had to update the existing `SGR 22` handler to
reset _Faint_ in addition to _Bold_, since they share the same reset
sequence. For that reason, I thought it a good idea to change the name
of the `SGR 22` enum to `NotBoldOrFaint`.
For the purpose of rendering, I've updated the
`TextAttribute::CalculateRgbColors` method to return a dimmer version of
the foreground color when the _Faint_ attribute is set. This is simply
achieved by dividing each color component by two, which produces a
reasonable effect without being too complicated. Note that the _Faint_
effect is applied before _Reverse Video_, so if the output it reversed,
it's the background that will be faint.
The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, _Bold_ and _Faint_ share the same
reset sequence, so to forward that state over conpty we have to go
through a slightly more complicated process than with other attributes.
We first check whether either attribute needs to be turned off to send
the reset sequence, and then check if the individual attributes need to
be turned on again.
## Validation
I've extended the existing SGR unit tests to cover the new attribute in
the `AdapterTest`, the `ScreenBufferTests`, and the `VtRendererTest`,
and added a test to confirm the color calculations when _Faint_ is set
in the `TextAttributeTests`.
I've also done a bunch of manual testing with all the different VT color
types and confirmed that our output is comparable to most other
terminals.
## Summary of the Pull Request
This PR adds support for the `SGR 53` and `SGR 55` escapes sequences,
which enable and disable the ANSI _overline_ graphic rendition
attribute, the equivalent of the console character attribute
`COMMON_LVB_GRID_HORIZONTAL`. When a character is output with this
attribute set, a horizontal line is rendered at the top of the character
cell.
## PR Checklist
* [x] Closes#6000
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
To start with, I added `SetOverline` and `IsOverlined` methods to the
`TextAttribute` class, to set and get the legacy
`COMMON_LVB_GRID_HORIZONTAL` attribute. Technically there was already an
`IsTopHorizontalDisplayed` method, but I thought it more readable to add
a separate `IsOverlined` as an alias for that.
Then it was just a matter of adding calls to set and reset the attribute
in response to the `SGR 53` and `SGR 55` sequences in the
`SetGraphicsRendition` methods of the two dispatchers. The actual
rendering was already taken care of by the `PaintBufferGridLines` method
in the rendering engines.
The only other change required was to update the `_UpdateExtendedAttrs`
method in the `Xterm256Engine` of the VT renderer, to ensure the
attribute state would be forwarded to the Windows Terminal over conpty.
## Validation Steps Performed
I've extended the existing SGR unit tests to cover the new attribute in
the `AdapterTest`, the `OutputEngineTest`, and the `VtRendererTest`.
I've also manually tested the `SGR 53` and `SGR 55` sequences to confirm
that they do actually render (or remove) an overline on the characters
being output.
This PR reimplements the VT rendering engines to do a better job of
preserving the original color types when propagating attributes over
ConPTY. For the 16-color renderers it provides better support for
default colors and improves the efficiency of the color narrowing
conversions. It also fixes problems with the ordering of character
renditions that could result in attributes being dropped.
Originally the base renderer would calculate the RGB color values and
legacy/extended attributes up front, passing that data on to the active
engine's `UpdateDrawingBrushes` method. With this new implementation,
the renderer now just passes through the original `TextAttribute` along
with an `IRenderData` interface, and leaves it to the engines to extract
the information they need.
The GDI and DirectX engines now have to lookup the RGB colors themselves
(via simple `IRenderData` calls), but have no need for the other
attributes. The VT engines extract the information that they need from
the `TextAttribute`, instead of having to reverse engineer it from
`COLORREF`s.
The process for the 256-color Xterm engine starts with a check for
default colors. If both foreground and background are default, it
outputs a SGR 0 reset, and clears the `_lastTextAttribute` completely to
make sure any reset state is reapplied. With that out the way, the
foreground and background are updated (if changed) in one of 4 ways.
They can either be a default value (SGR 39 and 49), a 16-color index
(using ANSI or AIX sequences), a 256-color index, or a 24-bit RGB value
(both using SGR 38 and 48 sequences).
Then once the colors are accounted for, there is a separate step that
handles the character rendition attributes (bold, italics, underline,
etc.) This step must come _after_ the color sequences, in case a SGR
reset is required, which would otherwise have cleared any character
rendition attributes if it came last (which is what happened in the
original implementation).
The process for the 16-color engines is a little different. The target
client in this case (Windows telnet) is incapable of setting default
colors individually, so we need to output an SGR 0 reset if _either_
color has changed to default. With that out the way, we use the
`TextColor::GetLegacyIndex` method to obtain an approximate 16-color
index for each color, and apply the bold attribute by brightening the
foreground index (setting bit 8) if the color type permits that.
However, since Windows telnet only supports the 8 basic ANSI colors, the
best we can do for bright colors is to output an SGR 1 attribute to get
a bright foreground. There is nothing we can do about a bright
background, so after that we just have to drop the high bit from the
colors. If the resulting index values have changed from what they were
before, we then output ANSI 8-color SGR sequences to update them.
As with the 256-color engine, there is also a final step to handle the
character rendition attributes. But in this case, the only supported
attributes are underline and reversed video.
Since the VT engines no longer depend on the active color table and
default color values, there was quite a lot of code that could now be
removed. This included the `IDefaultColorProvider` interface and
implementations, the `Find(Nearest)TableIndex` functions, and also the
associated HLS conversion and difference calculations.
VALIDATION
Other than simple API parameter changes, the majority of updates
required in the unit tests were to correct assumptions about the way the
colors should be rendered, which were the source of the narrowing bugs
this PR was trying to fix. Like passing white on black to the
`UpdateDrawingBrushes` API, and expecting it to output the default `SGR
0` sequence, or passing an RGB color and expecting an indexed SGR
sequence.
In addition to that, I've added some VT renderer tests to make sure the
rendition attributes (bold, underline, etc) are correctly retained when
a default color update causes an `SGR 0` sequence to be generated (the
source of bug #3076). And I've extended the VT renderer color tests
(both 256-color and 16-color) to make sure we're covering all of the
different color types (default, RGB, and both forms of indexed colors).
I've also tried to manually verify that all of the test cases in the
linked bug reports (and their associated duplicates) are now fixed when
this PR is applied.
Closes#2661Closes#3076Closes#3717Closes#5384Closes#5864
This is only a partial fix for #293, but I suspect the remaining cases
are unfixable.
## Summary of the Pull Request
This is the plan that @miniksa suggested to me. Instead of trying to do lots of work in all the renderers to do backgrounds as one pass, and foregrounds as another, we can localize this change to basically just the DX renderer.
1. First, we give the DX engine a "heads up" on where the cursor is going to be drawn during the frame, in `PrepareRenderInfo`.
- This function is left unimplemented in the other render engines.
2. While printing runs of text, the DX renderer will try to paint the cursor in `CustomTextRenderer::DrawGlyphRun` INSTEAD of `DxEngine::PaintCursor`. This lets us weave the cursor background between the text background and the text.
## References
* #6151 was a spec in this general area. I should probably go back and update it, and we should probably approve that first.
* #6193 is also right up in this mess
## PR Checklist
* [x] Closes#1203
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
* This is essentially `"cursorTextColor": "textForeground"` from #6151.
* A follow up work item is needed to add support for the current behavior, (`"cursorTextColor": null`), and hooking up that setting to the renderer.
This PR introduces a new `ColorType` to allow us to distinguish between
`SGR` indexed colors from the 16 color table, the lower half of which
can be brightened, and the ISO/ITU indexed colors from the 256 color
table, which have a fixed brightness. Retaining the distinction between
these two types will enable us to forward the correct `SGR` sequences to
conpty when addressing issue #2661.
The other benefit of retaining the color index (which we didn't
previously do for ISO/ITU colors) is that it ensures that the colors are
updated correctly when the color scheme is changed.
## References
* This is another step towards fixing the conpty narrowing bugs in issue
#2661.
* This is technically a fix for issue #5384, but that won't be apparent
until #2661 is complete.
## PR Checklist
* [x] Closes#1223
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
The first part of this PR was the introduction of a new `ColorType` in
the `TextColor` class. Instead of just the one `IsIndex` type, there is
now an `IsIndex16` and an `IsIndex256`. `IsIndex16` covers the eight
original ANSI colors set with `SGR 3x` and `SGR 4x`, as well as the
brighter aixterm variants set with `SGR 9x` and `SGR 10x`. `IsIndex256`
covers the 256 ISO/ITU indexed colors set with `SGR 38;5` and `SGR
48;5`.
There are two reasons for this distinction. The first is that the ANSI
colors have the potential to be brightened by the `SGR 1` bold
attribute, while the ISO/ITO color do not. The second reason is that
when forwarding an attributes through conpty, we want to try and
preserve the original SGR sequence that generated each color (to the
extent that that is possible). By having the two separate types, we can
map the `IsIndex16` colors back to ANSI/aixterm values, and `IsIndex256`
to the ISO/ITU sequences.
In addition to the VT colors, we also have to deal with the legacy
colors set by the Windows console APIs, but we don't really need a
separate type for those. It seemed most appropriate to me to store them
as `IsIndex256` colors, since it doesn't make sense to have them
brightened by the `SGR 1` attribute (which is what would happen if they
were stored as `IsIndex16`). If a console app wanted a bright color it
would have selected one, so we shouldn't be messing with that choice.
The second part of the PR was the unification of the two color tables.
Originally we had a 16 color table for the legacy colors, and a separate
table for the 256 ISO/ITU colors. These have now been merged into one,
so color table lookups no longer need to decide which of the two tables
they should be referencing. I've also updated all the methods that took
a color table as a parameter to use a `basic_string_view` instead of
separate pointer and length variables, which I think makes them a lot
easier and safer to work with.
With this new architecture in place, I could now update the
`AdaptDispatch` SGR implementation to store the ISO/ITU indexed colors
as `IsIndex256` values, where before they were mapped to RGB values
(which prevented them reflecting any color scheme changes). I could also
update the `TerminalDispatch` implementation to differentiate between
the two index types, so that the `SGR 1` brightening would only be
applied to the ANSI colors.
I've also done a bit of code refactoring to try and minimise any direct
access to the color tables, getting rid of a lot of places that were
copying tables with `memmove` operations. I'm hoping this will make it
easier for us to update the code in the future if we want to reorder the
table entries (which is likely a requirement for unifying the
`AdaptDispatch` and `TerminalDispatch` implementations).
## Validation Steps Performed
For testing, I've just updated the existing unit tests to account for
the API changes. The `TextColorTests` required an extra parameter
specifying the index type when setting an index. And the `AdapterTest`
and `ScreenBufferTests` required the use of the new `SetIndexedXXX`
methods in order to be explicit about the index type, instead of relying
on the `TextAttribute` constructor and the old `SetForeground` and
`SetBackground` methods which didn't have a way to differentiate index
types.
I've manually tested the various console APIs
(`SetConsoleTextAttribute`, `ReadConsoleOutputAttribute`, and
`ReadConsoleOutput`), to make sure they are still setting and reading
the attributes as well as they used to. And I've tested the
`SetConsoleScreenBufferInfoEx` and `GetConsoleScreenBufferInfoEx` APIs
to make sure they can read and write the color table correctly. I've
also tested the color table in the properties dialog, made sure it was
saved and restored from the registry correctly, and similarly saved and
restored from a shortcut link.
Note that there are still a bunch of issues with the color table APIs,
but no new problems have been introduced by the changes in this PR, as
far as I could tell.
I've also done a bunch of manual tests of `OSC 4` to make sure it's
updating all the colors correctly (at least in conhost), and confirmed
that the test case in issue #1223 now works as expected.
## Summary of the Pull Request
Moves the ConPTY drawing mechanism (`VtRenderer`) to use the fine-grained `til::bitmap` individual-dirty-bit tracking mechanism instead of coarse-grained rectangle unions to improve drawing performance by dramatically reducing the total area redrawn.
## PR Checklist
* [x] Part of #778 and #1064
* [x] I work here
* [x] Tests added and updated.
* [x] I'm a core contributor
## Detailed Description of the Pull Request / Additional comments
- Converted `GetDirtyArea()` interface from `IRenderEngine` to use a vector of `til::rectangle` instead of the `SMALL_RECT` to banhammer inclusive rectangles.
- `VtEngine` now holds and operates on the `til::bitmap` for invalidation regions. All invalidation operation functions that used to be embedded inside `VtEngine` are deleted in favor of using the ones in `til::bitmap`.
- Updated `VtEngine` tracing to use new `til::bitmap` on trace and the new `to_string()` methods detailed below.
- Comparison operators for `til::bitmap` and complementary tests.
- Fixed an issue where the dirty rectangle shortcut in `til::bitmap` was set to 0,0,0,0 by default which means that `|=` on it with each `set()` operation was stretching the rectangle from 0,0. Now it's a `std::optional` so it has no value after just being cleared and will build from whatever the first invalidated rectangle is. Complementary tests added.
- Optional run caching for `til::bitmap` in the `runs()` method since both VT and DX renderers will likely want to generate the set of runs at the beginning of a frame and refer to them over and over through that frame. Saves the iteration and creation and caches inside `til::bitmap` where the chance of invalidation of the underlying data is known best. It is still possible to iterate manually with `begin()` and `end()` from the outside without caching, if desired. Complementary tests added.
- WEX templates added for `til::bitmap` and used in tests.
- `translate()` method for `til::bitmap` which will slide the dirty points in the direction specified by a `til::point` and optionally back-fill the uncovered area as dirty. Complementary tests added.
- Moves all string generation for `til` types `size`, `point`, `rectangle`, and `some` into a `to_string` method on each object such that it can be used in both ETW tracing scenarios AND in the TAEF templates uniformly. Adds a similar method for `bitmap`.
- Add tagging to `_bitmap_const_iterator` such that it appears as a valid **Input Iterator** to STL collections and can be used in a `std::vector` constructor as a range. Adds and cleans up operators on this iterator to match the theoretical requirements for an **Input Iterator**. Complementary tests added.
- Add loose operators to `til` which will allow some basic math operations (+, -, *, /) between `til::size` and `til::point` and vice versa. Complementary tests added. Complementary tests added.
- Adds operators to `til::rectangle` to allow scaling with basic math operations (+, -, *) versus `til::size` and translation with basic math operations (+, -) against `til::point`. Complementary tests added.
- In-place variants of some operations added to assorted `til` objects. Complementary tests added.
- Update VT tests to compare invalidation against the new map structure instead of raw rectangles where possible.
## Validation Steps Performed
- Wrote additional til Unit Tests for all additional operators and functions added to the project to support this operation
- Updated the existing VT renderer tests
- Ran perf check
This PR adds support for "Resize with Reflow" to the Terminal. In
conhost, `ResizeWithReflow` is the function that's responsible for
reflowing wrapped lines of text as the buffer gets resized. Now that
#4415 has merged, we can also implement this in the Terminal. Now, when
the Terminal is resized, it will reflow the lines of it's buffer in the
same way that conhost does. This means, the terminal will no longer chop
off the ends of lines as the buffer is too small to represent them.
As a happy side effect of this PR, it also fixed#3490. This was a bug
that plagued me during the investigation into this functionality. The
original #3490 PR, #4354, tried to fix this bug with some heavy conpty
changes. Turns out, that only made things worse, and far more
complicated. When I really got to thinking about it, I realized "conhost
can handle this right, why can't the Terminal?". Turns out, by adding
resize with reflow, I was also able to fix this at the same time.
Conhost does a little bit of math after reflowing to attempt to keep the
viewport in the same relative place after a reflow. By re-using that
logic in the Terminal, I was able to fix#3490.
I also included that big ole test from #3490, because everyone likes
adding 60 test cases in a PR.
## References
* #4200 - this scenario
* #405/#4415 - conpty emits wrapped lines, which was needed for this PR
* #4403 - delayed EOL wrapping via conpty, which was also needed for
this
* #4354 - we don't speak of this PR anymore
## PR Checklist
* [x] Closes#1465
* [x] Closes#3490
* [x] Closes#4771
* [x] Tests added/passed
## EDIT: Changes to this PR on 5 March 2020
I learned more since my original version of this PR. I wrote that in
January, and despite my notes that say it was totally working, it
_really_ wasn't.
Part of the hard problem, as mentioned in #3490, is that the Terminal
might request a resize to (W, H-1), and while conpty is preparing that
frame, or before the terminal has received that frame, the Terminal
resizes to (W, H-2). Now, there aren't enough lines in the terminal
buffer to catch all the lines that conpty is about to emit. When that
happens, lines get duplicated in the buffer. From a UX perspective, this
certainly looks a lot worse than a couple lost lines. It looks like
utter chaos.
So I've introduced a new mode to conpty to try and counteract this
behavior. This behavior I'm calling "quirky resize". The **TL;DR** of
quirky resize mode is that conpty won't emit the entire buffer on a
resize, and will trust that the terminal is prepared to reflow it's
buffer on it's own.
This will enable the quirky resize behavior for applications that are
prepared for it. The "quirky resize" is "don't `InvalidateAll` when the
terminal resizes". This is added as a quirk as to not regress other
terminal applications that aren't prepared for this behavior
(gnome-terminal, conhost in particular). For those kinds of terminals,
when the buffer is resized, it's just going to lose lines. That's what
currently happens for them.
When the quirk is enabled, conpty won't repaint the entire buffer. This
gets around the "duplicated lines" issue that requesting multiple
resizes in a row can cause. However, for these terminals that are
unprepared, the conpty cursor might end up in the wrong position after a
quirky resize.
The case in point is maximizing the terminal. For maximizing
(height->50) from a buffer that's 30 lines tall, with the cursor on
y=30, this is what happens:
* With the quirk disabled, conpty reprints the entire buffer. This is
60 lines that get printed. This ends up blowing away about 20 lines
of scrollback history, as the terminal app would have tried to keep
the text pinned to the bottom of the window. The term. app moved the
viewport up 20 lines, and then the 50 lines of conpty output (30
lines of text, and 20 blank lines at the bottom) overwrote the lines
from the scrollback. This is bad, but not immediately obvious, and
is **what currently happens**.
* With the quirk enabled, conpty doesn't emit any lines, but the
actual content of the window is still only in the top 30 lines.
However, the terminal app has still moved 20 lines down from the
scrollback back into the viewport. So the terminal's cursor is at
y=50 now, but conpty's is at 30. This means that the terminal and
conpty are out of sync, and there's not a good way of re-syncing
these. It's very possible (trivial in `powershell`) that the new
output will jump up to y=30 override the existing output in the
terminal buffer.
The Windows Terminal is already prepared for this quirky behavior, so it
doesn't keep the output at the bottom of the window. It shifts it's
viewport down to match what conpty things the buffer looks like.
What happens when we have passthrough mode and WT is like "I would like
quirky resize"? I guess things will just work fine, cause there won't be
a buffer behind the passthrough app that the terminal cares about. Sure,
in the passthrough case the Terminal could _not_ quirky resize, but the
quirky resize won't be wrong.
## Summary of the Pull Request
Allows VT engine methods that print formatted strings (cursor movements, color changes, etc.) to provide a guess at the max buffer size required eliminating the double-call for formatting in the common case.
## PR Checklist
* [x] Found while working on #778
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
- The most common case for VT rendering is formatting a few numbers into a sequence. For the most part, we can already tell the maximum length that the string could be based on the number of substitutions and the size of the parameters.
- The existing formatting method would always double-call. It would first call for how long the string was going to be post-formatting, allocate that memory, then call again and fill it up. This cost two full times of running through the string to find a length we probably already knew for the most part.
- Now if a size is provided, we allocate that first and attempt the "second pass" of formatting directly into the buffer. This saves the count step in the common case.
- If this fails, we fall back and do the two-pass method (which theoretically means the bad case is now 3 passes.)
- The next biggest waste of time in this method was allocating and freeing strings for every format pass. Due to the nature of the VT renderer, many things need to be formatted this way. I've now instead moved the format method to hold a static string that really only grows over the course of the session for all of these format operations. I expect a majority of the time, it will only be consuming approximately 5-15 length of a std::string of memory space. I cannot currently see a circumstance where it would use more than that, but I'm consciously trading memory usage when running as a PTY for overall runtime performance here.
## Validation Steps Performed
- Ran the thing manually and checked it out with wsl and cmatrix and Powershell and such attached to Terminal
- Wrote and ran automated tests on formatting method
## Summary of the Pull Request
Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines.
## References
* Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that
* Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo
* #4200 is the mega bucket with all this work
* MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out.
* #4403 - I made sure this worked with that PR before I even sent #4403
## PR Checklist
* [x] Closes#405
* [x] Closes#3367
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I started with the following implementation:
When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text.
However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist.
## Validation Steps Performed
* Ran tests
* Checked how the Windows Terminal behaves with these changes
* Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty
Generated by https://github.com/jsoref/spelling `f`; to maintain your repo, please consider `fchurn`
I generally try to ignore upstream bits. I've accidentally included some items from the `deps/` directory. I expect someone will give me a list of items to drop, I'm happy to drop whole files/directories, or to split the PR into multiple items (E.g. comments/locals/public).
Closes#4294
## Summary of the Pull Request
Adds support for Italics, Blinking, Invisible, CrossedOut text, THROUGH CONPTY. This does **NOT** add support for those styles to conhost or the terminal.
We will store these "Extended Text Attributes" in a `TextAttribute`. When we go to render a line, we'll see if the state has changed from our previous state, and if so, we'll appropriately toggle that state with VT. Boldness has been moved from a `bool` to a single bit in these flags.
Technically, now that these are stored in the buffer, we only need to make changes to the renderers to be able to support them. That's not being done as a part of this PR however.
## References
See also #2915 and #2916, which are some follow-up tasks from this fix. I thought them too risky for 20H1.
## PR Checklist
* [x] Closes#2554
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
<hr>
* store text with extended attributes too
* Plumb attributes through all the renderers
* parse extended attrs, though we're not renderering them right
* Render these states correctly
* Add a very extensive test
* Cleanup for PR
* a block of PR feedback
* add 512 test cases
* Fix the build
* Fix @carlos-zamora's suggestions
* @miniksa's PR feedback
Adjusts the startup and shutdown behavior of most threads in the console host to alleviate race conditions that are either exacerbated or introduced by the VT PTY threads.
James Holderness
Chester Liu
Dan Thompson
Dustin L. Howett
Mike Griese
Michael Niksa
Josh Soref
adiviness
