From 4bcb283274693dab6e089ac99e7002e96250fab3 Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Mon, 23 Sep 2019 07:16:40 -0700 Subject: [PATCH 1/8] Remember last-used string in the Find dialog in conhost (#2845) (cherry picked from commit bfb14847082f9ae085efe1c6b8cba16027815bca) --- src/interactivity/win32/find.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/interactivity/win32/find.cpp b/src/interactivity/win32/find.cpp index 91124cec480..db995dc3683 100644 --- a/src/interactivity/win32/find.cpp +++ b/src/interactivity/win32/find.cpp @@ -23,6 +23,8 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l // This bool is used to track which option - up or down - was used to perform the last search. That way, the next time the // find dialog is opened, it will default to the last used option. static bool fFindSearchUp = true; + static std::wstring lastFindString; + WCHAR szBuf[SEARCH_STRING_LENGTH + 1]; switch (Message) { @@ -30,6 +32,7 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l SetWindowLongPtrW(hWnd, DWLP_USER, lParam); SendDlgItemMessageW(hWnd, ID_CONSOLE_FINDSTR, EM_LIMITTEXT, ARRAYSIZE(szBuf) - 1, 0); CheckRadioButton(hWnd, ID_CONSOLE_FINDUP, ID_CONSOLE_FINDDOWN, (fFindSearchUp ? ID_CONSOLE_FINDUP : ID_CONSOLE_FINDDOWN)); + SetDlgItemText(hWnd, ID_CONSOLE_FINDSTR, lastFindString.c_str()); return TRUE; case WM_COMMAND: { @@ -40,6 +43,7 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l USHORT const StringLength = (USHORT)GetDlgItemTextW(hWnd, ID_CONSOLE_FINDSTR, szBuf, ARRAYSIZE(szBuf)); if (StringLength == 0) { + lastFindString.clear(); break; } bool const IgnoreCase = IsDlgButtonChecked(hWnd, ID_CONSOLE_FINDCASE) == 0; @@ -48,7 +52,7 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l SCREEN_INFORMATION& ScreenInfo = gci.GetActiveOutputBuffer(); std::wstring wstr(szBuf, StringLength); - + lastFindString = wstr; LockConsole(); auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); From 929c9af6ae5e9edbf34d492cce196bd73e4e1019 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 23 Sep 2019 09:39:24 -0700 Subject: [PATCH 2/8] Bugfix: TextBuffer Line Wrapping from VTRenderer (#2797) * Bugfix: line selection copy * Revert clipboard change Change VT renderer to do erase line instead of a ton of erase chars * revert TerminalApi change (cherry picked from commit 8afc5b2f596335b47ecc89172ccd9820ec510579) --- src/renderer/vt/paint.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index da49d1d5246..2523fefaa21 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -475,7 +475,6 @@ using namespace Microsoft::Console::Types; if (useEraseChar) { - RETURN_IF_FAILED(_EraseCharacter(sNumSpaces)); // ECH doesn't actually move the cursor itself. However, we think that // the cursor *should* be at the end of the area we just erased. Stash // that position as our new deferred position. If we don't move the @@ -483,6 +482,15 @@ using namespace Microsoft::Console::Types; // cursor to the deferred position at the end of the frame, or right // before we need to print new text. _deferredCursorPos = { _lastText.X + sNumSpaces, _lastText.Y }; + + if (_deferredCursorPos.X < _lastViewport.RightInclusive()) + { + RETURN_IF_FAILED(_EraseCharacter(sNumSpaces)); + } + else + { + RETURN_IF_FAILED(_EraseLine()); + } } else if (_newBottomLine) { From 5d4e6ced4b91862b1a6982786b9339175b8caf76 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett (MSFT)" Date: Mon, 23 Sep 2019 15:06:47 -0700 Subject: [PATCH 3/8] Add some retry support to Renderer::PaintFrame (#2830) If _PaintFrameForEngine returns E_PENDING, we'll give it another two tries to get itself straight. If it continues to fail, we'll take down the application. We observed that the DX renderer was failing to present the swap chain and failfast'ing when it did so; however, there are some errors from which DXGI guidance suggests we try to recover. We'll now return E_PENDING (and destroy our device resources) when we hit those errors. Fixes #2265. (cherry picked from commit 277acc3383ae3dace82c08ef46549b6eba654a73) --- src/renderer/base/renderer.cpp | 18 +++++++++++++++++- src/renderer/dx/DxRenderer.cpp | 22 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 2dffc9a1550..e434c5d0725 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -10,6 +10,8 @@ using namespace Microsoft::Console::Render; using namespace Microsoft::Console::Types; +static constexpr auto maxRetriesForRenderEngine = 3; + // Routine Description: // - Creates a new renderer controller for a console. // Arguments: @@ -62,7 +64,21 @@ Renderer::~Renderer() for (IRenderEngine* const pEngine : _rgpEngines) { - LOG_IF_FAILED(_PaintFrameForEngine(pEngine)); + auto tries = maxRetriesForRenderEngine; + while (tries > 0) + { + const auto hr = _PaintFrameForEngine(pEngine); + if (E_PENDING == hr) + { + if (--tries == 0) + { + FAIL_FAST_HR_MSG(E_UNEXPECTED, "A rendering engine required too many retries."); + } + continue; + } + LOG_IF_FAILED(hr); + break; + } } return S_OK; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index e8cb5589de6..3d52a4b68d1 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -866,15 +866,31 @@ void DxEngine::_InvalidOr(RECT rc) noexcept // Arguments: // - // Return Value: -// - S_OK or relevant DirectX error +// - S_OK on success, E_PENDING to indicate a retry or a relevant DirectX error [[nodiscard]] HRESULT DxEngine::Present() noexcept { if (_presentReady) { try { - FAIL_FAST_IF_FAILED(_dxgiSwapChain->Present(1, 0)); - /*FAIL_FAST_IF_FAILED(_dxgiSwapChain->Present1(1, 0, &_presentParams));*/ + HRESULT hr = S_OK; + + hr = _dxgiSwapChain->Present(1, 0); + /*hr = _dxgiSwapChain->Present1(1, 0, &_presentParams);*/ + + if (FAILED(hr)) + { + // These two error codes are indicated for destroy-and-recreate + if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET) + { + // We don't need to end painting here, as the renderer has done it for us. + _ReleaseDeviceResources(); + FAIL_FAST_IF_FAILED(InvalidateAll()); + return E_PENDING; // Indicate a retry to the renderer. + } + + FAIL_FAST_HR(hr); + } RETURN_IF_FAILED(_CopyFrontToBack()); _presentReady = false; From 7e1e7192ca0a031af8d9ed0116f454d20734861e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 23 Sep 2019 18:07:47 -0400 Subject: [PATCH 4/8] Enable VT processing by default for ConPTY (#2824) This change enables VT processing by default for _all_ conpty clients. See #1965 for a discussion on why we believe this is a righteous change. Also mentioned in the issue was the idea of only checking the `VirtualTerminalLevel` reg key in the conpty startup. I don't think this would be a more difficult change, looks like all we'd need is a simple `reg.LoadGlobalsFromRegistry();` call instead of this change. # Validation Steps Performed Manually launched a scratch app in both the terminal and the console. The console launch's output mode was 0x3, and the terminal's was 0x7. 0x4 is the ` ENABLE_VIRTUAL_TERMINAL_PROCESSING` flag, which the client now had by default in the Terminal. Closes #1965 (cherry picked from commit 1c412d42b3ac7fbaaf75324936211f4b4ed0e86f) --- src/host/srvinit.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index 63c2bddf9fb..e1f8001187b 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -139,6 +139,17 @@ static bool s_IsOnDesktop() reg.LoadFromRegistry(Title); } } + else + { + // microsoft/terminal#1965 - Let's just always enable VT processing by + // default for conpty clients. This prevents peculiar differences in + // behavior between conhost and terminal applications when the user has + // VirtualTerminalLevel=1 in their registry. + // We want everyone to be using VT by default anyways, so this is a + // strong nudge in that direction. If an application _doesn't_ want VT + // processing, it's free to disable this setting, even in conpty mode. + settings.SetVirtTermLevel(1); + } // 1. The settings we were passed contains STARTUPINFO structure settings to be applied last. settings.ApplyStartupInfo(pStartupSettings); From 04c2d6b035128e8bfe03db805303a4f2dcbacd27 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 24 Sep 2019 00:16:54 +0100 Subject: [PATCH 5/8] Remove unwanted DECSTBM clipping (#2764) The `DECSTBM` margins are meant to define the range of lines within which certain vertical scrolling operations take place. However, we were applying these margin restrictions in the `ScrollRegion` function, which is also used in a number of places that shouldn't be affected by `DECSTBM`. This includes the `ICH` and `DCH` escape sequences (which are only affected by the horizontal margins, which we don't yet support), the `ScrollConsoleScreenBuffer` API (which is public Console API, not meant to be affected by the VT terminal emulation), and the `CSI 3 J` erase scrollback extension (which isn't really scrolling as such, but uses the `ScrollRegion` function to manipulate the scrollback buffer). This commit moves the margin clipping out of the `ScrollRegion` function, so it can be applied exclusively in the places that need it. With the margin clipping removed from the `ScrollRegion` function, it now had to be applied manually in the places it was actually required. This included: * The `DoSrvPrivateReverseLineFeed` function (for the `RI` control): This was * just a matter of updating the bottom of the scroll rect to the bottom margin * (at least when the margins were actually set), since the top of the scroll * rect would always be the top of the viewport. The * `DoSrvPrivateModifyLinesImpl` function (for the `IL` and `DL` commands): * Again this was just a matter of updating the bottom of the scroll rect, since * the cursor position would always determine the top of the scroll rect. The * `AdaptDispatch::_ScrollMovement` method (for the `SU` and `SD` commands): * This required updating both the top and bottom coordinates of the scroll * rect, and also a simpler destination Y coordinate (the way the `ScrollRegion` * function worked before, the caller was expected to take the margins into * account when determining the destination). On the plus side, there was now no longer a need to override the margins when calling `ScrollRegion` in the `AdjustCursorPosition` function. In the first case, the margins had needed to be cleared (_stream.cpp 143-145), but that is now the default behaviour. In the second case, there had been a more complicated adjustment of the margins (_stream.cpp 196-209), but that code was never actually used so could be removed completely (to get to that point either _fScrollUp_ was true, so _scrollDownAtTop_ couldn't also be true, or _fScrollDown_ was true, but in that case there is a check to make sure _scrollDownAtTop_ is false). While testing, I also noticed that one of the `ScrollRegion` calls in the `AdjustCursorPosition` function was not setting the horizontal range correctly - the scrolling should always affect the full buffer width rather than just the viewport width - so I've fixed that now as well. ## Validation Steps Performed For commands like `RI`, `IL`, `DL`, etc. where we've changed the implementation but not the behaviour, there were already unit tests that could confirm that the new implementation was still producing the correct results. Where there has been a change in behaviour - namely for the `ICH` and `DCH` commands, and the `ScrollConsoleScreenBuffer` API - I've extended the existing unit tests to check that they still function correctly even when the `DECSTBM` margins are set (which would previously have caused them to fail). I've also tested manually with the test cases in issues #2543 and #2659, and confirmed that they now work as expected. Closes #2543 Closes #2659 (cherry picked from commit 0c8a4df9636982fc92137c47b31877f2c7b5efdb) --- src/host/_stream.cpp | 30 +++----------------------- src/host/getset.cpp | 10 +++++++++ src/host/output.cpp | 25 --------------------- src/host/ut_host/ApiRoutinesTests.cpp | 10 +++++++++ src/host/ut_host/ScreenBufferTests.cpp | 26 ++++++++++++++++++++++ src/terminal/adapter/adaptDispatch.cpp | 10 ++++++--- 6 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 3e99d36ec55..94d4f7c74c3 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -140,15 +140,11 @@ using Microsoft::Console::VirtualTerminal::StateMachine; const COORD newPostMarginsOrigin = { 0, moveToYPosition }; const COORD newViewOrigin = { 0, newViewTop }; - // Unset the margins to scroll the content below the margins, - // then restore them after. - screenInfo.SetScrollMargins(Viewport::FromInclusive({ 0 })); try { ScrollRegion(screenInfo, scrollRect, std::nullopt, newPostMarginsOrigin, UNICODE_SPACE, bufferAttributes); } CATCH_LOG(); - screenInfo.SetScrollMargins(relativeMargins); // Move the viewport down auto hr = screenInfo.SetViewportOrigin(true, newViewOrigin, true); @@ -186,39 +182,19 @@ using Microsoft::Console::VirtualTerminal::StateMachine; SMALL_RECT scrollRect = { 0 }; scrollRect.Top = srMargins.Top; scrollRect.Bottom = srMargins.Bottom; - scrollRect.Left = screenInfo.GetViewport().Left(); // NOTE: Left/Right Scroll margins don't do anything currently. - scrollRect.Right = screenInfo.GetViewport().RightInclusive(); + scrollRect.Left = 0; // NOTE: Left/Right Scroll margins don't do anything currently. + scrollRect.Right = bufferSize.X - 1; // -1, otherwise this would be an exclusive rect. COORD dest; dest.X = scrollRect.Left; dest.Y = scrollRect.Top - diff; - SMALL_RECT clipRect = scrollRect; - // Typically ScrollRegion() clips by the scroll margins. However, if - // we're scrolling down at the top of the viewport, we'll need to - // not clip at the margins, instead move the contents of the margins - // up above the viewport. So we'll clear out the current margins, and - // set them to the viewport+(#diff rows above the viewport). - if (scrollDownAtTop) - { - clipRect.Top -= diff; - auto fakeMargins = srMargins; - fakeMargins.Top -= diff; - auto fakeRelative = viewport.ConvertToOrigin(Viewport::FromInclusive(fakeMargins)); - screenInfo.SetScrollMargins(fakeRelative); - } - try { - ScrollRegion(screenInfo, scrollRect, clipRect, dest, UNICODE_SPACE, bufferAttributes); + ScrollRegion(screenInfo, scrollRect, scrollRect, dest, UNICODE_SPACE, bufferAttributes); } CATCH_LOG(); - if (scrollDownAtTop) - { - // Undo the fake margins we set above - screenInfo.SetScrollMargins(relativeMargins); - } coordCursor.Y -= diff; } diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 7bf5c74d97e..16aa6633a71 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1368,6 +1368,11 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool srScroll.Right = SHORT_MAX; srScroll.Top = viewport.Top; srScroll.Bottom = viewport.Bottom; + // Clip to the DECSTBM margin boundary + if (screenInfo.AreMarginsSet()) + { + srScroll.Bottom = screenInfo.GetAbsoluteScrollMargins().BottomInclusive(); + } // Paste coordinate for cut text above COORD coordDestination; coordDestination.X = 0; @@ -2044,6 +2049,11 @@ void DoSrvPrivateModifyLinesImpl(const unsigned int count, const bool insert) srScroll.Right = SHORT_MAX; srScroll.Top = cursorPosition.Y; srScroll.Bottom = screenInfo.GetViewport().BottomInclusive(); + // Clip to the DECSTBM margin boundary + if (screenInfo.AreMarginsSet()) + { + srScroll.Bottom = screenInfo.GetAbsoluteScrollMargins().BottomInclusive(); + } // Paste coordinate for cut text above COORD coordDestination; coordDestination.X = 0; diff --git a/src/host/output.cpp b/src/host/output.cpp index 8147de84535..8b4e0bbbbda 100644 --- a/src/host/output.cpp +++ b/src/host/output.cpp @@ -361,19 +361,6 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo, // If there was no clip rect, we'll clip to the entire buffer size. auto clip = Viewport::FromInclusive(clipRectGiven.value_or(buffer.ToInclusive())); - // Account for the scroll margins set by DECSTBM - // DECSTBM command can sometimes apply a clipping behavior as well. Check if we have any - // margins defined by DECSTBM and further restrict the clipping area here. - if (screenInfo.AreMarginsSet()) - { - const auto margin = screenInfo.GetScrollingRegion(); - - // Update the clip rectangle to only include the area that is also in the margin. - clip = Viewport::Intersect(clip, margin); - - // We'll also need to update the source rectangle, but we need to do that later. - } - // OK, make sure that the clip rectangle also fits inside the buffer clip = Viewport::Intersect(buffer, clip); @@ -416,18 +403,6 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo, targetOrigin.Y += currentSourceOrigin.Y - originalSourceOrigin.Y; } - // See MSFT:20204600 - Update the source rectangle to only include the region - // inside the scroll margins. We need to do this AFTER we calculate the - // delta between the currentSourceOrigin and the originalSourceOrigin. - // Don't combine this with the above block, because if there are margins set - // and the source rectangle was clipped by the buffer, we still want to - // adjust the target origin point based on the clipping of the buffer. - if (screenInfo.AreMarginsSet()) - { - const auto margin = screenInfo.GetScrollingRegion(); - source = Viewport::Intersect(source, margin); - } - // And now the target viewport is the same size as the source viewport but at the different position. auto target = Viewport::FromDimensions(targetOrigin, source.Dimensions()); diff --git a/src/host/ut_host/ApiRoutinesTests.cpp b/src/host/ut_host/ApiRoutinesTests.cpp index a8e425882b9..eb5d6c0a29e 100644 --- a/src/host/ut_host/ApiRoutinesTests.cpp +++ b/src/host/ut_host/ApiRoutinesTests.cpp @@ -594,9 +594,12 @@ class ApiRoutinesTests TEST_METHOD(ApiScrollConsoleScreenBufferW) { BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"data:setMargins", L"{false, true}") TEST_METHOD_PROPERTY(L"data:checkClipped", L"{false, true}") END_TEST_METHOD_PROPERTIES(); + bool setMargins; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setMargins", setMargins), L"Get whether or not we should set the DECSTBM margins."); bool checkClipped; VERIFY_SUCCEEDED(TestData::TryGetValue(L"checkClipped", checkClipped), L"Get whether or not we should check all the options using a clipping rectangle."); @@ -605,6 +608,13 @@ class ApiRoutinesTests VERIFY_SUCCEEDED(si.GetTextBuffer().ResizeTraditional({ 5, 5 }), L"Make the buffer small so this doesn't take forever."); + // Tests are run both with and without the DECSTBM margins set. This should not alter + // the results, since ScrollConsoleScreenBuffer should not be affected by VT margins. + auto& stateMachine = si.GetStateMachine(); + stateMachine.ProcessString(setMargins ? L"\x1b[2;4r" : L"\x1b[r"); + // Make sure we clear the margins on exit so they can't break other tests. + auto clearMargins = wil::scope_exit([&] { stateMachine.ProcessString(L"\x1b[r"); }); + gci.LockConsole(); auto Unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 71d3aa7ea06..aa9195ce322 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -3282,6 +3282,13 @@ void ScreenBufferTests::ScrollOperations() void ScreenBufferTests::InsertChars() { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:setMargins", L"{false, true}") + END_TEST_METHOD_PROPERTIES(); + + bool setMargins; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setMargins", setMargins)); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer(); auto& stateMachine = si.GetStateMachine(); @@ -3295,6 +3302,12 @@ void ScreenBufferTests::InsertChars() VERIFY_SUCCEEDED(si.ResizeScreenBuffer({ bufferWidth, bufferHeight }, false)); si.SetViewport(Viewport::FromExclusive({ viewportStart, 0, viewportEnd, 25 }), true); + // Tests are run both with and without the DECSTBM margins set. This should not alter + // the results, since the ICH operation is not affected by vertical margins. + stateMachine.ProcessString(setMargins ? L"\x1b[15;20r" : L"\x1b[r"); + // Make sure we clear the margins on exit so they can't break other tests. + auto clearMargins = wil::scope_exit([&] { stateMachine.ProcessString(L"\x1b[r"); }); + Log::Comment( L"Test 1: Fill the line with Qs. Write some text within the viewport boundaries. " L"Then insert 5 spaces at the cursor. Watch spaces get inserted, text slides right " @@ -3425,6 +3438,13 @@ void ScreenBufferTests::InsertChars() void ScreenBufferTests::DeleteChars() { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:setMargins", L"{false, true}") + END_TEST_METHOD_PROPERTIES(); + + bool setMargins; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setMargins", setMargins)); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer(); auto& stateMachine = si.GetStateMachine(); @@ -3438,6 +3458,12 @@ void ScreenBufferTests::DeleteChars() VERIFY_SUCCEEDED(si.ResizeScreenBuffer({ bufferWidth, bufferHeight }, false)); si.SetViewport(Viewport::FromExclusive({ viewportStart, 0, viewportEnd, 25 }), true); + // Tests are run both with and without the DECSTBM margins set. This should not alter + // the results, since the DCH operation is not affected by vertical margins. + stateMachine.ProcessString(setMargins ? L"\x1b[15;20r" : L"\x1b[r"); + // Make sure we clear the margins on exit so they can't break other tests. + auto clearMargins = wil::scope_exit([&] { stateMachine.ProcessString(L"\x1b[r"); }); + Log::Comment( L"Test 1: Fill the line with Qs. Write some text within the viewport boundaries. " L"Then delete 5 characters at the cursor. Watch the rest of the line slide left, " diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 1e6089ed22d..7123e695087 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -963,13 +963,17 @@ bool AdaptDispatch::_ScrollMovement(const ScrollDirection sdDirection, _In_ unsi srScreen.Right = SHORT_MAX; srScreen.Top = csbiex.srWindow.Top; srScreen.Bottom = csbiex.srWindow.Bottom - 1; // srWindow is exclusive, hence the - 1 + // Clip to the DECSTBM margin boundaries + if (_srScrollMargins.Top < _srScrollMargins.Bottom) + { + srScreen.Top = csbiex.srWindow.Top + _srScrollMargins.Top; + srScreen.Bottom = csbiex.srWindow.Top + _srScrollMargins.Bottom; + } // Paste coordinate for cut text above COORD coordDestination; coordDestination.X = srScreen.Left; - // Scroll starting from the top of the scroll margins. - coordDestination.Y = (_srScrollMargins.Top + srScreen.Top) + sDistance * (sdDirection == ScrollDirection::Up ? -1 : 1); - // We don't need to worry about clipping the margins at all, ScrollRegion inside conhost will do that correctly for us + coordDestination.Y = srScreen.Top + sDistance * (sdDirection == ScrollDirection::Up ? -1 : 1); // Fill character for remaining space left behind by "cut" operation (or for fill if we "cut" the entire line) CHAR_INFO ciFill; From fc0a099800a7ed059c4633273d2a958abb277623 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett (MSFT)" Date: Mon, 23 Sep 2019 18:35:53 -0700 Subject: [PATCH 6/8] conhost: if we start with invalid terminal colors, reset them to sanity (#2855) * conhost: if we start with invalid terminal colors, reset them to sanity We've seen a number of cases where the user's settings can get corrupted and their default foreground/background and cursor color get set to all black (black on black). This results in a fairly unhappy user and probably a great number of support incidents. Let's declare that an invalid state. * Add some comments to the comments (cherry picked from commit a1bd7967e22a64625a52e4df06bad0403698cf01) --- src/host/settings.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 27b41905998..e4fd970a8e0 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -326,6 +326,24 @@ void Settings::Validate() WI_ClearAllFlags(_wFillAttribute, ~(FG_ATTRS | BG_ATTRS)); WI_ClearAllFlags(_wPopupFillAttribute, ~(FG_ATTRS | BG_ATTRS)); + // If the extended color options are set to invalid values (all the same color), reset them. + if (_CursorColor != Cursor::s_InvertCursorColor && _CursorColor == _DefaultBackground) + { + _CursorColor = Cursor::s_InvertCursorColor; + } + + if (_DefaultForeground != INVALID_COLOR && _DefaultForeground == _DefaultBackground) + { + // INVALID_COLOR is used as an "unset" sentinel in future attribute functions. + _DefaultForeground = _DefaultBackground = INVALID_COLOR; + // If the damaged settings _further_ propagated to the default fill attribute, fix it. + if (_wFillAttribute == 0) + { + // These attributes were taken from the Settings ctor and equal "gray on black" + _wFillAttribute = FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE; + } + } + FAIL_FAST_IF(!(_dwWindowSize.X > 0)); FAIL_FAST_IF(!(_dwWindowSize.Y > 0)); FAIL_FAST_IF(!(_dwScreenBufferSize.X > 0)); From f53143488649643eb3af7ea40aea081b88c1b0b0 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 24 Sep 2019 13:50:53 -0700 Subject: [PATCH 7/8] Improve debugging experience of key events (#2872) ... by adding natvis rules for display and by typifying the flags field so the debugger presents it as flags naturally. (cherry picked from commit 60c6a9fb8fbd40ecb792ad16e79b84ed53a2917e) --- src/types/IInputEventStreams.cpp | 2 +- src/types/KeyEvent.cpp | 14 +++++++---- src/types/inc/IInputEvent.hpp | 40 +++++++++++++++++++++++++------- tools/ConsoleTypes.natvis | 5 ++++ 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/types/IInputEventStreams.cpp b/src/types/IInputEventStreams.cpp index 55c2d8ec2fd..6e4e9ff3317 100644 --- a/src/types/IInputEventStreams.cpp +++ b/src/types/IInputEventStreams.cpp @@ -57,7 +57,7 @@ std::wostream& operator<<(std::wostream& stream, const KeyEvent* const pKeyEvent L"keyCode: " << pKeyEvent->_virtualKeyCode << L", " << L"scanCode: " << pKeyEvent->_virtualScanCode << L", " << L"char: " << charData << L", " << - L"mods: " << pKeyEvent->_activeModifierKeys << L")"; + L"mods: " << pKeyEvent->GetActiveModifierKeys() << L")"; // clang-format on } diff --git a/src/types/KeyEvent.cpp b/src/types/KeyEvent.cpp index 869d302d5a4..ab891b814cf 100644 --- a/src/types/KeyEvent.cpp +++ b/src/types/KeyEvent.cpp @@ -17,7 +17,7 @@ INPUT_RECORD KeyEvent::ToInputRecord() const noexcept record.Event.KeyEvent.wVirtualKeyCode = _virtualKeyCode; record.Event.KeyEvent.wVirtualScanCode = _virtualScanCode; record.Event.KeyEvent.uChar.UnicodeChar = _charData; - record.Event.KeyEvent.dwControlKeyState = _activeModifierKeys; + record.Event.KeyEvent.dwControlKeyState = GetActiveModifierKeys(); return record; } @@ -53,19 +53,23 @@ void KeyEvent::SetCharData(const wchar_t character) noexcept void KeyEvent::SetActiveModifierKeys(const DWORD activeModifierKeys) noexcept { - _activeModifierKeys = activeModifierKeys; + _activeModifierKeys = static_cast(activeModifierKeys); } void KeyEvent::DeactivateModifierKey(const ModifierKeyState modifierKey) noexcept { DWORD const bitFlag = ToConsoleControlKeyFlag(modifierKey); - WI_ClearAllFlags(_activeModifierKeys, bitFlag); + auto keys = GetActiveModifierKeys(); + WI_ClearAllFlags(keys, bitFlag); + SetActiveModifierKeys(keys); } void KeyEvent::ActivateModifierKey(const ModifierKeyState modifierKey) noexcept { DWORD const bitFlag = ToConsoleControlKeyFlag(modifierKey); - WI_SetAllFlags(_activeModifierKeys, bitFlag); + auto keys = GetActiveModifierKeys(); + WI_SetAllFlags(keys, bitFlag); + SetActiveModifierKeys(keys); } bool KeyEvent::DoActiveModifierKeysMatch(const std::unordered_set& consoleModifiers) const @@ -75,7 +79,7 @@ bool KeyEvent::DoActiveModifierKeysMatch(const std::unordered_set(_activeModifierKeys); } void SetKeyDown(const bool keyDown) noexcept; @@ -255,7 +277,7 @@ class KeyEvent : public IInputEvent WORD _virtualKeyCode; WORD _virtualScanCode; wchar_t _charData; - DWORD _activeModifierKeys; + Modifiers _activeModifierKeys; friend constexpr bool operator==(const KeyEvent& a, const KeyEvent& b) noexcept; #ifdef UNIT_TESTING diff --git a/tools/ConsoleTypes.natvis b/tools/ConsoleTypes.natvis index d2d6183034d..dab839ec6b1 100644 --- a/tools/ConsoleTypes.natvis +++ b/tools/ConsoleTypes.natvis @@ -72,4 +72,9 @@ _Mypair._Myval2 + + + {{↓ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} + {{↑ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} + From 2e17593b1fdbec053de7976d3e11b8f206457cb3 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Thu, 26 Sep 2019 06:09:56 +0800 Subject: [PATCH 8/8] Retarget VtPipeTerm & terminalcore-lib to 18362 (#2885) (cherry picked from commit 1386148191f463dfde656f885ddf44c43cc1cae8) --- src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj | 2 +- src/tools/vtpipeterm/VtPipeTerm.vcxproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj b/src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj index 161c77f6939..f6085e342fb 100644 --- a/src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj +++ b/src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj @@ -13,7 +13,7 @@ TerminalCore TerminalCore 10.0.17763.0 - 10.0.17763.0 + 10.0.18362.0 Microsoft.Terminal.Core diff --git a/src/tools/vtpipeterm/VtPipeTerm.vcxproj b/src/tools/vtpipeterm/VtPipeTerm.vcxproj index 347a996b137..4581bf226c1 100644 --- a/src/tools/vtpipeterm/VtPipeTerm.vcxproj +++ b/src/tools/vtpipeterm/VtPipeTerm.vcxproj @@ -14,7 +14,7 @@ VtPipeTerm VtPipeTerm VtPipeTerm - 10.0.17763.0 + 10.0.18362.0