Terminal output not speed enough #1064
Assignees
Labels
Area-Performance
Performance-related issue
Issue-Task
It's a feature request, but it doesn't really need a major design.
Needs-Tag-Fix
Doesn't match tag requirements
Priority-0
Bugs that we consider release-blocking/recall-class (P0)
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
ghost
added
Needs-Triage
|
FWIW, terminal output is smoother on Windows Terminal, while Fluent Terminal is noticeably "choppier". If that is the primary source of perf difference, choosing smoothness over raw perf is a totally viable choice. Again, just sharing the info. |
|
Well, the goal certainly is to be faster. We haven't necessarily optimized all that much yet - we've been focusing mostly on getting the foundations up. This is definitely a path we should profile, and make sure to get every ounce of perf out of. @miniksa because he's been doing a ton of this recently. I wouldn't be surprised if part of this is due to conpty getting in the way. I don't know if Fluent Terminal uses winpty as it's pty API, or if it's just using a pair of pipes directly to wsl. If it's using direct pipes, then it's definitely going to have a speed edge till we can implement a passthrough mode for conpty. |
zadjii-msft
added
Product-Terminal
|
I think a decent part of this is due to windows terminal performing a full repaint of the visible buffer for every change. It doesn't do differential rendering yet (only painting the parts/regions of the screen that changed) edit: there's an issue tracking this here - #778 |
|
Yes, I very much want to improve this. I am personally very passionate about the performance of this and want to strive to beat competitors in this area. I am glad you filed this issue so I can use it as a test case when I get time to focus on the raw performance and make this improve. But please be patient. |
DHowett-MSFT
removed
the
Needs-Triage
|
@miniksa I am patient, no need to rush :) As I said, there are more important issues before this, although I too am very passionate about performance. (I'm a GPGPU programmer, what else would I be professionally passionate about?) ;) |
|
To throw another torture-test onto the pile, old Conhost noticeably outperforms the new terminal when running a tight loop: for($r = 0; $r -le 255; $r++) {
for($g = 0; $g -le 255; $g++) {
for($b = 0; $b -le 255; $b++) {
Write-Host "`e[38;2;$r;$g;$($b)m█`e[0m" -NoNewline
}
}
}(Was creating a short demo to show off 24-bit color support, noticed choppiness) Video evidence (gfycat link, because nobody wants to load a 9MB gif: https://gfycat.com/thinlividclumber) Details: OS: Windows 10 18362.145
|
This was referenced Aug 12, 2019
|
I accidentally found this. While I was outputing large record to stdout, I found the build-it terminal in VS code is really fast. this is a simple test code" #include <iostream>
using namespace std;
int main()
{
for(int i = 0 ; i < 10000 ; i++) {
cout << "NUMBER " << i <<std::endl;
cout << "TEST STRING" <<std::endl;
}
}The legacy cmd use ~2.5 sec The VS code terminal ~5.8 sec (really smooth) That is really interesting! |
|
terminal is noticeably sluggish when dealing with large amounts of output, so I did some comparisons. Ubuntu WSL2 terminal v. 06.2951.0 on Surface Laptop 2 now actually draw output for comparison, conemu does worse: but the old terminal (conhost?) does much better: vscode with remote extension shell into WSL does much much better! But gnome-terminal via VcXsrv leads the pack: Not really fair to compare it to my Linux deskstop but... Anyway, I guess the point is, we need about an order of magnitude improvement! |
|
How about comparisons with conhost.exe? I timed the time needed to simply type a text file in Windows Terminal (0.6.2951.0) and Windows Console. The Console is consistently 10 times faster or more. |
cinnamon-msft
added
Priority-0
Closed
|
I also have done some measurements using various terminals and environments for comparison reasons. The test used was to dump (cat for bash/get-content for powershell/type for CMD.exe) a big text file that is available here: https://norvig.com/big.txt. I hope these would be helpful and that Windows Terminal will soon get to the same level of performance as the best terminal performers do. Here are the results: cmd.exe on Windows 10 v1909: 3.66 sec CMD.EXE under Windows Terminal (Preview) Version: 0.9.433.0: 6.70 sec Cygwin under Windows Terminal (Preview) Version: 0.9.433.0: 14.594 sec Powershell Core 6.2.0 under Windows Terminal (Preview) Version: 0.9.433.0: 18.458 sec Git-Bash under MinTTY3.1.0: 1.058 sec Putty 0.71 connected to an Ubuntu OS: 2.763 sec konsole with cygwin under XWing on Windows 10 v1909:3.198 sec urxvt with cygwin under XWing on Windows 10 v1909: 3.144 sec xterm under XWing on Windows 10 v1909: 3.311sec Arch Linux xterm:3.822 sec Arch Linux Konsole: 0.616 sec |
5 tasks
|
@miniksa Keep up the good work! :) if you need any help or testing just let us know. |
|
@NeKJ, I believe that the testing results show that Terminal is "speed enough" for 1.0 release with the differential rendering that #5345 is about to bring in under a slightly punishing environment (200% DPI on a massive display, mobile-type processor with the i7 HQ). So I'm going to mark this one as "complete" with #5345 and we can create future follow on post-1.0 items for specific performance issues beyond this point. |
9 tasks
|
#3075 looks like a good place to focus on other massive-output work Post 1.0. |
ghost
added
Needs-Tag-Fix
miniksa
added a commit
that referenced
this issue
Apr 22, 2020
## Summary of the Pull Request - Adjusts scaling practices in `DxEngine` (and related scaling practices in `TerminalControl`) for pixel-perfect row baselines and spacing at High DPI such that differential row-by-row rendering can be applied at High DPI. ## References - #5185 ## PR Checklist * [x] Closes #5320, closes #3515, closes #1064 * [x] I work here. * [x] Manually tested. * [x] No doc. * [x] Am core contributor. Also discussed with some of them already via Teams. ## Detailed Description of the Pull Request / Additional comments **WAS:** - We were using implicit DPI scaling on the `ID2D1RenderTarget` and running all of our processing in DIPs (Device-Independent Pixels). That's all well and good for getting things bootstrapped quickly, but it leaves the actual scaling of the draw commands up to the discretion of the rendering target. - When we don't get to explicitly choose exactly how many pixels tall/wide and our X/Y placement perfectly, the nature of floating point multiplication and division required to do the presentation can cause us to drift off slightly out of our control depending on what the final display resolution actually is. - Differential drawing cannot work unless we can know the exact integer pixels that need to be copied/moved/preserved/replaced between frames to give to the `IDXGISwapChain1::Present1` method. If things spill into fractional pixels or the sizes of rows/columns vary as they are rounded up and down implicitly, then we cannot do the differential rendering. **NOW:** - When deciding on a font, the `DxEngine` will take the scale factor into account and adjust the proposed height of the requested font. Then the remainder of the existing code that adjusts the baseline and integer-ifies each character cell will run naturally from there. That code already works correctly to align the height at normal DPI and scale out the font heights and advances to take an exact integer of pixels. - `TermControl` has to use the scale now, in some places, and stop scaling in other places. This has to do with how the target's nature used to be implicit and is now explicit. For instance, determining where the cursor click hits must be scaled now. And determining the pixel size of the display canvas must no longer be scaled. - `DxEngine` will no longer attempt to scale the invalid regions per my attempts in #5185 because the cell size is scaled. So it should work the same as at 96 DPI. - The block is removed from the `DxEngine` that was causing a full invalidate on every frame at High DPI. - A TODO was removed from `TermControl` that was invalidating everything when the DPI changed because the underlying renderer will already do that. ## Validation Steps Performed * [x] Check at 150% DPI. Print text, scroll text down and up, do selection. * [x] Check at 100% DPI. Print text, scroll text down and up, do selection. * [x] Span two different DPI monitors and drag between them. * [x] Giant pile of tests in #5345 (comment) Co-authored-by: Dustin Howett <duhowett@microsoft.com> Co-authored-by: Mike Griese <migrie@microsoft.com>
|
🎉This issue was addressed in #5345, which has now been successfully released as Handy links: |
|
I have a console program that outputs a lot of tracing text. I did some performance tests. Windows Terminal v1.0.1401.0: 1600s |
|
I tested Fluent Terminal as well, at least Windows Terminal is faster than that. Fluent Terminal 0.7.1.0: 2930s |
|
The Windows Terminal I'm running is fast (Stable release): Although, while it's writing STDOUT really fast, there is a long delay to interrupt the foreground process, almost as if the terminal is not able to output the text rapidly and send the interruption sequence to the underlying shell. I would imagine that it makes more sense to prioritize the user's input if there's a bottleneck somewhere. |
Closed
8 tasks
ghost
pushed a commit
that referenced
this issue
Jun 10, 2020
… VT output graphics options (#6420) ## Summary of the Pull Request Caches vectors in the class and uses a new helper to opportunistically shrink/grow as viewport sizes change in order to save performance on alloc/free of commonly used vectors. ## PR Checklist * [x] Scratches a perf itch. * [x] I work here. * [x] wil tests added * [x] No add'l doc. * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two fixes: 1. For outputting lots of text, the base renderer class spent a lot of time allocating and freeing and reallocating the `Cluster` vector that adapts the text buffer information into render clusters. I've now cached this vector in the base render class itself and I shrink/grow it based on the viewport update that happens at the top of every frame. To prevent too much thrashing in the downward/shrink direction, I wrote the `til::manage_vector` helper that contains a threshold to only shrink if it asks for small enough of a size relative to the existing one. I used 80% of the existing size as the threshold for this one. 2. For outputting lots of changing colors, the VT graphics output engine spent a bunch of time allocating and reallocating the vector for `GraphicsOptions`. This one doesn't really have a predictable size, but I never expect it to get extremely big. So I just held it in the base class. ## Validation Steps Performed * [x] Ran the til unit test * [x] Checked render cluster vector time before/after against `big.txt` from #1064 * [x] Checked VT graphics output vector time before/after against `cacafire` Case | Before | After ---|---|---| `big.txt` |  |  `cacafire` |  | 
There's #6258 reporting the interruption issue. IMO responsiveness much more important than throughput on this particular issue. |
DHowett
pushed a commit
that referenced
this issue
Jun 24, 2020
… VT output graphics options (#6420) ## Summary of the Pull Request Caches vectors in the class and uses a new helper to opportunistically shrink/grow as viewport sizes change in order to save performance on alloc/free of commonly used vectors. ## PR Checklist * [x] Scratches a perf itch. * [x] I work here. * [x] wil tests added * [x] No add'l doc. * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two fixes: 1. For outputting lots of text, the base renderer class spent a lot of time allocating and freeing and reallocating the `Cluster` vector that adapts the text buffer information into render clusters. I've now cached this vector in the base render class itself and I shrink/grow it based on the viewport update that happens at the top of every frame. To prevent too much thrashing in the downward/shrink direction, I wrote the `til::manage_vector` helper that contains a threshold to only shrink if it asks for small enough of a size relative to the existing one. I used 80% of the existing size as the threshold for this one. 2. For outputting lots of changing colors, the VT graphics output engine spent a bunch of time allocating and reallocating the vector for `GraphicsOptions`. This one doesn't really have a predictable size, but I never expect it to get extremely big. So I just held it in the base class. ## Validation Steps Performed * [x] Ran the til unit test * [x] Checked render cluster vector time before/after against `big.txt` from #1064 * [x] Checked VT graphics output vector time before/after against `cacafire` Case | Before | After ---|---|---| `big.txt` |  |  `cacafire` |  |  (cherry picked from commit 0c93b2e)
DHowett
pushed a commit
that referenced
this issue
Jun 24, 2020
… VT output graphics options (#6420) ## Summary of the Pull Request Caches vectors in the class and uses a new helper to opportunistically shrink/grow as viewport sizes change in order to save performance on alloc/free of commonly used vectors. ## PR Checklist * [x] Scratches a perf itch. * [x] I work here. * [x] wil tests added * [x] No add'l doc. * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two fixes: 1. For outputting lots of text, the base renderer class spent a lot of time allocating and freeing and reallocating the `Cluster` vector that adapts the text buffer information into render clusters. I've now cached this vector in the base render class itself and I shrink/grow it based on the viewport update that happens at the top of every frame. To prevent too much thrashing in the downward/shrink direction, I wrote the `til::manage_vector` helper that contains a threshold to only shrink if it asks for small enough of a size relative to the existing one. I used 80% of the existing size as the threshold for this one. 2. For outputting lots of changing colors, the VT graphics output engine spent a bunch of time allocating and reallocating the vector for `GraphicsOptions`. This one doesn't really have a predictable size, but I never expect it to get extremely big. So I just held it in the base class. ## Validation Steps Performed * [x] Ran the til unit test * [x] Checked render cluster vector time before/after against `big.txt` from #1064 * [x] Checked VT graphics output vector time before/after against `cacafire` Case | Before | After ---|---|---| `big.txt` |  |  `cacafire` |  |  (cherry picked from commit 0c93b2e)
ThunderPower
added a commit
to ThunderPower/terminal
that referenced
this issue
Sep 5, 2020
* Draw the cursor underneath text, and above the background (#6337)
## 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.
* Add a test to ensure support for trailing commas in `settings.json` (#6312)
## Summary of the Pull Request
Adds support for trailing commas in our json files.
## References
* Enabled due to the excellent work over in https://github.com/open-source-parsers/jsoncpp/pull/1098
## PR Checklist
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
* Improve support for VT character sets (#4496)
This PR improves our VT character set support, enabling the [`SCS`]
escape sequences to designate into all four G-sets with both 94- and
96-character sets, and supports invoking those G-sets into both the GL
and GR areas of the code table, with [locking shifts] and [single
shifts]. It also adds [`DOCS`] sequences to switch between UTF-8 and the
ISO-2022 coding system (which is what the VT character sets require),
and adds support for a lot more characters sets, up to around the level
of a VT510.
[`SCS`]: https://vt100.net/docs/vt510-rm/SCS.html
[locking shifts]: https://vt100.net/docs/vt510-rm/LS.html
[single shifts]: https://vt100.net/docs/vt510-rm/SS.html
[`DOCS`]: https://en.wikipedia.org/wiki/ISO/IEC_2022#Interaction_with_other_coding_systems
## Detailed Description of the Pull Request / Additional comments
To make it easier for us to declare a bunch of character sets, I've made
a little `constexpr` class that can build up a mapping table from a base
character set (ASCII or Latin1), along with a collection of mappings for
the characters the deviate from the base set. Many of the character sets
are simple variations of ASCII, so they're easy to define this way.
This class then casts directly to a `wstring_view` which is how the
translation tables are represented in most of the code. We have an array
of four of these tables representing the four G-sets, two instances for
the active left and right tables, and one instance for the single shift
table.
Initially we had just one `DesignateCharset` method, which could select
the active character set. We now have two designate methods (for 94- and
96- character sets), and each takes a G-set number specifying the target
of the designation, and a pair of characters identifying the character
set that will be designated (at the higher VT levels, character sets are
often identified by more than one character).
There are then two new `LockingShift` methods to invoke these G-sets
into either the GL or GR area of the code table, and a `SingleShift`
method which invokes a G-set temporarily (for just the next character
that is output).
I should mention here that I had to make some changes to the state
machine to make these single shift sequences work. The problem is that
the input state machine treats `SS3` as the start of a control sequence,
while the output state machine needs it to be dispatched immediately
(it's literally the _Single Shift 3_ escape sequence). To make that
work, I've added a `ParseControlSequenceAfterSs3` callback in the
`IStateMachineEngine` interface to decide which behavior is appropriate.
When it comes to mapping a character, it's simply an array reference
into the appropriate `wstring_view` table. If the single shift table is
set, that takes preference. Otherwise the GL table is used for
characters in the range 0x20 to 0x7F, and the GR table for characters
0xA0 to 0xFF (technically some character sets will only map up to 0x7E
and 0xFE, but that's easily controlled by the length of the
`wstring_view`).
The `DEL` character is a bit of a special case. By default it's meant to
be ignored like the `NUL` character (it's essentially a time-fill
character). However, it's possible that it could be remapped to a
printable character in a 96-character set, so we need to check for that
after the translation. This is handled in the `AdaptDispatch::Print`
method, so it doesn't interfere with the primary `PrintString` code
path.
The biggest problem with this whole process, though, is that the GR
mappings only really make sense if you have access to the raw output,
but by the time the output gets to us, it would already have been
translated to Unicode by the active code page. And in the case of UTF-8,
the characters we eventually receive may originally have been composed
from two or more code points.
The way I've dealt with this was to disable the GR translations by
default, and then added support for a pair of ISO-2022 `DOCS` sequences,
which can switch the code page between UTF-8 and ISO-8859-1. When the
code page is ISO-8859-1, we're essentially receiving the raw output
bytes, so it's safe to enable the GR translations. This is not strictly
correct ISO-2022 behavior, and there are edge cases where it's not going
to work, but it's the best solution I could come up with.
## Validation Steps Performed
As a result of the `SS3` changes in the state machine engine, I've had
to move the existing `SS3` tests from the `OutputEngineTest` to the
`InputEngineTest`, otherwise they would now fail (technically they
should never have been output tests).
I've added no additional unit tests, but I have done a lot of manual
testing, and made sure we passed all the character set tests in Vttest
(at least for the character sets we currently support). Note that this
required a slightly hacked version of the app, since by default it
doesn't expose a lot of the test to low-level terminals, and we
currently identify as a VT100.
Closes #3377
Closes #3487
* Reflect OS build fixes on 7b489128ac back to inbox
Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 677d15a4e298a0e1e3ce093bc1dff8d832e3c2b1
Related work items: #26765368
* Allow Ctrl+Alt <> AltGr aliasing to be disabled (#6212)
## Summary of the Pull Request
Some people wish to use Ctrl+Alt combinations without Windows treating those as an alias for AltGr combinations. This PR adds a new `altGrAliasing` setting allowing one to control this behavior.
## PR Checklist
* [x] Closes #6211
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Manual testing
* [x] Requires documentation to be updated: https://github.com/MicrosoftDocs/terminal/issues/50
* [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
## Validation Steps Performed
* Choose a German keyboard layout
* Using `showkey -a` ensured that both `Ctrl+Alt+Q/E` and `AltGr+Q/E` produce `@/€`
* Added `"altGrAliasing": false` to the WSL profile
* Using `showkey -a` ensured `Ctrl+Alt+Q/E` now produces `^[^Q/E` while `AltGr+Q/E` continues to produce `@/€`
* Move to Microsoft.UI.Xaml 2.4.0 (#5778)
This brings support for "Compact" tab sizing, which compresses all inactive tabs to just the size of their icons plus the close button. Neat!
It also just keeps us generally up-to-date and good citizens.
* Spec for tab tear off and default app (#2080)
docs have no build, cleaning out draft spec from PR queue
* First draft of a spec for panes with non-terminal content (#1080)
Co-authored-by: Michael Niksa <miniksa@microsoft.com>
putting outdated draft spec into drafts folder and closing PR. CI has no function on docs.
* fix spell things I forced into master.
* Add support for `compact` tab sizing (#5787)
## Summary of the Pull Request
Really couldn't be more starightforward. MUX 2.4 added support for "compact" sized tabs. This PR (targeting the 2.4 PR currently, will move to `master` when that merges) enables users to specify `"tabWidthMode": "compact"` in their global settings to get this behavior.
## References
* #5778 - PR to move to MUX 2.4
* [microsoft-ui-xaml#2016](https://github.com/microsoft/microsoft-ui-xaml/pull/2016) - the MUX PR for compact tab sizing.
* #597 - Tab sizing options?
## PR Checklist
* [x] I don't think we have an issue for this, though I could be wrong.
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
In this screenshot, I'm hovering over tab 2, but the ubuntu tab is focused:
In this screenshot, tab 2 is focused:
* spelling I
* spelling II
* Last round of master spelling fixes
* Update the tab's close button color to match the tab text color (#5789)
## Summary of the Pull Request
When we select a color for the tab, we update the foreground color of the text so that it maintains acceptable contrast with the new tab color. However, we weren't also updating the foreground color of the close button.
This is understandable though, because apparently this wasn't fixable until MUX 2.4 arrived. I'm not a XAML expert, but I know that setting this key only works when we're using MUX 2.4, so I'm assuming something about the TabView implementation changed in that release. _This PR is marked as a draft until #5778 is merged, then I'll re-target to master._
## References
* #5778 - PR to move to MUX 2.4
* This bug was introduced with the tab color picker in #3789
## PR Checklist
* [x] Closes #5780
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
A light tab color:
A dark tab color:
* Make the conversion from WORD to TextAttribute explicit (#6380)
In Windows, we build with /Zc:wchar_t- (which makes wchar_t an unsigned
short typedef.) This causes build breaks when we compare two wchar_t
values (or a wchar_t and an enum class that's of type wchar_t) and the
compiler decides that it might want to _promote them to TextAttribute_
before doing the comparison.
* Reflect OS build fixes back from f90f3bf99
Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 272dfa1c4ad5e4202c4c56f3db7a445dc0b003cf
* 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
* Move all wildcards into targets or expand them (#6406)
Wildcards are not allowed in toplevel ItemGroups in vcxproj; they must
be generated by targets.
We mostly use wildcards for pulling in PRI files that are dumped on disk
by the translation tool. We don't want to check those in, so we can't
expand references to them.
To that end, I've introduced a new target that will take a list of
folders containing resw files and expand wildcards under them.
All[1] other wildcards have been moved into their respective targets
_or_ simply expanded.
[1]: Nothing has complained about the resource wildcards in
CascadiaResources.build.items, so I haven't exploded it yet.
Fixes #6214.
* Remove the shell extension from directory backgrounds (#6415)
We're removing this because of MSFT:24623699, which prevents us from being able to do the right thing when we're called on the background of a directory for a range of OS builds.
#6414 will track re-adding this to the Terminal when the original issue is closed.
* [x] closes #6245
* I work here
* Add support for win32-input-mode to conhost, ConPTY, Terminal (#6309)
Adds support for `win32-input-mode` to conhost, conpty, and the Windows
Terminal.
* The shared `terminalInput` class supports sending these sequences when
a VT client application requests this mode.
* ConPTY supports synthesizing `INPUT_RECORD`s from the input sent to it
from a terminal
* ConPTY requests this mode immediately on startup (if started with a
new flag, `PSEUDOCONSOLE_WIN32_INPUT_MODE`)
* The Terminal now supports sending this input as well, when conpty asks
for it.
Also adds a new ConPTY flag `PSEUDOCONSOLE_WIN32_INPUT_MODE` which
requests this functionality from conpty, and the Terminal requests this
by default.
Also adds `experimental.input.forceVT` as a global setting to let a user
opt-out of this behavior, if they don't want it / this ends up breaking
horribly.
## Validation Steps Performed
* played with this mode in vtpipeterm
* played with this mode in Terminal
* checked a bunch of scenarios, as outlined in a [comment] on #4999
[comment]: https://github.com/microsoft/terminal/issues/4999#issuecomment-628718631
References #4999: The megathread
References #5887: The spec
Closes #879
Closes #2865
Closes #530
Closes #3079
Closes #1119
Closes #1694
Closes #3608
Closes #4334
Closes #4446
* Remove parentheses from Preview and Dev build (#6418)
## Summary of the Pull Request
Remove parentheses from the Preview and Dev build. Now they're called Windows Terminal Preview and Windows Terminal Dev Build respectively.
Also removed them from other identifiers of Terminal for consistency.
## PR Checklist
* [X] Closes #5974
* Upload Windows Terminal 2.0 roadmap (#6419)
<!-- 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
Upload the roadmap for Windows Terminal 2.0 and link to it on the README.
<!-- 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
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
<!-- 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
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* wpf: add a .NET Core WPF Test project for the WPF Control (#6441)
This commit introduces a new project that lets you F5 a working instance
of the Wpf Terminal Control.
To make the experience as seamless as possible, I've introduced another
solution platform called "DotNet_x64Test". It is set to build the WPF
projects for "Any CPU" and every project that PublicTerminalCore
requires (including itself) for "x64". This is the only way to ensure
that when you press F5, all of the native and managed dependencies get
updated.
It's all quite cool when it works.
* Set tab title as early as possible (#6433)
When opening a new tab, it takes a few milliseconds before title to
appears. This PR makes it instantaneous.
* Updated the Terminal so that it can load the title from the settings
before it is initialized.
* Load terminal settings in TermControl constructor before the terminal
is initialized (see above).
* Update Tab so that it sets the TabViewItem's title in the constructor
(in Tab::_MakeTabViewItem) instead of waiting for the VT sequence to
set the title (from what I understand).
NOTE 1: there is a similar problem with the tabview icon which is not
fixed by this PR.
NOTE 2: This is only a problem with animations disabled because
otherwise the title fades in so there is enough time for it to be set
when it becomes visible.
## Validation
I ran the terminal and opened a new tab. The title appears instantly.
* Don't snap on input nor dismiss selection for just a modifier key (#6431)
Does what it says on the label. Pure modifier keys weren't making it
this far at all prior to #6309. This PR changes these methods to make
sure that we only dismiss a selection or snap on input when the key
pressed isn't a modifier key.
## References
* regressed in #6309
## PR Checklist
* [x] Closes #6423
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Tried to repro this in the Terminal, couldn't anymore.
* Don't send a manual F7 keyup (#6442)
## Summary of the Pull Request
When someone asked "Do we need to send a F7 keyup too" in #6309, the right answer was actually _no_. Turns out that while XAML will eat the F7 key**down**, it _won't_ eat the F7 key**up**.
## References
* regressed in #6309
## PR Checklist
* [x] Closes #6438
* [x] I work here
* [ ] Tested manually
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Checked this with the debug tap
* Tie up some A11y loose threads (#6417)
This pull request moves WindowUiaProvider back into Win32 interactivity
and deletes all mention of it from Windows Terminal. Terminal does not
have a single toplevel window that requires Console-like UIA, as each
Xaml control inside it is in charge of its own destiny.
I've also merged `IUiaWindow` and `IConsoleWindow` back together, as
well as `WindowUiaProviderBase` and `WindowUiaProvider`.
Things look a lot more like they did before we tore them apart.
## PR Checklist
* [x] Closes #3564
* [x] CLA
* [x] Tests added/passed (manual)
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already
## Validation
Carlos validated conhost and terminal on this branch.
* WpfTest: Add an x86/Win32 build, make DPI aware (#6455)
This matches more closely how Visual Studio uses the WPF control.
It comes in the form of _another platform_ (sorry), `DotNet_x86Test`.
* Improve perf by avoiding vector reallocation in renderer clusters and VT output graphics options (#6420)
## Summary of the Pull Request
Caches vectors in the class and uses a new helper to opportunistically shrink/grow as viewport sizes change in order to save performance on alloc/free of commonly used vectors.
## PR Checklist
* [x] Scratches a perf itch.
* [x] I work here.
* [x] wil tests added
* [x] No add'l doc.
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
Two fixes:
1. For outputting lots of text, the base renderer class spent a lot of time allocating and freeing and reallocating the `Cluster` vector that adapts the text buffer information into render clusters. I've now cached this vector in the base render class itself and I shrink/grow it based on the viewport update that happens at the top of every frame. To prevent too much thrashing in the downward/shrink direction, I wrote the `til::manage_vector` helper that contains a threshold to only shrink if it asks for small enough of a size relative to the existing one. I used 80% of the existing size as the threshold for this one.
2. For outputting lots of changing colors, the VT graphics output engine spent a bunch of time allocating and reallocating the vector for `GraphicsOptions`. This one doesn't really have a predictable size, but I never expect it to get extremely big. So I just held it in the base class.
## Validation Steps Performed
* [x] Ran the til unit test
* [x] Checked render cluster vector time before/after against `big.txt` from #1064
* [x] Checked VT graphics output vector time before/after against `cacafire`
Case | Before | After
---|---|---|
`big.txt` |  | 
`cacafire` |  | 
* Reduce latency with DXGI 1.3 GetFrameLatencyWaitableObject (#6435)
This pull request reduces input lag, especially with selection, by using
`IDXGISwapChain2::GetFrameLatencyWaitableObject`.
This is based on the [DXGI 1.3 documentation].
Excerpt from the [DXGI 1.3 improvement list]:
> The following functionality has been added in Microsoft DirectX
Graphics Infrastructure (DXGI) 1.3, which is included starting in
Windows 8.1.
Before, during rendering:
1. render frame
2. call `Present` on swap chain:
2.a. blocks until it can present
2.b. meanwhile, selection/text in terminal might have changed, but
we're still using the frame that we rendered before blocking
2.c. presents
After, during rendering:
1. block until we can present
2. render frame with latest data
3. call `Present` on swap chain:
3.a. present without blocking
[DXGI 1.3 documentation]: https://docs.microsoft.com/en-us/windows/uwp/gaming/reduce-latency-with-dxgi-1-3-swap-chains
[DXGI 1.3 improvement list]: https://docs.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-1-3-improvements:
* Open the system menu when user right clicks the drag bar (#6443)
Related to #1375 ("Click/Right click icon should display
Minimize/Maximize/Close menu")
* Deps: Bump Newtonsoft.Json from 10.0.3 to 12.0.3 (#6369)
Bump Newtonsoft.Json from 10.0.3 to 12.0.3
## References
Part of #5297
## PR Checklist
* [ ] Closes (none)
* [x] CLA signed
* [ ] Tests added/passed N/A
* [ ] Requires documentation to be updated N/A
* [ ] 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
## Validation Steps Performed
CI builds successfully
* Fix the x86 build and re-enable x86 CI (#6467)
This was a miss.
* Add Mini-Spec for openSettings (#5915)
The spec introduces a keybinding argument of 'target' to be able to open a specific settings file. When the Settings UI gets implemented, it will also become an option.
Alternative designs were presented but the 'target' model was decided on.
* Fix 3 different bugs in the WPF control (#6464)
* [wpf] WM_KEYUP crashes on x64 #6444
- Turns out that doing the `(uint)lParam` cast worked fine for the
keydowns, because the value of lParam usually didn't have super
high-order bits set. That's not the case for keyups, where the 30th
bit is _always_ set. This is fixed by explicitly getting the byte
with the scancode in it.
* [wpf] WM_KEYUP generates wrong value in Win32 input mode #6445
- This was fixed by basically the same thing as the above.
* [wpf] WPF control crashes on startup trying to render cursor #6446
- This was a regression from #6337. I forgot to initialize the brush
used to paint the cursor, because the UWP version always uses color
(but the WPF one relies on the text foreground color).
* Also adds a minor change to the WPF test app, so that the user can
actually exit `win32-input-mode`.
* #6337 regressed #6446
* #6309 regressed the other two.
Closes #6444
Closes #6445
Closes #6446
* Finalize Command Palette Spec (#5674)
## Summary of the Pull Request
This PR aims to move the command palette spec out of the draft state and into a finalized state for inclusion in the 2.0 version of the Windows Terminal.
Notably, I've added sections regarding the ability to run `wt` commandlines using the Command Palette UI, something we hadn't considered in the original draft, because `wt` commandlines didn't land for like _4 months_ after this first draft.
## References
* #2046 - the original command palette thread
* #2193 - the original draft PR
* #5400 - the new command palette megathread for WT 2.0, which I'll be updating with follow-up tasks as we work on implementing this.
## PR Checklist
* [x] Specs #2046
* [x] I work here
* [x] Is documentation
## Detailed Description of the Pull Request / Additional comments
_read the spec_
* Pass <Alt> to the application (#6461)
For mysterious reasons lost to the sands of time, XAML will _never_ pass
us a VK_MENU event. This is something that'll probably get fixed in
WinUI 3, but considering we're stuck on system XAML for the time being,
the only way to work around this bug is to pass the event through
manually. This change generalizes the F7 handler into a "direct key
event" handler that uses the same focus and tunneling method to send
different key events, and then uses it to send VK_MENU.
## Validation Steps Performed
Opened the debug tap, verified that I was seeing alt key ups.
Also used some alt keybindings to make sure I didn't break them.
Closes #6421
* Throttle scrollbar updates in TermControl to ~one per 8ms (#4608)
In addition to the below (original) description, this commit introduces
a ThrottledFunc template that can throttle _any_ function. It applies
that type to muffle updates to the scrollbar.
---
Redo #3531 but without the bug that it caused (#3622) which is why it
was reverted.
I'm sorry if I explain this badly. If you don't understand a part, make
sure to let me know and I will explain it better.
### Explanation
How it worked before: `Terminal` signals that viewport changed ->
`TermControl::_TerminalScrollPositionChanged` gets called on the
terminal thread -> it dispatches work for later to be ran the UI thread
to updates the scrollbar's values
Why it's bad:
* If we have many viewport changes, it will create a long stack of
operations to run. Instead, we should just update the scroll bar with
the most recent information that we know.
* Imagine if the rate that the work gets pushed on the UI thread is
greater than the rate that it can handle: it might freeze?
* No need to be real time, we can wait just a little bit (8ms) to
accumulate viewport changes before we actually change the scroll bar's
value because it appears to be expensive (see perf below).
Now: `Terminal` signals that viewport changed ->
`TermControl::_TerminalScrollPositionChanged` gets called on the
terminal thread -> it tells the `ScrollBarUpdater` about a new update ->
the `ScrollBarUpdater` only runs one job (I don't know if that's the
right term) on the UI thread at a time. If a job is already running but
hasn't updated the scroll bar yet, it changes the setting in the already
existing job to update the scroll bar with the new values. A job "waits"
some time before doing the update to throttle updates because we don't
need real time scroll bar updates. -> eventually, it updates the scroll
bar If the user scrolls when a scroll bar update is pending, we keep the
scroll bar's Maximum and Minimum but let the user choose its new Value
with the `CancelPendingValueChange` method.
### Note
Also I changed a little bit the code from the Terminal to notify the
TermControl less often when possible.
I tried to scroll with the scroll bar, with the mouse wheel. I tried to
scroll while content is being outputted.
I tried to reproduce the crash from #2248 without success (good).
Co-authored-by: Leonard Hecker <leonard@hecker.io>
Closes #3622
* Add keybinding arg to openSettings (#6299)
## Summary of the Pull Request
Adds the `target` keybinding arg to `openSettings`. Possible values include: `defaultsFile`, `settingsFile`, and `allFiles`.
## References
#5915 - mini-spec
## PR Checklist
* [x] Closes #2557
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
Implemented as discussed in the attached spec. A new enum will be added for the SettingsUI when it becomes available.
## Validation Steps Performed
Added the following to my settings.json:
```json
{ "command": "openSettings", "keys":... },
{ "command": { "action": "openSettings" }, "keys":... },
{ "command": { "action": "openSettings", "target": "settingsFile" }, "keys":... },
{ "command": { "action": "openSettings", "target": "defaultsFile" }, "keys":... },
{ "command": { "action": "openSettings", "target": "allFiles" }, "keys":... }
```
* Add fast path to til::bitmap::translate using bitshifts (#6493)
This commit adds a fast path to `til::bitmap::translate`: use bit shifts
when the delta is vertical.
Performance while printing the content of a big file, with the patch
from #6492 which hasn't been merged yet, in Release mode:
Before:
* translate represents 13.08% of samples in InvalidateScroll
After:
* translate represents 0.32% of samples in InvalidateScroll
## Validation
Tests passed.
* Clear cached runs after translate_y (#6501)
"While re-reading the code, I found out that I forgot to do clear cached runs
after translate_y in c360b7588ff8d389b49a4ed60cdee51401a5e172."
* Reintroduce the check for VT_INPUT_MODE in AdaptDispatch (#6485)
This commit reverts the removal of the "SSH hack" in #5383. It was
originally added as a solution to #4911, when we realized that SSH would
request the SS3 cursor key encoding but we weren't equipped to handle
it.
A number of folks have filed issues that, in summary, say "when I use
SSH, I can't select/copy/paste text". It turns out that SSH will _also_
pass through requests for mouse input. Terminal dutifully responds to
those requests, of course, by disabling mouse selection/copy/paste. SSH
is **NOT** actually in VT_INPUT_MODE, so it will never receive the mouse
messages.
It's important to note that even with #376 fixed, we are still required
to keep this check. With the closure of #376, we'll be able to convert
VT mouse input back into Win32 mouse input for Win32 applications . . .
but SSH also doesn't know how to handle Win32 mouse input.
Fixes #6476.
Fixes #6196.
Fixes #5704.
Fixes #5608.
* Extract `ActionAndArgs::FromJson` into its own class (#6351)
## Summary of the Pull Request
Pulls the `ActionAndArgs` deserializing into its own class, separate from `AppKeyBindings`. Some 2.0 features are going to need to re-use these actions in their json, so we'll want one unified way of deserializing them.
## References
* Done primarily as part of the work on #2046/#5400/#5674
* Also related: #1571/#5888
* Will aggressively conflict with any open PRs that introduced keybindings (looking at #6299)
## PR Checklist
* [x] Closes nothing, this is code refactoring
* [x] I work here
* [x] Current tests passed
* [n/a] Requires documentation to be updated
* Fix sending a NUL on alt key up (#6516)
## Summary of the Pull Request
Make sure to set the scancode for the manual alt-up's we're sending. If you don't, then terminalInput in the conpty is going to treat that keypress as an actual NUL, and send that to the connected client.
## References
* regressed in #6421
## PR Checklist
* [x] Closes #6513
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Tested `showkeys -a`
* Only snap on key _downs_ (#6517)
## Summary of the Pull Request
Prior to #6309, we'd only snap on input for non-modifier key_down_ events. #6423 fixed this for modifier keys, but didn't fix this for keyups.
## References
* #6423 was an incomplete fix to this problem, which caused this regression
## PR Checklist
* [x] Closes #6481
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
* Fix openSettings misspelling in defaults (#6520)
Fixes #6486.
* Remove the WinTelnetEngine (#6526)
Nobody was using it.
Discussed in #2661.
* Spec: Advanced Tab Switcher (#3753)
<!-- 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 is the spec for the Advanced Tab Switcher. This would allow the user to navigate through a vertical list of tabs through a UI, similar to those found in VSCode and Visual Studio.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#1502: Feature Request: Advanced Tab Switcher
#973: Ctrl+Tab toggling between two tabs
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Spec for #1502
* [x] CLA signed.
* Introduce JsonUtilsNew as documented in #5875 (#6355)
Read the [JsonUtils Spec] for more details.
This pull request introduces the next version of JsonUtils. It is in a
separate file for ease of review and testing.
JsonUtilsNew will be renamed in a subsequent commit that rewrites our
JSON deserializers.
### Implementer's Notes
I went with telescoping exceptions for the key parsing code, because
it's totally possible that you can be five keys deep and encounter a
type error. This lets us encode information about all failures in the
chain instead of just the topmost one.
The original JsonUtilsNew code changed to use `decay` everywhere because
the tests wouldn't compile. We want to treat `GetValue<const guid>` _the
same as_ `GetValue<guid>`, and this lets us do so. `decay` is awesome.
I've been developing this with a shim that redirects `JsonUtils.h` to
`JsonUtilsNew.h`. I am not comfortable deleting the original until we've
moved off of it, and that _will_ be the subject of a followup PR.
## Validation Steps Performed
So many tests.
[JsonUtils Spec]: https://github.com/microsoft/terminal/blob/master/doc/cascadia/Json-Utility-API.md
Refs #2550
* Fixed #6377: TerminalCore::_altGrAliasing is undefined by default (#6571)
## Summary of the Pull Request
Fixes #6377. `TerminalCore` does not initialize `_altGrAliasing`. The impact is minimized in WT because it defaults to `true` in higher layers. It's not initialized when WPF is driving.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #6377
* [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 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
* Use early returns in TermControl::_KeyHandler (#6575)
## Summary of the Pull Request
This PR changes `TermControl::_KeyHandler` to use early returns, which you can think of as "guard clauses".
This has the benefit of a reduced nesting level, easier to understand control flow and opens op the way to more complex conditions.
## PR Checklist
* [ ] Closes #xxx
* [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 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
## Validation Steps Performed
Everything still works as expected.
* doc: copyedit the roadmap a little bit (#6587)
* Add 1.1 blog post to 2.0 roadmap (#6581)
* Update PR template with link to docs (#6583)
* Improved ATTR_ROW::ReplaceAttrs performance (#6573)
## Summary of the Pull Request
Improve `ATTR_ROW::ReplaceAttrs` performance by only reserving the necessary capacity instead of resizing the new run.
That way `TextAttributeRun`s are only instantiated once instead of twice.
## PR Checklist
* [ ] Closes #xxx
* [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 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
Performance could be further improved by directly moving `TextAttributeRun`s into the new vector, but I considered this out of scope for this PR.
## Validation Steps Performed
CPU usage when running `cacafire` is slightly reduced.
* Add schema check to PR template (#6599)
This adds a check for updating the schema, and rewords the documentation checkbox to match the wording of the others.
* Enable hot reload of renderer settings that aren't already hot reload capable (#6551)
## Summary of the Pull Request
## PR Checklist
* [x] Closes #3927
* [x] I work here.
* [x] Tested manually.
* [x] Requires documentation to be updated: (generate doc bug here)
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
- I found four settings that weren't hot reloadable with the 3927 comment above them:
1. Experimental retro terminal effect
2. Experimental software rendering
3. Experimental full repaint rendering
4. Antialiasing settings for text
I made them all hot reloadable by telling the `TermControl` to propagate them on settings change to the `DxEngine`.
Then I set up the `DxEngine` inside the setters to only set them if they changed. And if they do change, to trigger a full repaint and/or a complete drop and recreate of the entire DX device chain (as would happen if it were lost for another reason like a user-mode graphics failure, disconnected display, etc.)
I made the boolean an atomic because the settings can be coming in off of another thread (the XAML eventing one) and the renderer is picking the status up on its thread at the top of the BeginPaint frame.
## Validation Steps Performed
- [x] Opened it up and toggled all the settings while staring at PowerShell
- [x] Opened it up and toggled all the settings while staring at something intensive like a `cacafire` fire
* Improve bitmap::_calculateArea performance (#6572)
`bitmap::_calculateArea` performance can be improved by leveraging the
optimized `find_first`/`find_next` methods instead of iterating through
the bitmap manually.
* Use D2DDeviceContext and friends over D2DRenderTarget (#6527)
I was told that the DeviceContext version supercedes the RenderTarget
one. This moves us to it so we can gain access to a higher level of
control over the various pieces in our pipeline as we continue to evolve
the renderer.
The underlying motivation here is to potentially use a
`ID2D1CommandList` to batch our commands and run them all later outside
the lock. That can only really be done with this more granular level of
control over the pipeline. So this moves to that in a single step that
is easily findable in history should we have problems
I discussed this with @NiklasBorson of the Direct2D/DirectWrite team as
well as with @DHowett before doing it.
## Validation
- [x] Checked docs to make sure that these work on Windows 7 with
Platform Update
- [x] Manual smoke test real quick
- [ ] Try running on Win7 + Platform Update after change
- [x] Probably do more than just a smoke test manually or otherwise
Closes #6525
* Always use the dark window borders (#6624)
* Recycle assorted rendering components to accelerate drawing (#6483)
This saves an awful lot of construction/destruction and memory
allocation, especially with text that changes a lot (see: cacafire).
Three things:
1. Recycling the text layouts. This holds onto the `CustomTextLayout` so
all the things that don't change related to drawing targets and
whatnot aren't freed and recreated every frame.
2. Reordering the runs in place. This saves a vector
allocation/copy/delete every time OrderRuns is called. They can be
rearranged in place.
3. Only clip once per row. This reduces the clip push/pop to only one
time per row. Since we're always redrawing an entire row at a time,
this saves a lot of alloc/free of the clip frame, dramatically
reduces queued commands, and makes less work on the flush since
clipping requires staging the drawing and then bringing it back to
the main surface.
* Double-click a tab to rename it (#6628)
<!-- 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
When the user double clicks on a tab, show the tab rename box
as if they right clicked on the tab and clicked on "Rename".
<!-- 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 #6600
* [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
I added a handler for the `DoubleTapped` event on the tab view item
when we are constructing it for the tab (in `Tab::_MakeTabViewItem`).
The code for that handler was copied the "rename tab menu item" click
handler.
I did not extract the code into a member function because it is very
short (only 2 lines of code) and only used twice so it is not worth
it IMO.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* Spec for unified keybindings and commands, and synthesized action names (#6532)
## Summary of the Pull Request
This is another iteration on the Command Palette spec, from #5674. These were some ideas that were tossed around by @DHowett, @cinnamon-msft and myself, formalized here. I proposed this as an addendum to the original spec, since I think the first made sense atomically, and this only makes sense as a set of changes to the original. I didn't want to go hacking up the original doc to add this set of changes.
**There are two proposals in this spec - they should be viewed as two atomic units. They can be accepted or rejected independently. I'm suggesting we approve both. They work _together_. I'm realizing now that this is worded confusingly, and it's on me to fix that.**
## PR Checklist
* [x] Another spec in the #2046 / #5400 saga
* [x] I work here
* [x] _is a doc_
> ## Abstract
>
> This document is intended to serve as an addition to the [Command Palette Spec].
> While that spec is complete in it's own right, subsequent discussion revealed
> additional ways to improve the functionality and usability of the command
> palette. This document builds largely on the topics already introduced in the
> original spec, so readers should first familiarize themselves with that
> document.
>
> One point of note from the original document was that the original specification
> was entirely too verbose when defining both keybindings and commands for
> actions. Consider, for instance, a user that wants to bind the action "duplicate
> the current pane". In that spec, they need to add both a keybinding and a
> command:
>
> ```json
> {
> "keybindings": [
> { "keys": [ "ctrl+alt+t" ], "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" } },
> ],
> "commands": [
> { "name": "Duplicate Pane", "action": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "icon": null },
> ]
> }
> ```
>
> These two entries are practically the same, except for two key differentiators:
> * the keybinding has a `keys` property, indicating which key chord activates the
> action.
> * The command has a `name` property, indicating what name to display for the
> command in the Command Palette.
>
> What if the user didn't have to duplicate this action? What if the user could
> just add this action once, in their `keybindings` or `commands`, and have it
> work both as a keybinding AND a command?
>
* Optimize booleans (#6548)
<!-- 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
Many places in this codebase has an equality comparison to the boolean FALSE. This adds unneeded complexity as C and C++ has a NOT operand for use of these in if statements. This makes the code more readable in those areas.
<!-- 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] 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 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
One boolean being compared to FALSE was only used once, with the boolean name being "b", so it is better off not existing at all.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Unit Testing passed, compiler refactoring
* Replace std::map with std::unordered_map (#6640)
Replace std::map with std::unordered_map when the order doesn't matter
and hash functions are provided. Simple optimizations, but I expect the
performance should be strictly better, especially for
CodepointWidthDetector.hpp.
* Update _TerminalCursorPositionChanged to use ThrottledFunc (#6492)
* Update _TerminalCursorPositionChanged to use ThrottledFunc.
* Rename previous ThrottledFunc to ThrottledArgFunc because now
ThrottledFunc is for functions that do not take an argument.
* Update ThrottledFunc and ThrottledArgFunc to accept a CoreDispatcher
on which the function should be called for convenience.
* Don't use coroutines/winrt::fire_and_forget in
ThrottledFunc/ThrottledArgFunc because they are too slow (see PR).
_AdjustCursorPosition went from 17% of samples to 3% in performance
testing.
* version: bump to 1.2 on master
* Add keybinding to rename tab (#6557)
<!-- 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
Add keybinding for renaming a tab
<!-- 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] Fulfills format requirements set by #6567
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests passed
* [X] Requires documentation to be 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: #6567 and here (#6557)
This no longer c loses #6256, as the spec changed.
<!-- 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
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* Revert "Skip ... analysis when the ... text is simple (6206)" (#6665)
This reverts commit 94eab6e391a099814ce8f9a08c8cccf9f588b7d1.
We'll reintroduce this again after making sure it plays nicely with
recycling and box drawing glyphs.
Fixes #6488
Fixes #6664
* Implement Shift+MultiClick Selection Expansion (#6322)
This pull request implements shift+double/triple click. Proper behavior
(as described in #4557) is to only expand one selection point, not both.
Adding the `bool targetStart` was a bit weird. I decided on this being
the cleanest approach though because I still want `PivotSelection` to be
its own helper function. Otherwise, the concept of "pivoting" gets kinda
messy.
## Validation Steps Performed
Manual testing as described on attached issue.
Tests were added for Shift+Click and pivoting the selection too.
Closes #4557
* Add `setTabColor` and `openTabColorPicker` actions (#6567)
## Summary of the Pull Request
Adds a pair of `ShortcutAction`s for setting the tab color.
* `setTabColor`: This changes the color of the current tab to the provided color, or can be used to clear the color.
* `openTabColorPicker`: This keybinding immediately activates the tab color picker for the currently focused tab.
## References
## PR Checklist
* [x] scratches my own itch
* [x] I work here
* [x] Tests added/passed
* [x] https://github.com/MicrosoftDocs/terminal/pull/69
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
* hey look there are tests
* Tested with the following:
```json
// { "command": "setTabColor", "keys": [ "alt+c" ] },
{ "keys": "ctrl+alt+c", "command": { "action": "setTabColor", "color": "#123456" } },
{ "keys": "alt+shift+c", "command": { "action": "setTabColor", "color": null} },
{ "keys": "alt+c", "command": "openTabColorPicker" },
```
* When we add a new tab in compact mode, re-enforce Compact mode (#6670)
This workaround was suggested by @chingucoding in
microsoft/microsoft-ui-xaml#2711
Fixes #6570
## References
microsoft/microsoft-ui-xaml#2711
#6570
## PR Checklist
* [x] Closes an issue
* [x] CLA
* [x] Tested
* [x] Docs not required
* [x] Schema not required
* Hardcode the paths to Windows PowerShell and CMD (#6684)
Occasionally, we get users with corrupt PATH environment variables: they
can't lauch PowerShell, because for some reason it's dropped off their
PATH. We also get users who have stray applications named
`powershell.exe` just lying around in random system directories.
We can combat both of these issues by simply hardcoding where we expect
PowerShell and CMD to live. %SystemRoot% was chosen over %WINDIR%
because apparently (according to Stack Overflow), SystemPath is
read-only and WINDIR isn't.
Refs #6039, #4390, #4228 (powershell was not found)
Refs #4682, Fixes #6082 (stray powershell.exe)
* Add support for the Command Palette (#6635)
## Summary of the Pull Request
This adds a first iteration on the command palette. Notable missing features are:
* Commandline mode: This will be a follow-up PR, following the merge of #6537
* nested and iterable commands: These will additionally be a follow-up PR.
This is also additionally based off the addenda in #6532.
This does not bind a key for the palette by default. That will be done when the above follow-ups are completed.
## References
* #2046 - The original command palette thread
* #5400 - This is the megathread for all command palette issues, which is tracking a bunch of additional follow up work
* #5674 and #6532 - specs
* #6537 - related
## PR Checklist
* [x] Closes #2046
- incidentally also closes #6645
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - delaying this until it's more polished.
## Detailed Description of the Pull Request / Additional comments
* There's a lot of code for autogenerating command names. That's all in `ActionArgs.cpp`, because each case is so _not_ boilerplate, unlike the rest of the code in `ActionArgs.h`.
## Validation Steps Performed
* I've been playing with this for months.
* Tests
* Selfhost with the team
* Restore simple text runs, correct for crashes (#6695)
Restores the simple text run analysis and skipping of most of the
shaping/layout steps. Corrects one of the fast-path steps to ensure that
offsets and clusters are assigned.
## References
- Bug #6488
- Bug #6664
- Simple run PR #6206
- Simple run revert PR #6665
- Recycle glyph runs PR #6483
The "correction" functions, by which box drawing analysis is one of
them, is dependent on the steps coming before it properly assigning the
four main vectors of the text layout glyphs: indices, advances, offsets,
and clusters. When the fast path is identified by the code from #6206,
only two of those are fully updated: indices and advances. The offsets
doesn't tend to cause a problem because offsets are rarely used so
they're pretty much always 0 already (but this PR enforces that they're
zero for the simple/fast path.) The clusters, however, were not mapped
for the fast path. This showed itself in one of two ways:
1. Before the recycled runs PR #6483, the cluster map had a 0 in every
field for the stock initialized vector.
2. After the recycled runs PR #6483, the cluster map had the previous
run's mapping in it.
This meant that when we reached the steps where glyph runs were
potentially split during the correction phase for box drawing
characters, unexpected values were present to map the glyph indices to
clusters and were corrected, adjusted, or split in an unexpected
fashion.
For instance, the index out of range bug could appear when the default 0
values appended to the end of the clusters vector were decremented down
to a negative value during the run splitter as the true DWrite cluster
mapper doesn't generate that sort of pattern in the slow path case
without also breaking the run itself.
The resolution here is therefore to ensure that all of the fields
related to glyph layout are populated even in the fast path. This
doesn't affect the slow path because that one always populated all
fields by asking DWrite to do it. The fast path just skips a bunch of
DWrite steps because it can implicitly identify patterns and save a
bunch of time.
I've also identified a few vectors that weren't cleared on reset/reuse
of the layout. I'm clearing those now so the `.resize()` operations
performed on them to get to the correct lengths will fill them with
fresh and empty values instead of hanging on to ones that may have been
from the previous. This should be OK memory perf wise because the act of
`.clear()` on a vector shouldn't free anything, just mark it invalid.
And doing `.resize()` from an empty one should just default construct
them into already allocated space (which ought to be super quick).
## Validation
* [x] Far.exe doesn't crash and looks fine
* [x] "\e[30;47m\u{2500} What \u{2500}\e[m" from #6488 appears
appropriately antialiased
* [x] Validate the "\e[30;47m\u{2500} What \u{2500}\e[m" still works
when `FillGeometry` is nerfed as a quick test that the runs are split
correctly.
* [x] Put `u{fffd} into Powershell Core to make a replacement char in
the output. Then press enter a few times and see that shrunken initial
characters on random rows. Verify this is gone.
Closes #6668
Closes #6669
Co-Authored-By: Chester Liu <skyline75489@outlook.com>
* Update Cascadia Code to 2007.01 (#6721)
* Add support for OSC 52 (copy-to-clipboard) (#5823)
With this commit, terminal will be able to copy text to the system
clipboard by using OSC 52 MANIPULATE SELECTION DAATA.
We chose not to implement the clipboard querying functionality offered
by OSC 52, as sending the clipboard text to an application without the
user's knowledge or consent is an immense security hole.
We do not currently support the clipboard specifier Pc to specify which
clipboard buffer should be filled
# Base64 encoded `foo`
$ echo -en "\e]52;;Zm9v\a"
# Multiple lines
# Base64 encoded `foo\r\nbar`
$ echo -en "\e]52;;Zm9vDQpiYXI=\a"
Closes #2946.
* Replace old C headers (xxx.h) with modern ones (cxxx) (#5080)
* 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 …
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The claims on launch were that the speed of the terminal is superb. Looking at the code it does seem very much like it, that the fastest technologies are used and the coding standards are extremely high. However, when I tried to measure these claims, I was surprised that comparing it with Fluent Terminal (which is built atop web technologies) outperforms Windows Terminal in raw output speed.
Environment
Steps to reproduce
In my favorite Ubuntu WSL instance, I issued
Expected behavior
I'd have guessed Windows Terminal to be the fastest when compared with Fluent Terminal (my current terminal of choice) and the old-school terminal. Was there a goal for performance? This is totally a non critical issue, I just wanted to let you guys know. (Critical issue is AltGr combinations not working. Until that's not sorted out, I literally cannot type in the terminal.)
Actual behavior
Old-school terminal
Fluent Terminal
Windows Terminal
The text was updated successfully, but these errors were encountered: