mirror of
https://github.com/microsoft/terminal.git
synced 2025-12-10 00:48:23 -06:00
15 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Michael Niksa
|
2c2f4f9be2
|
[FHL] Make VTApiRoutines, which does VT translation for output (#11264)
Make a VTApiRoutines servicer that does minimal translations instead of environmental simulation for some output methods. Remaining methods are backed on the existing console host infrastructure (primarily input related methods). ## PR Checklist * [x] I work here * [x] It's Fix-Hack-Learn quality so it's behind a feature gate so we can keep refining it. But it's a start! To turn this on, you will have to be in the Dev or Preview rings (feature staged). Then add `experimental.connection.passthroughMode: true` to a profile and on the next launch, the flags will propagate down through the `ConptyConnection` into the underlying `Openconsole.exe` startup and tell it to use the passthrough mode instead of the full simulation mode. ## Validation Steps Performed - Played with it manually in CMD.exe, it seems to work mostly. - Played with it manually in Ubuntu WSL, it seems to work. - Played with it manually in Powershell and it's mostly sad. It'll get there. Starts #1173 |
||
|
Leonard Hecker
|
b3fab518f8
|
Prepare til wrappers for migrating off of SMALL_RECT (#11902)
This commit makes the following changes to `til::point/size/rectangle` for the following reasons: * Rename `rectangle` into `rect` This will make the naming consistent with a later `small_rect` struct as well as the existing Win32 POINT/SIZE/RECT structs. * Standardizes til wrappers on `int32_t` instead of `ptrdiff_t` Provides a consistent behavior between x86 and x64, preventing accidental errors on x86, as it's less rigorously tested than x64. Additionally it improves interop with MIDL3 which only supports fixed width integer types. * Standardizes til wrappers on throwing `gsl::narrow_error` Makes the behavior of our code more consistent. * Makes all eligible functions `constexpr` Because why not. * Removes implicit constructors and conversion operators This is a complex and controversial topic. My reasons are: You can't Ctrl+F for an implicit conversion. This breaks most non-IDE engines, like the one on GitHub or those we have internally at MS. This is important for me as these implicit conversion operators aren't cost free. Narrowing integers itself, as well as the boundary checks that need to be done have a certain, fixed overhead each time. Additionally the lack of noexcept prevents many advanced compiler optimizations. Removing their use entirely drops conhost's code segment size by around ~6.5%. ## References Preliminary work for #4015. ## PR Checklist * [x] I work here * [x] Tests added/passed ## Validation Steps Performed I'm mostly relying on our unit tests here. Both OpenConsole and WT appear to work fine. |
||
|
Michael Niksa
|
66fdc645f7
|
Set keyword flags on all tracelog events (#10098)
Set keyword flags on all events so those sharing a provider with telemetry do not fire unless tracing is enabled ## PR Checklist * [x] Closes #10093 * [x] I work here * [x] Tests passed * [x] Documentation added in `til.h` about how keywords work and at the only other site of keywords we define in the Host project tracing files. ## Detailed Description of the Pull Request / Additional comments I initially thought that we would need to split providers here to accomplish this... but @DHowett helped me realize that might be a lot of additional metadata and bloat binary size. So with help from a friend from fundamentals, I realized that we could use Keywords to differentiate here. We can no longer define 0 keywords as that represents an any/all scenario. Every `TraceLoggingWrite` event now needs a keyword. When our events have a keyword, they're not included in any trace. Additionally, when we have an explicit keyword to check that is different from the ones used for the telemetry pipeline, we can ensure that we only do "hard work" to generate debug trace data when an "ALL" type listener like TraceView or Windows Performance Recorder with our profiles is listening to these providers for ALL keyworded events. ## Validation Steps Performed - [x] - Built with full release build config to confirm performance is worse than dev builds BECAUSE of the telemetry event collector camping our provider and triggering full trace event generation on shared providers. - [x] - Built with full release build config to enable statistics collection and validated trace event collection is excluded and trace event short-circuits work with this change. - [x] - Checked that TraceView still sees both telemetry and tracing events - [x] - Checked that WPR with our .wprp profile sees both telemetry and tracing events |
||
|
Michael Niksa
|
525be22bd8
|
Eliminate more transient allocations: Titles and invalid rectangles and bitmap runs and utf8 conversions (#8621)
## References * See also #8617 ## PR Checklist * [x] Supports #3075 * [x] I work here. * [x] Manual test. ## Detailed Description of the Pull Request / Additional comments ### Window Title Generation Every time the renderer checks the title, it's doing two bad things that I've fixed: 1. It's assembling the prefix to the full title doing a concatenation. No one ever gets just the prefix ever after it is set besides the concat. So instead of storing prefix and the title, I store the assembled prefix + title and the bare title. 2. A copy must be made because it was returning `std::wstring` instead of `std::wstring&`. Now it returns the ref. ### Dirty Area Return Every time the renderer checks the dirty area, which is sometimes multiple times per pass (regular text printing, again for selection, etc.), a vector is created off the heap to return the rectangles. The consumers only ever iterate this data. Now we return a span over a rectangle or rectangles that the engine must store itself. 1. For some renderers, it's always a constant 1 element. They update that 1 element when dirty is queried and return it in the span with a span size of 1. 2. For other renderers with more complex behavior, they're already holding a cached vector of rectangles. Now it's effectively giving out the ref to those in the span for iteration. ### Bitmap Runs The `til::bitmap` used a `std::optional<std::vector<til::rectangle>>` inside itself to cache its runs and would clear the optional when the runs became invalidated. Unfortunately doing `.reset()` to clear the optional will destroy the underlying vector and have it release its memory. We know it's about to get reallocated again, so we're just going to make it a `std::pmr::vector` and give it a memory pool. The alternative solution here was to use a `bool` and `std::vector<til::rectangle>` and just flag when the vector was invalid, but that was honestly more code changes and I love excuses to try out PMR now. Also, instead of returning the ref to the vector... I'm just returning a span now. Everyone just iterates it anyway, may as well not share the implementation detail. ### UTF-8 conversions When testing with Terminal and looking at the `conhost.exe`'s PTY renderer, it spends a TON of allocation time on converting all the UTF-16 stuff inside to UTF-8 before it sends it out the PTY. This was because `ConvertToA` was allocating a string inside itself and returning it just to have it freed after printing and looping back around again... as a PTY does. The change here is to use `til::u16u8` that accepts a buffer out parameter so the caller can just hold onto it. ## Validation Steps Performed - [x] `big.txt` in conhost.exe (GDI renderer) - [x] `big.txt` in Terminal (DX, PTY renderer) - [x] Ensure WDDM and BGFX build under Razzle with this change. |
||
|
Michael Niksa
|
99c33e084a
|
Avoid copying the bitmap on the way into the tracing function (#6839)
## PR Checklist * [x] Closes perf itch. * [x] I work here. * [x] Manual perf test. * [x] Documentation irrelevant. * [x] Schema irrelevant. * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Passes the bitmap by ref into the tracing function instead of making a copy on the way in. It's only read anyway for tracing (if enabled) so the copy was a pointless oversight. ## Validation Steps Performed - Observed WPR trace before and after with `time cat big.txt` in WSL. |
||
|
Mike Griese
|
6fabc4abb7
|
Fix copying wrapped lines by implementing better scrolling (#5181)
Now that the Terminal is doing a better job of actually marking which lines were and were not wrapped, we're not always copying lines as "wrapped" when they should be. We're more correctly marking lines as not wrapped, when previously we'd leave them marked wrapped. The real problem is here in the `ScrollFrame` method - we'd manually newline the cursor to make the terminal's viewport shift down to a new line. If we had to scroll the viewport for a _wrapped_ line, this would cause the Terminal to mark that line as broken, because conpty would emit an extra `\n` that didn't actually exist. This more correctly implements `ScrollFrame`. Now, well move where we "thought" the cursor was, so when we get to the next `PaintBufferLine`, if the cursor needs to newline for the next line, it'll newline, but if we're in the middle of a wrapped line, we'll just keep printing the wrapped line. A couple follow up bugs were found to be caused by the same bad logic. See #5039 and #5161 for more details on the investigations there. ## References * #4741 RwR, which probably made this worse * #5122, which I branched off of * #1245, #357 - a pair of other conpty wrapped lines bugs * #5228 - A followup issue for this PR ## PR Checklist * [x] Closes #5113 * [x] Closes #5180 (by fixing DECRST 25) * [x] Closes #5039 * [x] Closes #5161 (by ensuring we only `removeSpaces` on the actual bottom line) * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed * Checked the cases from #1245, #357 to validate that they still work * Added more and more tests for these scenarios, and then I added MORE tests * The entire team played with this in selfhost builds |
||
|
Michael Niksa
|
ef80f665d3
|
Correct scrolling invalidation region for tmux in pty w/ bitmap (#5122)
Correct scrolling invalidation region for tmux in pty w/ bitmap Add tracing for circling and scrolling operations. Fix improper invalidation within AdjustCursorPosition routine in the subsection about scrolling down at the bottom with a set of margins enabled. ## References - Introduced with #5024 ## Detailed Description of the Pull Request / Additional comments - This occurs when there is a scroll region restriction applied and a newline operation is performed to attempt to spin the contents of just the scroll region. This is a frequent behavior of tmux. - Right now, the Terminal doesn't support any sort of "scroll content" operation, so what happens here generally speaking is that the PTY in the ConHost will repaint everything when this happens. - The PTY when doing `AdjustCursorPosition` with a scroll region restriction would do the following things: 1. Slide literally everything in the direction it needed to go to take advantage of rotating the circular buffer. (This would force a repaint in PTY as the PTY always forces repaint when the buffer circles.) 2. Copy the lines that weren't supposed to move back to where they were supposed to go. 3. Backfill the "revealed" region that encompasses what was supposed to be the newline. - The invalidations for the three operations above were: 1. Invalidate the number of rows of the delta at the top of the buffer (this part was wrong) 2. Invalidate the lines that got copied back into position (probably unnecessary, but OK) 3. Invalidate the revealed/filled-with-spaces line (this is good). - When we were using a simple single rectangle for invalidation, the union of the top row of the buffer from 1 and the bottom row of the buffer from 2 (and 3 was irrelevant as it was already unioned it) resulted in repainting the entire buffer and all was good. - When we switched to a bitmap, it dutifully only repainted the top line and the bottom two lines as the middle ones weren't a consequence of intersect. - The logic was wrong. We shouldn't be invalidating rows-from-the-top for the amount of the delta. The 1 part should be invalidating everything BUT the lines that were invalidated in parts 2 and 3. (Arguably part 2 shouldn't be happening at all, but I'm not optimizing for that right now.) - So this solves it by restoring an entire screen repaint for this sort of slide data operation by giving the correct number of invalidated lines to the bitmap. ## Validation Steps Performed - Manual validation with the steps described in #5104 - Automatic test `ConptyRoundtripTests::ScrollWithMargins`. Closes #5104 |
||
|
Michael Niksa
|
ca33d895a3
|
Move ConPTY to use til::bitmap (#5024)
## Summary of the Pull Request Moves the ConPTY drawing mechanism (`VtRenderer`) to use the fine-grained `til::bitmap` individual-dirty-bit tracking mechanism instead of coarse-grained rectangle unions to improve drawing performance by dramatically reducing the total area redrawn. ## PR Checklist * [x] Part of #778 and #1064 * [x] I work here * [x] Tests added and updated. * [x] I'm a core contributor ## Detailed Description of the Pull Request / Additional comments - Converted `GetDirtyArea()` interface from `IRenderEngine` to use a vector of `til::rectangle` instead of the `SMALL_RECT` to banhammer inclusive rectangles. - `VtEngine` now holds and operates on the `til::bitmap` for invalidation regions. All invalidation operation functions that used to be embedded inside `VtEngine` are deleted in favor of using the ones in `til::bitmap`. - Updated `VtEngine` tracing to use new `til::bitmap` on trace and the new `to_string()` methods detailed below. - Comparison operators for `til::bitmap` and complementary tests. - Fixed an issue where the dirty rectangle shortcut in `til::bitmap` was set to 0,0,0,0 by default which means that `|=` on it with each `set()` operation was stretching the rectangle from 0,0. Now it's a `std::optional` so it has no value after just being cleared and will build from whatever the first invalidated rectangle is. Complementary tests added. - Optional run caching for `til::bitmap` in the `runs()` method since both VT and DX renderers will likely want to generate the set of runs at the beginning of a frame and refer to them over and over through that frame. Saves the iteration and creation and caches inside `til::bitmap` where the chance of invalidation of the underlying data is known best. It is still possible to iterate manually with `begin()` and `end()` from the outside without caching, if desired. Complementary tests added. - WEX templates added for `til::bitmap` and used in tests. - `translate()` method for `til::bitmap` which will slide the dirty points in the direction specified by a `til::point` and optionally back-fill the uncovered area as dirty. Complementary tests added. - Moves all string generation for `til` types `size`, `point`, `rectangle`, and `some` into a `to_string` method on each object such that it can be used in both ETW tracing scenarios AND in the TAEF templates uniformly. Adds a similar method for `bitmap`. - Add tagging to `_bitmap_const_iterator` such that it appears as a valid **Input Iterator** to STL collections and can be used in a `std::vector` constructor as a range. Adds and cleans up operators on this iterator to match the theoretical requirements for an **Input Iterator**. Complementary tests added. - Add loose operators to `til` which will allow some basic math operations (+, -, *, /) between `til::size` and `til::point` and vice versa. Complementary tests added. Complementary tests added. - Adds operators to `til::rectangle` to allow scaling with basic math operations (+, -, *) versus `til::size` and translation with basic math operations (+, -) against `til::point`. Complementary tests added. - In-place variants of some operations added to assorted `til` objects. Complementary tests added. - Update VT tests to compare invalidation against the new map structure instead of raw rectangles where possible. ## Validation Steps Performed - Wrote additional til Unit Tests for all additional operators and functions added to the project to support this operation - Updated the existing VT renderer tests - Ran perf check |
||
|
Mike Griese
|
93b31f6e3f
|
Add support for "reflow"ing the Terminal buffer (#4741)
This PR adds support for "Resize with Reflow" to the Terminal. In conhost, `ResizeWithReflow` is the function that's responsible for reflowing wrapped lines of text as the buffer gets resized. Now that #4415 has merged, we can also implement this in the Terminal. Now, when the Terminal is resized, it will reflow the lines of it's buffer in the same way that conhost does. This means, the terminal will no longer chop off the ends of lines as the buffer is too small to represent them. As a happy side effect of this PR, it also fixed #3490. This was a bug that plagued me during the investigation into this functionality. The original #3490 PR, #4354, tried to fix this bug with some heavy conpty changes. Turns out, that only made things worse, and far more complicated. When I really got to thinking about it, I realized "conhost can handle this right, why can't the Terminal?". Turns out, by adding resize with reflow, I was also able to fix this at the same time. Conhost does a little bit of math after reflowing to attempt to keep the viewport in the same relative place after a reflow. By re-using that logic in the Terminal, I was able to fix #3490. I also included that big ole test from #3490, because everyone likes adding 60 test cases in a PR. ## References * #4200 - this scenario * #405/#4415 - conpty emits wrapped lines, which was needed for this PR * #4403 - delayed EOL wrapping via conpty, which was also needed for this * #4354 - we don't speak of this PR anymore ## PR Checklist * [x] Closes #1465 * [x] Closes #3490 * [x] Closes #4771 * [x] Tests added/passed ## EDIT: Changes to this PR on 5 March 2020 I learned more since my original version of this PR. I wrote that in January, and despite my notes that say it was totally working, it _really_ wasn't. Part of the hard problem, as mentioned in #3490, is that the Terminal might request a resize to (W, H-1), and while conpty is preparing that frame, or before the terminal has received that frame, the Terminal resizes to (W, H-2). Now, there aren't enough lines in the terminal buffer to catch all the lines that conpty is about to emit. When that happens, lines get duplicated in the buffer. From a UX perspective, this certainly looks a lot worse than a couple lost lines. It looks like utter chaos. So I've introduced a new mode to conpty to try and counteract this behavior. This behavior I'm calling "quirky resize". The **TL;DR** of quirky resize mode is that conpty won't emit the entire buffer on a resize, and will trust that the terminal is prepared to reflow it's buffer on it's own. This will enable the quirky resize behavior for applications that are prepared for it. The "quirky resize" is "don't `InvalidateAll` when the terminal resizes". This is added as a quirk as to not regress other terminal applications that aren't prepared for this behavior (gnome-terminal, conhost in particular). For those kinds of terminals, when the buffer is resized, it's just going to lose lines. That's what currently happens for them. When the quirk is enabled, conpty won't repaint the entire buffer. This gets around the "duplicated lines" issue that requesting multiple resizes in a row can cause. However, for these terminals that are unprepared, the conpty cursor might end up in the wrong position after a quirky resize. The case in point is maximizing the terminal. For maximizing (height->50) from a buffer that's 30 lines tall, with the cursor on y=30, this is what happens: * With the quirk disabled, conpty reprints the entire buffer. This is 60 lines that get printed. This ends up blowing away about 20 lines of scrollback history, as the terminal app would have tried to keep the text pinned to the bottom of the window. The term. app moved the viewport up 20 lines, and then the 50 lines of conpty output (30 lines of text, and 20 blank lines at the bottom) overwrote the lines from the scrollback. This is bad, but not immediately obvious, and is **what currently happens**. * With the quirk enabled, conpty doesn't emit any lines, but the actual content of the window is still only in the top 30 lines. However, the terminal app has still moved 20 lines down from the scrollback back into the viewport. So the terminal's cursor is at y=50 now, but conpty's is at 30. This means that the terminal and conpty are out of sync, and there's not a good way of re-syncing these. It's very possible (trivial in `powershell`) that the new output will jump up to y=30 override the existing output in the terminal buffer. The Windows Terminal is already prepared for this quirky behavior, so it doesn't keep the output at the bottom of the window. It shifts it's viewport down to match what conpty things the buffer looks like. What happens when we have passthrough mode and WT is like "I would like quirky resize"? I guess things will just work fine, cause there won't be a buffer behind the passthrough app that the terminal cares about. Sure, in the passthrough case the Terminal could _not_ quirky resize, but the quirky resize won't be wrong. |
||
|
Mike Griese
|
e5182fb3e8
|
Make Conpty emit wrapped lines as actually wrapped lines (#4415)
## Summary of the Pull Request Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines. ## References * Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that * Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo * #4200 is the mega bucket with all this work * MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out. * #4403 - I made sure this worked with that PR before I even sent #4403 ## PR Checklist * [x] Closes #405 * [x] Closes #3367 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I started with the following implementation: When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text. However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist. ## Validation Steps Performed * Ran tests * Checked how the Windows Terminal behaves with these changes * Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty |
||
|
Michael Niksa
|
4420950337
|
Restrict DX run height adjustment to only relevant glyph AND Correct PTY rendering on trailing half of fullwidth glyphs (#4668)
## Summary of the Pull Request - Height adjustment of a glyph is now restricted to itself in the DX renderer instead of applying to the entire run - ConPTY compensates for drawing the right half of a fullwidth character. The entire render base has this behavior restored now as well. ## PR Checklist * [x] Closes #2191 * [x] I work here * [x] Tests added/passed * [x] No doc * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two issues: 1. On the DirectX renderer side, when confronted with shrinking a glyph, the correction code would apply the shrunken size to the entire run, not just the potentially individual glyph that needed to be reduced in size. Unfortunately while adjusting the horizontal X width can be done for each glyph in a run, the vertical Y height has to be adjusted for an entire run. So the solution here was to split the individual glyph needing shrinking out of the run into its own run so it can be shrunk. 2. On the ConPTY side, there was a long standing TODO that was never completed to deal with a request to draw only the right half of a two-column character. This meant that when encountering a request for the right half only, we would transmit the entire full character to be drawn, left and right halves, struck over the right half position. Now we correct the cursor back a position (if space) and draw it out so the right half is struck over where we believe the right half should be (and the left half is updated as well as a consequence, which should be OK.) The reason this happens right now is because despite VIM only updating two cells in the buffer, the differential drawing calculation in the ConPTY is very simplistic and intersects only rectangles. This means from the top left most character drawn down to the row/col cursor count indicator in vim's modeline are redrawn with each character typed. This catches the line below the edited line in the typing and refreshes it. But incorrectly. We need to address making ConPTY smarter about what it draws incrementally as it's clearly way too chatty. But I plan to do that with some of the structures I will be creating to solve #778. ## Validation Steps Performed - Ran the scenario listed in #2191 in vim in the Terminal - Added unit tests similar to examples given around glyph/text mapping in runs from Microsoft community page |
||
|
Michael Niksa
|
b3145e4ec8
|
Avoid processing VT Render Trace strings when no one is listening (#4594)
## Summary of the Pull Request - If no one is listening to the ETW provider for the VT Renderer for diagnostic purposes, do not spend time allocating/deleting/formatting strings for presentation in TraceLogging messages. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes something I noticed while working on the renderer. * [x] I work here. * [x] Existing tests should pass * [x] No doc * [x] Am core contributor. ## Validation Steps Performed WPR/WPA Before: 321/3016 samples on hot path (10.64%)  After: 0/1266 samples on the same path (0%)  |
||
|
adiviness
|
9b92986b49
|
add clang-format conf to the project, format the c++ code (#1141) | ||
|
Eric Budai
|
06a5583c86 |
Fix a bunch of static analysis issues (#553)
* static analysis fixes * using C++ style casts * explicit delete changed to reset(nullptr) * fix for null apiMsg.OtherId during tracing in Compare() * changed INVALID_ID macro to constexpr * properly handle null ReplyMsg in ConsoleIoThread() * Fixed wrong static_cast for State.InputBuffer * compensate for null reply message to fix deref problem of ReplyMsg in srvinit.cpp by changing signature in DeviceComm.h |
||
|
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. |
