Merged PR 9838597: Migrate OSS up to db27348

- AtlasEngine: Minor bug fixes (GH-16219)
- Fix the fix for the fix of nearby font loading (GH-16196)
- Added selectionBackground to light color schemes (GH-16243)
- Another theoretical fix for a crash (GH-16267)
- Fix tabs being printed in cmd.exe prompts (GH-16273)

Related work items: MSFT-47266988

doc/COOKED_READ_DATA.md

@@ -0,0 +1,90 @@
+# COOKED_READ_DATA, aka conhost's readline implementation
+
+## Test instructions
+
+All of the following ✅ marks must be fulfilled during manual testing:
+* ASCII input
+* Chinese input (中文維基百科) ❔
+  * Resizing the window properly wraps/unwraps wide glyphs ❌
+    Broken due to `TextBuffer::Reflow` bugs
+* Surrogate pair input (🙂) ❔
+  * Resizing the window properly wraps/unwraps surrogate pairs ❌
+    Broken due to `TextBuffer::Reflow` bugs
+* In cmd.exe
+  * Create 2 file: "a😊b.txt" and "a😟b.txt"
+  * Press tab: Autocomplete to "a😊b.txt" ✅
+  * Navigate the cursor right past the "a"
+  * Press tab twice: Autocomplete to "a😟b.txt" ✅
+* Backspace deletes preceding glyphs ✅
+* Ctrl+Backspace deletes preceding words ✅
+* Escape clears input ✅
+* Home navigates to start ✅
+* Ctrl+Home deletes text between cursor and start ✅
+* End navigates to end ✅
+* Ctrl+End deletes text between cursor and end ✅
+* Left navigates over previous code points ✅
+* Ctrl+Left navigates to previous word-starts ✅
+* Right and F1 navigate over next code points ✅
+  * Pressing right at the end of input copies characters
+    from the previous command ✅
+* Ctrl+Right navigates to next word-ends ✅
+* Insert toggles overwrite mode ✅
+* Delete deletes next code point ✅
+* Up and F5 cycle through history ✅
+  * Doesn't crash with no history ✅
+  * Stops at first entry ✅
+* Down cycles through history ✅
+  * Doesn't crash with no history ✅
+  * Stops at last entry ✅
+* PageUp retrieves the oldest command ✅
+* PageDown retrieves the newest command ✅
+* F2 starts "copy to char" prompt ✅
+  * Escape dismisses prompt ✅
+  * Typing a character copies text from the previous command up
+    until that character into the current buffer (acts identical
+    to F3, but with automatic character search) ✅
+* F3 copies the previous command into the current buffer,
+  starting at the current cursor position,
+  for as many characters as possible ✅
+  * Doesn't erase trailing text if the current buffer
+    is longer than the previous command ✅
+  * Puts the cursor at the end of the copied text ✅
+* F4 starts "copy from char" prompt ✅
+  * Escape dismisses prompt ✅
+  * Erases text between the current cursor position and the
+    first instance of a given char (but not including it) ✅
+* F6 inserts Ctrl+Z ✅
+* F7 without modifiers starts "command list" prompt ✅
+  * Escape dismisses prompt ✅
+  * Minimum size of 40x10 characters ✅
+  * Width expands to fit the widest history command ✅
+  * Height expands up to 20 rows with longer histories ✅
+  * F9 starts "command number" prompt ✅
+  * Left/Right paste replace the buffer with the given command ✅
+    * And put cursor at the end of the buffer ✅
+  * Up/Down navigate selection through history ✅
+    * Stops at start/end with <10 entries ✅
+    * Stops at start/end with >20 entries ✅
+    * Wide text rendering during pagination with >20 entries ✅
+  * Shift+Up/Down moves history items around ✅
+  * Home navigates to first entry ✅
+  * End navigates to last entry ✅
+  * PageUp navigates by 20 items at a time or to first ✅
+  * PageDown navigates by 20 items at a time or to last ✅
+* Alt+F7 clears command history ✅
+* F8 cycles through commands that start with the same text as
+  the current buffer up until the current cursor position ✅
+  * Doesn't crash with no history ✅
+* F9 starts "command number" prompt ✅
+  * Escape dismisses prompt ✅
+  * Ignores non-ASCII-decimal characters ✅
+  * Allows entering between 1 and 5 digits ✅
+  * Pressing Enter fetches the given command from the history ✅
+* Alt+F10 clears doskey aliases ✅
+* In cmd.exe, with an empty prompt in an empty directory:
+  Pressing tab produces an audible bing and prints no text ✅
+* When Narrator is enabled, in cmd.exe:
+  * Typing individual characters announces only
+    exactly each character that is being typed ✅
+  * Backspacing at the end of a prompt announces
+    only exactly each deleted character ✅

src/cascadia/TerminalSettingsModel/defaults.json

@@ -174,6 +174,7 @@
             "foreground": "#383A42",
             "background": "#FAFAFA",
             "cursorColor": "#4F525D",
+            "selectionBackground": "#4F525D",
             "black": "#383A42",
             "red": "#E45649",
             "green": "#50A14F",
@@ -218,6 +219,7 @@
             "foreground": "#657B83",
             "background": "#FDF6E3",
             "cursorColor": "#002B36",
+            "selectionBackground": "#073642",
             "black": "#002B36",
             "red": "#DC322F",
             "green": "#859900",
@@ -262,6 +264,7 @@
             "foreground": "#555753",
             "background": "#FFFFFF",
             "cursorColor": "#000000",
+            "selectionBackground": "#555753",
             "black": "#000000",
             "red": "#CC0000",
             "green": "#4E9A06",

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -979,7 +979,8 @@ winrt::Windows::Foundation::IAsyncOperation<winrt::hstring> AppHost::_GetWindowL
     co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher());
 
     const auto strongThis = weakThis.lock();
-    if (!strongThis)
+    // GH #16235: If we don't have a window logic, we're already refrigerating, and won't have our _window either.
+    if (!strongThis || _windowLogic == nullptr)
     {
         co_return layoutJson;
     }
@@ -1267,7 +1268,8 @@ winrt::fire_and_forget AppHost::_QuitRequested(const winrt::Windows::Foundation:
     co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher());
 
     const auto strongThis = weakThis.lock();
-    if (!strongThis)
+    // GH #16235: If we don't have a window logic, we're already refrigerating, and won't have our _window either.
+    if (!strongThis || _windowLogic == nullptr)
     {
         co_return;
     }
@@ -1431,7 +1433,7 @@ winrt::fire_and_forget AppHost::_WindowInitializedHandler(const winrt::Windows::
 
     // If we're gone on the other side of this co_await, well, that's fine. Just bail.
     const auto strongThis = weakThis.lock();
-    if (!strongThis)
+    if (!strongThis || _window == nullptr)
     {
         co_return;
     }

src/host/readDataCooked.cpp

@@ -436,14 +436,17 @@ void COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers)
 
     if (_ctrlWakeupMask != 0 && wch < L' ' && (_ctrlWakeupMask & (1 << wch)))
     {
-        _flushBuffer();
-
         // The old implementation (all the way since the 90s) overwrote the character at the current cursor position with the given wch.
         // But simultaneously it incremented the buffer length, which would have only worked if it was written at the end of the buffer.
         // Press tab past the "f" in the string "foo" and you'd get "f\to " (a trailing whitespace; the initial contents of the buffer back then).
         // It's unclear whether the original intention was to write at the end of the buffer at all times or to implement an insert mode.
         // I went with insert mode.
+        //
+        // It is important that we don't actually print that character out though, as it's only for the calling application to see.
+        // That's why we flush the contents before the insertion and then ensure that the _flushBuffer() call in Read() exits early.
+        _flushBuffer();
         _buffer.Replace(_buffer.GetCursorPosition(), 0, &wch, 1);
+        _buffer.MarkAsClean();
 
         _controlKeyState = modifiers;
         _transitionState(State::DoneWithWakeupMask);

src/renderer/atlas/AtlasEngine.api.cpp

@@ -455,30 +455,34 @@ void AtlasEngine::SetWarningCallback(std::function<void(HRESULT)> pfn) noexcept
 
 [[nodiscard]] HRESULT AtlasEngine::UpdateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map<std::wstring_view, uint32_t>& features, const std::unordered_map<std::wstring_view, float>& axes) noexcept
 {
-    static constexpr std::array fallbackFaceNames{ static_cast<const wchar_t*>(nullptr), L"Consolas", L"Lucida Console", L"Courier New" };
-    auto it = fallbackFaceNames.begin();
-    const auto end = fallbackFaceNames.end();
+    try
+    {
+        _updateFont(fontInfoDesired.GetFaceName().c_str(), fontInfoDesired, fontInfo, features, axes);
+        return S_OK;
+    }
+    CATCH_LOG();
 
-    for (;;)
+    if constexpr (Feature_NearbyFontLoading::IsEnabled())
     {
         try
         {
-            _updateFont(*it, fontInfoDesired, fontInfo, features, axes);
+            // _resolveFontMetrics() checks `_api.s->font->fontCollection` for a pre-existing font collection,
+            // before falling back to using the system font collection. This way we can inject our custom one. See GH#9375.
+            // Doing it this way is a bit hacky, but it does have the benefit that we can cache a font collection
+            // instance across font changes, like when zooming the font size rapidly using the scroll wheel.
+            _api.s.write()->font.write()->fontCollection = FontCache::GetCached();
+            _updateFont(fontInfoDesired.GetFaceName().c_str(), fontInfoDesired, fontInfo, features, axes);
             return S_OK;
         }
-        catch (...)
-        {
-            ++it;
-            if (it == end)
-            {
-                RETURN_CAUGHT_EXCEPTION();
-            }
-            else
-            {
-                LOG_CAUGHT_EXCEPTION();
-            }
-        }
+        CATCH_LOG();
     }
+
+    try
+    {
+        _updateFont(nullptr, fontInfoDesired, fontInfo, features, axes);
+        return S_OK;
+    }
+    CATCH_RETURN();
 }
 
 void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept
@@ -490,20 +494,20 @@ void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept
 
 void AtlasEngine::_resolveTransparencySettings() noexcept
 {
+    // An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain().
+    // We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs.
+    const bool useAlpha = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty();
     // If the user asks for ClearType, but also for a transparent background
     // (which our ClearType shader doesn't simultaneously support)
     // then we need to sneakily force the renderer to grayscale AA.
-    const auto antialiasingMode = _api.enableTransparentBackground && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode;
-    const bool enableTransparentBackground = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty() || _api.s->misc->useRetroTerminalEffect;
+    const auto antialiasingMode = useAlpha && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode;
 
-    if (antialiasingMode != _api.s->font->antialiasingMode || enableTransparentBackground != _api.s->target->enableTransparentBackground)
+    if (antialiasingMode != _api.s->font->antialiasingMode || useAlpha != _api.s->target->useAlpha)
     {
         const auto s = _api.s.write();
         s->font.write()->antialiasingMode = antialiasingMode;
-        // An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain().
-        // We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs.
-        s->target.write()->enableTransparentBackground = enableTransparentBackground;
-        _api.backgroundOpaqueMixin = enableTransparentBackground ? 0x00000000 : 0xff000000;
+        s->target.write()->useAlpha = useAlpha;
+        _api.backgroundOpaqueMixin = useAlpha ? 0x00000000 : 0xff000000;
     }
 }
 
@@ -598,11 +602,7 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo
 
     if (!requestedFaceName)
     {
-        requestedFaceName = fontInfoDesired.GetFaceName().c_str();
-        if (!requestedFaceName)
-        {
-            requestedFaceName = L"Consolas";
-        }
+        requestedFaceName = L"Consolas";
     }
     if (!requestedSize.height)
     {
@@ -614,22 +614,19 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo
         requestedWeight = DWRITE_FONT_WEIGHT_NORMAL;
     }
 
-    wil::com_ptr<IDWriteFontCollection> fontCollection;
-    THROW_IF_FAILED(_p.dwriteFactory->GetSystemFontCollection(fontCollection.addressof(), FALSE));
+    // UpdateFont() (and its NearbyFontLoading feature path specifically) sets `_api.s->font->fontCollection`
+    // to a custom font collection that includes .ttf files that are bundled with our app package. See GH#9375.
+    // Doing it this way is a bit hacky, but it does have the benefit that we can cache a font collection
+    // instance across font changes, like when zooming the font size rapidly using the scroll wheel.
+    auto fontCollection = _api.s->font->fontCollection;
+    if (!fontCollection)
+    {
+        THROW_IF_FAILED(_p.dwriteFactory->GetSystemFontCollection(fontCollection.addressof(), FALSE));
+    }
 
     u32 index = 0;
     BOOL exists = false;
     THROW_IF_FAILED(fontCollection->FindFamilyName(requestedFaceName, &index, &exists));
-
-    if constexpr (Feature_NearbyFontLoading::IsEnabled())
-    {
-        if (!exists)
-        {
-            fontCollection = FontCache::GetCached();
-            THROW_IF_FAILED(fontCollection->FindFamilyName(requestedFaceName, &index, &exists));
-        }
-    }
-
     THROW_HR_IF(DWRITE_E_NOFONT, !exists);
 
     wil::com_ptr<IDWriteFontFamily> fontFamily;

