Merge remote-tracking branch 'origin/main' into dev/lhecker/9617-term…

…inal-race-conditions-fixup

.github/actions/spelling/allow/apis.txt

@@ -210,6 +210,7 @@ tlg
 TME
 tmp
 tmpdir
+tokeninfo
 tolower
 toupper
 TRACKMOUSEEVENT

oss/libpopcnt/libpopcnt.h

@@ -95,7 +95,7 @@
   #define HAVE_AVX512
 #endif
 
-#if defined(X86_OR_X64)
+#if defined(X86_OR_X64) && !defined(_M_ARM64EC)
   /* MSVC compatible compilers (Windows) */
   #if defined(_MSC_VER)
     /* clang-cl (LLVM 10 from 2020) requires /arch:AVX2 or

src/cascadia/TerminalControl/TermControl.cpp

@@ -2908,7 +2908,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     // However, it's likely that the control layer may need to
                     // know about the source anyways in the future, to support
                     // GH#3158
-                    if (_interactivity.ManglePathsForWsl())
+                    const auto isWSL = _interactivity.ManglePathsForWsl();
+
+                    if (isWSL)
                     {
                         std::replace(fullPath.begin(), fullPath.end(), L'\\', L'/');
 
@@ -2942,17 +2944,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                         }
                     }
 
-                    const auto containsSpaces = std::find(fullPath.begin(),
-                                                          fullPath.end(),
-                                                          L' ') != fullPath.end();
+                    const auto quotesNeeded = isWSL || fullPath.find(L' ') != std::wstring::npos;
+                    const auto quotesChar = isWSL ? L'\'' : L'"';
 
-                    if (containsSpaces)
+                    // Append fullPath and also wrap it in quotes if needed
+                    if (quotesNeeded)
                     {
-                        fullPath.insert(0, L"\"");
-                        fullPath += L"\"";
+                        allPathsString.push_back(quotesChar);
+                    }
+                    allPathsString.append(fullPath);
+                    if (quotesNeeded)
+                    {
+                        allPathsString.push_back(quotesChar);
                     }
-
-                    allPathsString += fullPath;
                 }
 
                 _pasteTextWithBroadcast(winrt::hstring{ allPathsString });

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -484,7 +484,7 @@ void AppHost::_revokeWindowCallbacks()
     // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first.
     _revokers = {};
     _showHideWindowThrottler.reset();
-
+    _stopFrameTimer();
     _revokeWindowCallbacks();
 
     // DO NOT CLOSE THE WINDOW

src/cascadia/WindowsTerminal/IslandWindow.cpp

@@ -73,6 +73,8 @@ void IslandWindow::Refrigerate() noexcept
     // This pointer will get re-set in _warmInitialize
     SetWindowLongPtr(_window.get(), GWLP_USERDATA, 0);
 
+    _resetSystemMenu();
+
     _pfnCreateCallback = nullptr;
     _pfnSnapDimensionCallback = nullptr;
 
@@ -1917,6 +1919,12 @@ void IslandWindow::RemoveFromSystemMenu(const winrt::hstring& itemLabel)
     _systemMenuItems.erase(it->first);
 }
 
+void IslandWindow::_resetSystemMenu()
+{
+    // GetSystemMenu(..., true) will revert the menu to the default state.
+    GetSystemMenu(_window.get(), TRUE);
+}
+
 void IslandWindow::UseDarkTheme(const bool v)
 {
     const BOOL attribute = v ? TRUE : FALSE;

src/cascadia/WindowsTerminal/IslandWindow.h

@@ -159,6 +159,7 @@ class IslandWindow :
 
     std::unordered_map<UINT, SystemMenuItemInfo> _systemMenuItems;
     UINT _systemMenuNextItemId;
+    void _resetSystemMenu();
 
 private:
     // This minimum width allows for width the tabs fit

src/host/ft_host/sources

@@ -55,4 +55,5 @@ TARGETLIBS = \
 
 DELAYLOAD = \
     $(DELAYLOAD) \
+    icu.dll; \
     ext-ms-win-rtcore-ntuser-dpi-l1.dll; \

src/host/ft_host/sources.dep

@@ -1,6 +1,3 @@
 BUILD_PASS1_CONSUMES= \
     onecore\windows\vcpkg|PASS1 \
 
-BUILD_PASS2_CONSUMES= \
-    onecore\windows\core\console\open\src\tools\nihilist|PASS2 \
-

src/host/ft_integrity/IntegrityTest.cpp

@@ -214,14 +214,14 @@ void IntegrityTest::_TestValidationHelper(const bool fIsBlockExpected,
     GetConsoleScreenBufferInfoEx(GetStdHandle(STD_OUTPUT_HANDLE),
                                  &csbiex);
 
-    LOG_OUTPUT(L"Buffer Size X:%d Y:%d", csbiex.dwSize.width, csbiex.dwSize.height);
+    LOG_OUTPUT(L"Buffer Size X:%d Y:%d", csbiex.dwSize.X, csbiex.dwSize.Y);
 
-    size_t cch = csbiex.dwSize.width;
+    size_t cch = csbiex.dwSize.X;
     wistd::unique_ptr<wchar_t[]> stringData = wil::make_unique_nothrow<wchar_t[]>(cch);
     THROW_IF_NULL_ALLOC(stringData);
 
     COORD coordRead = { 0 };
-    for (coordRead.y = 0; coordRead.y < 8; coordRead.y++)
+    for (coordRead.Y = 0; coordRead.Y < 8; coordRead.Y++)
     {
         ZeroMemory(stringData.get(), sizeof(wchar_t) * cch);
 
@@ -237,7 +237,7 @@ void IntegrityTest::_TestValidationHelper(const bool fIsBlockExpected,
         WEX::Common::String strActual;
 
         // At position 0, check the integrity.
-        if (coordRead.y == 0)
+        if (coordRead.Y == 0)
         {
             strExpected = pwszIntegrityExpected;
         }
@@ -246,11 +246,11 @@ void IntegrityTest::_TestValidationHelper(const bool fIsBlockExpected,
             // For the rest, check whether the API call worked.
             if (fIsBlockExpected)
             {
-                strExpected = _rgpwszExpectedFail[coordRead.y - 1];
+                strExpected = _rgpwszExpectedFail[coordRead.Y - 1];
             }
             else
             {
-                strExpected = _rgpwszExpectedSuccess[coordRead.y - 1];
+                strExpected = _rgpwszExpectedSuccess[coordRead.Y - 1];
             }
         }
         stringData[strExpected.GetLength()] = L'\0';
@@ -312,7 +312,7 @@ PCWSTR IntegrityTest::s_GetMyIntegrityLevel()
     DWORD dwIntegrityLevel = 0;
 
     // Get the Integrity level.
-    wistd::unique_ptr<TOKEN_MANDATORY_LABEL> tokenLabel;
+    wil::unique_tokeninfo_ptr<TOKEN_MANDATORY_LABEL> tokenLabel;
     THROW_IF_FAILED(wil::GetTokenInformationNoThrow(tokenLabel, GetCurrentProcessToken()));
 
     dwIntegrityLevel = *GetSidSubAuthority(tokenLabel->Label.Sid,

src/host/ft_integrity/sources

@@ -33,7 +33,6 @@ INCLUDES=\
     $(COM_INC_PATH); \
     $(ONECOREBASE_INTERNAL_INC_PATH_L)\appmodel\test\common; \
     $(ONECOREREDIST_INTERNAL_INC_PATH_L)\TAEF; \
-    $(ONECORE_PRIV_SDK_INC_PATH); \
     $(MINCORE_INTERNAL_PRIV_SDK_INC_PATH_L); \
 
 TARGETLIBS=\

src/host/sources.inc

@@ -198,6 +198,7 @@ DELAYLOAD = \
     DXGI.dll; \
     OLEAUT32.dll; \
     PROPSYS.dll; \
+    icu.dll; \
     api-ms-win-core-com-l1.dll; \
     api-ms-win-core-registry-l2.dll; \
     api-ms-win-mm-playsound-l1.dll; \

src/inc/HostAndPropsheetIncludes.h

@@ -14,7 +14,9 @@
 #include <windows.h>
 #undef WIN32_NO_STATUS
 
+#ifndef NO_WINTERNL_INBOX_BUILD
 #include <winternl.h>
+#endif
 
 #pragma warning(push)
 #pragma warning(disable:4430) // Must disable 4430 "default int" warning for C++ because ntstatus.h is inflexible SDK definition.

src/inc/LibraryIncludes.h

@@ -18,7 +18,9 @@
 // Block minwindef.h min/max macros to prevent <algorithm> conflict
 #define NOMINMAX
 // Exclude rarely-used stuff from Windows headers
+#ifndef WIN32_LEAN_AND_MEAN
 #define WIN32_LEAN_AND_MEAN
+#endif
 
 #include <algorithm>
 #include <atomic>
@@ -65,6 +67,7 @@
 // GSL
 // Block GSL Multi Span include because it both has C++17 deprecated iterators
 // and uses the C-namespaced "max" which conflicts with Windows definitions.
+#include <gsl/gsl>
 #include <gsl/gsl_util>
 #include <gsl/pointers>
 

src/interactivity/onecore/precomp.h

@@ -10,13 +10,16 @@
 #include <nt.h>
 #include <ntrtl.h>
 #include <nturtl.h>
+#define WIN32_NO_STATUS
 #include <windows.h>
+#undef WIN32_NO_STATUS
 #include "wchar.h"
 
 // Extension presence detection
 #include <sysparamsext.h>
 
 #define _DDK_INCLUDED
+#define NO_WINTERNL_INBOX_BUILD
 #include "../../host/precomp.h"
 
 #else

src/interactivity/win32/ut_interactivity_win32/sources

@@ -110,6 +110,7 @@ DELAYLOAD = \
     DXGI.dll; \
     D3D11.dll; \
     OLEAUT32.dll; \
+    icu.dll; \
     api-ms-win-mm-playsound-l1.dll; \
     api-ms-win-shcore-scaling-l1.dll; \
     api-ms-win-shell-dataobject-l1.dll; \

src/project.inc

@@ -35,7 +35,7 @@ USE_NATIVE_EH           = 1
 
 USE_STD_CPP20           = 1
 MSC_WARNING_LEVEL       = /W4 /WX
-USER_C_FLAGS            = $(USER_C_FLAGS) /fp:contract /utf-8
+USER_C_FLAGS            = $(USER_C_FLAGS) /Zc:preprocessor /fp:contract /utf-8
 
 # -------------------------------------
 # Common Console Includes and Libraries

src/propsheet/misc.cpp

@@ -398,6 +398,10 @@ int AddFont(
     /*
      * Store the font info
      */
