mirror of
https://github.com/microsoft/terminal.git
synced 2026-04-19 20:51:17 +00:00
3dca4263d5db491dd6da2ea863d65d83dedfb2a2
19 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
James Holderness
|
b2973eb573 |
Add support for the "concealed" graphic rendition attribute (#6907)
## Summary of the Pull Request This PR adds support for the `SGR 8` and `SGR 28` escape sequences, which enable and disable the _concealed/invisible_ graphic rendition attribute. When a character is output with this attribute set, it is rendered with the same foreground and background colors, so the text is essentially invisible. ## PR Checklist * [x] Closes #6876 * [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: #6876 ## Detailed Description of the Pull Request / Additional comments Most of the framework for this attribute was already implemented, so it was just a matter of updating the `TextAttribute::CalculateRgbColors` method to make the foreground the same as the background when the _Invisible_ flag was set. Note that this has to happen after the _Reverse Video_ attribute is applied, so if you have white-on-black text that is reversed and invisible, it should be all white, rather than all black. ## Validation Steps Performed There were already existing SGR unit tests covering this attribute in the `ScreenBufferTests`, and the `VtRendererTest`. But I've added to the `AdapterTest` which verifies the SGR sequences for setting and resetting the attribute, and I've extended the `TextAttributeTests` to verify that the color calculations return the correct values when the attribute is set. I've also manually confirmed that we now render the _concealed text_ values correctly in the _ISO 6429_ tests in Vttest. And I've manually tested the output of _concealed_ when combined with other attributes, and made sure that we're matching the behaviour of most other terminals. |
||
|
James Holderness
|
7d677c5511 |
Add support for the "faint" graphic rendition attribute (#6873)
## 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. |
||
|
James Holderness
|
3388a486dc |
Refactor the renderer color calculations (#6853)
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. |
||
Dustin L. Howett
|
1bf4c082b4 |
Reintroduce a color compatibility hack, but only for PowerShells (#6810)
There is going to be a very long tail of applications that will explicitly request VT SGR 40/37 when what they really want is to SetConsoleTextAttribute() with a black background/white foreground. Instead of making those applications look bad (and therefore making us look bad, because we're releasing this as an update to something that "looks good" already), we're introducing this compatibility quirk. Before the color reckoning in #6698 + #6506, *every* color was subject to being spontaneously and erroneously turned into the default color. Now, only the 16-color palette value that matches the active console background/foreground color will be destroyed, and only when received from specific applications. Removal will be tracked by #6807. Michael and I discussed what layer this quirk really belonged in. I originally believed it would be sufficient to detect a background color that matched the legacy default background, but @j4james provided an example of where that wouldn't work out (powershell setting the foreground color to white/gray). In addition, it was too heavyhanded: it re-broke black backgrounds for every application. Michael thought that it should live in the server, as a small VT parser that righted the wrongs coming directly out of the application. On further investigation, however, I realized that we'd need to push more information up into the server (so that it could make the decision about which VT was wrong and which was right) than should be strictly necessary. The host knows which colors are right and wrong, and it gets final say in what ends up in the buffer. Because of that, I chose to push the quirk state down through WriteConsole to DoWriteConsole and toggle state on the SCREEN_INFORMATION that indicates whether the colors coming out of the application are to be distrusted. This quirk _only applies to pwsh.exe and powershell.exe._ NOTE: This doesn't work for PowerShell the .NET Global tool, because it is run as an assembly through dotnet.exe. I have no opinion on how to fix this, or whether it is worth fixing. VALIDATION ---------- I configured my terminals to have an incredibly garish color scheme to show exactly what's going to happen as a result of this. The _default terminal background_ is purple or red, and the foreground green. I've printed out a heap of test colors to see how black interacts with them. Pull request #6810 contains the images generated from this test. The only color lines that change are the ones where black as a background or white as a foreground is selected out of the 16-color palette explicitly. Reverse video still works fine (because black is in the foreground!), and it's even possible to represent "black on default" and reverse it into "default on black", despite the black in question having been `40`. Fixes #6767. |
||
|
James Holderness
|
70a7ccc120 |
Add support for the "overline" graphic rendition attribute (#6754)
## 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. |
||
|
James Holderness
|
ddbe370d22 |
Improve the propagation of color attributes over ConPTY (#6506)
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 #2661 Closes #3076 Closes #3717 Closes #5384 Closes #5864 This is only a partial fix for #293, but I suspect the remaining cases are unfixable. |
||
|
James Holderness
|
f0df154ba9 |
Improve conpty rendering of default colors in legacy apps (#6698)
Essentially what this does is map the default legacy foreground and background attributes (typically white on black) to the `IsDefault` color type in the `TextColor` class. As a result, we can now initialize the buffer for "legacy" shells (like PowerShell and cmd.exe) with default colors, instead of white on black. This fixes the startup rendering in conpty clients, which expect an initial default background color. It also makes these colors update appropriately when the default palette values change. One complication in getting this to work, is that the console permits users to change which color indices are designated as defaults, so we can't assume they'll always be white on black. This means that the legacy-to-`TextAttribute` conversion will need access to those default values. Unfortunately the defaults are stored in the conhost `Settings` class (the `_wFillAttribute` field), which isn't easily accessible to all the code that needs to construct a `TextAttribute` from a legacy value. The `OutputCellIterator` is particularly problematic, because some iterator types need to generate a new `TextAttribute` on every iteration. So after trying a couple of different approaches, I decided that the least worst option would be to add a pair of static properties for the legacy defaults in the `TextAttribute` class itself, then refresh those values from the `Settings` class whenever the defaults changed (this only happens on startup, or when the conhost _Properties_ dialog is edited). And once the `TextAttribute` class had access to those defaults, it was fairly easy to adapt the constructor to handle the conversion of default values to the `IsDefault` color type. I could also then simplify the `TextAttribute::GetLegacyAttributes` method which does the reverse mapping, and which previously required the default values to be passed in as a parameter VALIDATION I had to make one small change to the `TestRoundtripExhaustive` unit test which assumed that all legacy attributes would convert to legacy color types, which is no longer the case, but otherwise all the existing tests passed as is. I added a new unit test verifying that the default legacy attributes correctly mapped to default color types, and the default color types were mapped back to the correct legacy attributes. I've manually confirmed that this fixed the issue raised in #5952, namely that the conhost screen is cleared with the correct default colors, and also that it is correctly refreshed when changing the palette from the properties dialog. And I've combined this PR with #6506, and confirmed that the PowerShell and the cmd shell renderings in Windows Terminal are at least improved, if not always perfect. This is a prerequisite for PR #6506 Closes #5952 |
||
|
James Holderness
|
ccea66710c |
Improve the legacy color conversions (#6358)
This PR provides a faster algorithm for converting 8-bit and 24-bit colors into the 4-bit legacy values that are required by the Win32 console APIs. It also fixes areas of the code that were incorrectly using a simple 16-color conversion that didn't handle 8-bit and 24-bit values. The faster conversion algorithm should be an improvement for issues #783 and #3950. One of the main points of this PR was to fix the `ReadConsoleOutputAttribute` API, which was using a simplified legacy color conversion (the original `TextAttribute:GetLegacyAttributes` method), which could only handle values from the 16-color table. RGB values, and colors from the 256-color table, would be mapped to completely nonsensical values. This API has now been updated to use the more correct `Settings::GenerateLegacyAttributes` method. But there were also a couple of other places in the code that were using `GetLegacyAttributes` when they really had no reason to be working with legacy attributes at all. This could result in colors being downgraded to 4-bit values (often badly, as explained above), when the code was already perfectly capable of displaying the full 24-bits. This included the fill colors in the IME composer (in `ConsoleImeInfo`), and the construction of the highlighting colors in the color search/selection handler (`Selection::_HandleColorSelection`). I also got rid of some legacy attribute code in the `Popup` class, which was originally intended to update colors below the popup when the settings changed, but actually caused more problems than it solved. The other major goal of this PR was to improve the performance of the `GenerateLegacyAttributes` method, since the existing implementation could be quite slow when dealing with RGB values. The simple cases are handled much the same as they were before. For an `IsDefault` color, we get the default index from the `Settings::_wFillAttribute` field. For an `IsIndex16` color, the index can just be returned as is. For an `IsRgb` color, the RGB components are compressed down to 8 bits (3 red, 3 green, 2 blue), simply by dropping the least significant bits. This 8-bit value is then used to lookup a representative 16-color value from a hard-coded table. An `IsIndex256` color is also converted with a lookup table, just using the existing 8-bit index. The RGB mapping table was calculated by taking each compressed 8-bit color, and picking a entry from the _Campbell_ palette that best approximated that color. This was done by looking at a range of 24-bit colors that mapped to the 8-bit value, finding the best _Campbell_ match for each of them (using a [CIEDE2000] color difference calculation), and then the most common match became the index that the 8-bit value would map to. The 256-color table was just a simpler version of this process. For each entry in the table, we take the default RGB palette value, and find it's closest match in the _Campbell_ palette. Because these tables are hard-coded, the results won't adjust to changes in the palette. However, they should still produce reasonable results for palettes that follow the standard ANSI color range. And since they're only a very loose approximation of the colors anyway, the exact value really isn't that important. That said, I have tried to make sure that if you take an RGB value for a particular index in a reasonable color scheme, then the legacy color mapped from that value should ideally match the same index. This will never be possible for all color schemes, but I have tweaked a few of the table entries to improve the results for some of the common schemes. One other point worth making regarding the hard-coded tables: even if we wanted to take the active palette into account, that wouldn't actually be possible over a conpty connection, because we can't easily know what color scheme the client application is using. At least this way the results in conhost are guaranteed to be the same as in the Windows Terminal. [CIEDE2000]: https://en.wikipedia.org/wiki/Color_difference#CIEDE2000 ## Validation Steps Performed This code still passes the `TextAttributeTests` that check the basic `GetLegacyAttribute` behaviour and verify the all legacy attributes roundtrip correctly. However, some of the values in the `RgbColorTests` had to be updated, since we're now intentionally returning different values as a result of the changes to the RGB conversion algorithm. I haven't added additional unit tests, but I have done a lot of manual testing to see how well the new algorithm works with a range of colors and a variety of different color schemes. It's not perfect in every situation, but I think it works well enough for the purpose it serves. I've also confirmed that the issues reported in #5940 and #6247 are now fixed by these changes. Closes #5940 Closes #6247 |
||
|
James Holderness
|
fa7c1abdf8 |
Fix SGR indexed colors to distinguish Indexed256 color (and more) (#5834)
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. |
||
|
James Holderness
|
e7a2732ffb |
Refactor the SGR implementation in AdaptDispatch (#5758)
This is an attempt to simplify the SGR (Select Graphic Rendition) implementation in conhost, to cut down on the number of methods required in the `ConGetSet` interface, and pave the way for future improvements and bug fixes. It already fixes one bug that prevented SGR 0 from being correctly applied when combined with meta attributes. * This a first step towards fixing the conpty narrowing bugs in issue #2661 * I'm hoping the simplification of `ConGetSet` will also help with #3849. * Some of the `TextAttribute` refactoring in this PR overlaps with similar work in PR #1978. ## Detailed Description of the Pull Request / Additional comments The main point of this PR was to simplify the `AdaptDispatch::SetGraphicsRendition` implementation. So instead of having it call a half a dozen methods in the `ConGetSet` API, depending on what kinds of attributes needed to be set, there is now just one call to get current attributes, and another call to set the new value. All adjustments to the attributes are made in the `AdaptDispatch` class, in a simple switch statement. To help with this refactoring, I also made some change to the `TextAttribute` class to make it easier to work with. This included adding a set of methods for setting (and getting) the individual attribute flags, instead of having the calling code being exposed to the internal attribute structures and messing with bit manipulation. I've tried to get rid of any methods that were directly setting legacy, meta, and extended attributes. Other than the fix to the `SGR 0` bug, the `AdaptDispatch` refactoring mostly follows the behaviour of the original code. In particular, it still maps the `SGR 38/48` indexed colors to RGB instead of retaining the index, which is what we ultimately need it to do. Fixing that will first require the color tables to be unified (issue #1223), which I'm hoping to address in a followup PR. But for now, mapping the indexed colors to RGB values required adding an an additional `ConGetSet` API to lookup the color table entries. In the future that won't be necessary, but the API will still be useful for other color reporting operations that we may want to support. I've made this API, and the existing setter, standardise on index values being in the "Xterm" order, since that'll be essential for unifying the code with the terminal adapter one day. I should also point out one minor change to the `SGR 38/48` behavior, which is that out-of-range RGB colors are now ignored rather than being clamped, since that matches the way Xterm works. ## Validation Steps Performed This refactoring has obviously required corresponding changes to the unit tests, but most were just minor updates to use the new `TextAttribute` methods without any real change in behavior. However, the adapter tests did require significant changes to accommodate the new `ConGetSet` API. The basic structure of the tests remain the same, but the simpler API has meant fewer values needed to be checked in each test case. I think they are all still covering the areas there were intended to, though, and they are all still passing. Other than getting the unit tests to work, I've also done a bunch of manual testing of my own. I've made sure the color tests in Vttest all still work as well as they used to. And I've confirmed that the test case from issue #5341 is now working correctly. Closes #5341 |
||
|
Dustin L. Howett (MSFT)
|
f919a46caf |
Optimize rendering runs of spaces when there is no visual change (#4877)
cmatrix is somewhat of a pathological case for our infrastructure: it prints out a bunch of green and white characters and then updates them a million times a second. It also maintains a column of space between every green character. When it prints this column, it prints it in "default" or "white". This ends up making runs of text that look like this: (def: G=green B=bright white W=white *=matrix char =space) G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W G W As characters trickle in: G*W G*W G*W G*W G*W G*W G*W B*W G*W G*W G*W G*W G*W G*W G*W G W G*W G*W G*W B*W G*W G*W G*W G W G*W B*W G*W G W G*W G*W G*W G*W G*W G W G*W G W G*W B*W G*W G*W B*W G W G*W G W G*W G W B*W G*W G W G W G*W G W G*W G W G W B*W G W G W B*W G W G*W G W G W G W Every one of those color transitions causes us to break up the run of text and start rendering it again. This impacts GDI, Direct2D *and* ConPTY. In the example above, there are 120 runs. The problem is, printing a space doesn't **use** the foreground color! This commit introduces an optimization. When we're about to break a text cluster because its attributes changed, we make sure that it's not just filled with spaces and doesn't differ in any visually-meaningful way (like underline or strikethrough, considering global invert state). This lets us optimize both the rendering _and_ the PTY output to look like this: G* * * * * * * B*G G* * * * * * * G* * * B*G * * * G* B*G * * * * * G* * * B*G * * B*G * * B*G * G * * B*G G B*G * Text will be printed at best line-by-line and at worst only when the visible properties of the screen actually change. In the example above, there are only 21 runs. This speeds up cmatrix remarkably. Refs #1064 |
||
|
James Holderness
|
381b11521a |
Correct fill attributes when scrolling and erasing (#3100)
## Summary of the Pull Request Operations that erase areas of the screen are typically meant to do so using the current color attributes, but with the rendition attributes reset (what we refer to as meta attributes). This also includes scroll operations that have to clear the area of the screen that has scrolled into view. The only exception is the _Erase Scrollback_ operation, which needs to reset the buffer with the default attributes. This PR updates all of these cases to apply the correct attributes when scrolling and erasing. ## PR Checklist * [x] Closes #2553 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed * [ ] Requires documentation to be updated * [ ] I've not really discussed this with core contributors. I'm ready to accept this work might be rejected in favor of a different grand plan. ## Detailed Description of the Pull Request / Additional comments My initial plan was to use a special case legacy attribute value to indicate the "standard erase attribute" which could safely be passed through the legacy APIs. But this wouldn't cover the cases that required default attributes to be used. And then with the changes in PR #2668 and #2987, it became clear that our requirements could be better achieved with a couple of new private APIs that wouldn't have to depend on legacy attribute hacks at all. To that end, I've added the `PrivateFillRegion` and `PrivateScrollRegion` APIs to the `ConGetSet` interface. These are just thin wrappers around the existing `SCREEN_INFORMATION::Write` method and the `ScrollRegion` function respectively, but with a simple boolean parameter to choose between filling with default attributes or the standard erase attributes (i.e the current colors but with meta attributes reset). With those new APIs in place, I could then update most scroll operations to use `PrivateScrollRegion`, and most erase operations to use `PrivateFillRegion`. The functions affected by scrolling included: * `DoSrvPrivateReverseLineFeed` (the RI command) * `DoSrvPrivateModifyLinesImpl` (the IL and DL commands) * `AdaptDispatch::_InsertDeleteHelper` (the ICH and DCH commands) * `AdaptDispatch::_ScrollMovement` (the SU and SD commands) The functions affected by erasing included: * `AdaptDispatch::_EraseSingleLineHelper` (the EL command, and most ED variants) * `AdaptDispatch::EraseCharacters` (the ECH command) While updating these erase methods, I noticed that both of them also required boundary fixes similar to those in PR #2505 (i.e. the horizontal extent of the erase operation should apply to the full width of the buffer, and not just the current viewport width), so I've addressed that at the same time. In addition to the changes above, there were also a few special cases, the first being the line feed handling, which required updating in a number of places to use the correct erase attributes: * `SCREEN_INFORMATION::InitializeCursorRowAttributes` - this is used to initialise the rows that pan into view when the viewport is moved down the buffer. * `TextBuffer::IncrementCircularBuffer` - this occurs when we scroll passed the very end of the buffer, and a recycled row now needs to be reinitialised. * `AdjustCursorPosition` - when within margin boundaries, this relies on a couple of direct calls to `ScrollRegion` which needed to be passed the correct fill attributes. The second special case was the full screen erase sequence (`ESC 2 J`), which is handled separately from the other ED sequences. This required updating the `SCREEN_INFORMATION::VtEraseAll` method to use the standard erase attributes, and also required changes to the horizontal extent of the filled area, since it should have been clearing the full buffer width (the same issue as the other erase operations mentioned above). Finally, there was the `AdaptDispatch::_EraseScrollback` method, which uses both scroll and fill operations, which could now be handled by the new `PrivateScrollRegion` and `PrivateFillRegion` APIs. But in this case we needed to fill with the default attributes rather than the standard erase attributes. And again this implementation needed some changes to make sure the full width of the active area was retained after the erase, similar to the horizontal boundary issues with the other erase operations. Once all these changes were made, there were a few areas of the code that could then be simplified quite a bit. The `FillConsoleOutputCharacterW`, `FillConsoleOutputAttribute`, and `ScrollConsoleScreenBufferW` were no longer needed in the `ConGetSet` interface, so all of that code could now be removed. The `_EraseSingleLineDistanceHelper` and `_EraseAreaHelper` methods in the `AdaptDispatch` class were also no longer required and could be removed. Then there were the hacks to handle legacy default colors in the `FillConsoleOutputAttributeImpl` and `ScrollConsoleScreenBufferWImpl` implementations. Since those hacks were only needed for VT operations, and the VT code no longer calls those methods, there was no longer a need to retain that behaviour (in fact there are probably some edge cases where that behaviour might have been considered a bug when reached via the public console APIs). ## Validation Steps Performed For most of the scrolling operations there were already existing tests in place, and those could easily be extended to check that the meta attributes were correctly reset when filling the revealed lines of the scrolling region. In the screen buffer tests, I made updates of that sort to the `ScrollOperations` method (handling SU, SD, IL, DL, and RI), the `InsertChars` and `DeleteChars` methods (ICH and DCH), and the `VtNewlinePastViewport` method (LF). I also added a new `VtNewlinePastEndOfBuffer` test to check the case where the line feed causes the viewport to pan past the end of the buffer. The erase operations, however, were being covered by adapter tests, and those aren't really suited for this kind of functionality (the same sort of issue came up in PR #2505). As a result I've had to reimplement those tests as screen buffer tests. Most of the erase operations are covered by the `EraseTests` method, except the for the scrollback erase which has a dedicated `EraseScrollbackTests` method. I've also had to replace the `HardReset` adapter test, but that was already mostly covered by the `HardResetBuffer` screen buffer test, which I've now extended slightly (it could do with some more checks, but I think that can wait for a future PR when we're fixing other RIS issues). |
||
Mike Griese
|
dec5c11e19 |
Add support for passing through extended text attributes, like… (#2917)
## 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 |
||
Michael Niksa
|
4f1157c044 | C26447,C26440 - is noexcept but can throw or doesn't throw but not noexcept | ||
|
Michael Niksa
|
8ea7401dc9 | C26472, no static_cast for arithmetic conversions. narrow or narrow_cast | ||
|
Michael Niksa
|
c63289b114 | C26493, no C-style casts. | ||
|
Michael Niksa
|
b33a59816e | C26496, mark const if it's never written after creation | ||
Hermès BÉLUSCA - MAÏTO
|
acabbe0459 |
Fix it's versus its typo. (#911)
|
||
|
Dustin Howett
|
d4d59fa339 |
Initial release of the Windows Terminal source code
This commit introduces all of the Windows Terminal and Console Host source, under the MIT license. |