src/renderer/atlas/AtlasEngine.r.cpp

@@ -329,7 +329,7 @@ void AtlasEngine::_createSwapChain()
         .SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL,
         // If our background is opaque we can enable "independent" flips by setting DXGI_ALPHA_MODE_IGNORE.
         // As our swap chain won't have to compose with DWM anymore it reduces the display latency dramatically.
-        .AlphaMode = _p.s->target->enableTransparentBackground ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
+        .AlphaMode = _p.s->target->useAlpha ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
         .Flags = swapChainFlags,
     };
 
@@ -360,6 +360,8 @@ void AtlasEngine::_createSwapChain()
     _p.swapChain.targetSize = _p.s->targetSize;
     _p.swapChain.waitForPresentation = true;
 
+    LOG_IF_FAILED(_p.swapChain.swapChain->SetMaximumFrameLatency(1));
+
     WaitUntilCanRender();
 
     if (_p.swapChainChangedCallback)

src/renderer/atlas/BackendD3D.cpp

@@ -403,30 +403,36 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
             /* ppCode      */ blob.addressof(),
             /* ppErrorMsgs */ error.addressof());
 
-        // Unless we can determine otherwise, assume this shader requires evaluation every frame
-        _requiresContinuousRedraw = true;
-
         if (SUCCEEDED(hr))
         {
-            THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.put()));
+            THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.addressof()));
 
             // Try to determine whether the shader uses the Time variable
             wil::com_ptr<ID3D11ShaderReflection> reflector;
-            if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.put()))))
+            if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.addressof()))))
             {
+                // Depending on the version of the d3dcompiler_*.dll, the next two functions either return nullptr
+                // on failure or an instance of CInvalidSRConstantBuffer or CInvalidSRVariable respectively,
+                // which cause GetDesc() to return E_FAIL. In other words, we have to assume that any failure in the
+                // next few lines indicates that the cbuffer is entirely unused (--> _requiresContinuousRedraw=false).
                 if (ID3D11ShaderReflectionConstantBuffer* constantBufferReflector = reflector->GetConstantBufferByIndex(0)) // shader buffer
                 {
                     if (ID3D11ShaderReflectionVariable* variableReflector = constantBufferReflector->GetVariableByIndex(0)) // time
                     {
                         D3D11_SHADER_VARIABLE_DESC variableDescriptor;
-                        if (SUCCEEDED_LOG(variableReflector->GetDesc(&variableDescriptor)))
+                        if (SUCCEEDED(variableReflector->GetDesc(&variableDescriptor)))
                         {
                             // only if time is used
                             _requiresContinuousRedraw = WI_IsFlagSet(variableDescriptor.uFlags, D3D_SVF_USED);
                         }
                     }
                 }
             }
+            else
+            {
+                // Unless we can determine otherwise, assume this shader requires evaluation every frame
+                _requiresContinuousRedraw = true;
+            }
         }
         else
         {
@@ -447,8 +453,6 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
     else if (p.s->misc->useRetroTerminalEffect)
     {
         THROW_IF_FAILED(p.device->CreatePixelShader(&custom_shader_ps[0], sizeof(custom_shader_ps), nullptr, _customPixelShader.put()));
-        // We know the built-in retro shader doesn't require continuous redraw.
-        _requiresContinuousRedraw = false;
     }
 
     if (_customPixelShader)

src/renderer/atlas/common.h

@@ -313,7 +313,7 @@ namespace Microsoft::Console::Render::Atlas
     struct TargetSettings
     {
         HWND hwnd = nullptr;
-        bool enableTransparentBackground = false;
+        bool useAlpha = false;
         bool useSoftwareRendering = false;
     };
 
@@ -470,7 +470,6 @@ namespace Microsoft::Console::Render::Atlas
         wil::com_ptr<IDWriteFontFallback> systemFontFallback;
         wil::com_ptr<IDWriteFontFallback1> systemFontFallback1; // optional, might be nullptr
         wil::com_ptr<IDWriteTextAnalyzer1> textAnalyzer;
-        wil::com_ptr<IDWriteRenderingParams1> renderingParams;
         std::function<void(HRESULT)> warningCallback;
         std::function<void(HANDLE)> swapChainChangedCallback;