+    if (FontInfo[nFont].hFont != nullptr)
+    {
+        DeleteObject(FontInfo[nFont].hFont);
+    }
     FontInfo[nFont].hFont = hFont;
     FontInfo[nFont].Family = tmFamily;
     FontInfo[nFont].Size = SizeActual;

src/renderer/atlas/AtlasEngine.api.cpp

@@ -490,20 +490,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;
     }
 }
 

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;
     };
 

src/terminal/adapter/ut_adapter/sources

@@ -105,6 +105,7 @@ DELAYLOAD = \
     DXGI.dll; \
     D3D11.dll; \
     OLEAUT32.dll; \
+    icu.dll; \
     api-ms-win-mm-playsound-l1.dll; \
     api-ms-win-shcore-scaling-l1.dll; \
     api-ms-win-shell-dataobject-l1.dll; \
@@ -144,7 +145,6 @@ DLOAD_ERROR_HANDLER = kernelbase
 
 #INCLUDES                        =   $(INCLUDES); \
 #                                    ..\..\..\inc; \
-#                                    $(SDKTOOLS_INC_PATH)\WexTest\Cue; \
 #
 #SOURCES                         =   $(SOURCES) \
 #

src/terminal/parser/ut_parser/sources

@@ -96,6 +96,7 @@ DELAYLOAD = \
     DXGI.dll; \
     D3D11.dll; \
     OLEAUT32.dll; \
+    icu.dll; \
     api-ms-win-mm-playsound-l1.dll; \
     api-ms-win-shcore-scaling-l1.dll; \
     api-ms-win-shell-dataobject-l1.dll; \

src/tools/integrity/lib/util.cpp

@@ -14,7 +14,7 @@ PCWSTR GetIntegrityLevel()
     DWORD dwIntegrityLevel = 0;
 
     // Get the Integrity level.
-    wistd::unique_ptr<TOKEN_MANDATORY_LABEL> tokenLabel;
+    wil::unique_tokeninfo_ptr<TOKEN_MANDATORY_LABEL> tokenLabel;
     THROW_IF_FAILED(wil::GetTokenInformationNoThrow(tokenLabel, GetCurrentProcessToken()));
 
     dwIntegrityLevel = *GetSidSubAuthority(tokenLabel->Label.Sid,