Keep the font size delta across settings reloads (#20230)

This is a trivial fix for an issue we get a somewhat outsized number of
complaints about.

When the user has adjusted the font size in any direction, we'll just
keep track of the magnitude and apply it every time the settings change.

Yes, this means that if you zoom once and then change your real font
size it's going to zoom even more.

That probably doesn't matter.

Refs #11522

.github/actions/spelling/expect/expect.txt

@@ -1075,6 +1075,7 @@ NOCONTEXTHELP
 NOCOPYBITS
 nodiscard
 NODUP
+NODEFAULT
 noexcepts
 NOFONT
 NOHIDDENTEXT
@@ -1561,6 +1562,7 @@ SMARTQUOTE
 SMTO
 snapcx
 snapcy
+SND
 snk
 SOLIDBOX
 Solutiondir

src/cascadia/TerminalApp/TerminalPaneContent.cpp

@@ -71,18 +71,6 @@ namespace winrt::TerminalApp::implementation
         _removeControlEvents();
 
         _control.Close();
-
-        // Clear out our media player callbacks, and stop any playing media. This
-        // will prevent the callback from being triggered after we've closed, and
-        // also make sure that our sound stops when we're closed.
-        if (_bellPlayer)
-        {
-            _bellPlayer.Pause();
-            _bellPlayer.Source(nullptr);
-            _bellPlayer.Close();
-            _bellPlayer = nullptr;
-            _bellPlayerCreated = false;
-        }
     }
 
     winrt::hstring TerminalPaneContent::Icon() const
@@ -275,14 +263,15 @@ namespace winrt::TerminalApp::implementation
                     auto sounds{ _profile.BellSound() };
                     if (sounds && sounds.Size() > 0)
                     {
-                        winrt::hstring soundPath{ sounds.GetAt(rand() % sounds.Size()).Resolved() };
-                        winrt::Windows::Foundation::Uri uri{ soundPath };
-                        _playBellSound(uri);
+                        // Sound paths are resolved and validated by CascadiaSettings
+                        // before we reach this point.
+                        auto soundPath{ sounds.GetAt(rand() % sounds.Size()).Resolved() };
+                        PlaySoundW(soundPath.c_str(), nullptr, SND_FILENAME | SND_ASYNC | SND_SENTRY | SND_NODEFAULT);
                     }
                     else
                     {
-                        const auto soundAlias = reinterpret_cast<LPCTSTR>(SND_ALIAS_SYSTEMHAND);
-                        PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY);
+                        const auto soundAlias = reinterpret_cast<LPCWSTR>(SND_ALIAS_SYSTEMHAND);
+                        PlaySoundW(soundAlias, nullptr, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY);
                     }
                 }
 
@@ -300,33 +289,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    safe_void_coroutine TerminalPaneContent::_playBellSound(winrt::Windows::Foundation::Uri uri)
-    {
-        auto weakThis{ get_weak() };
-        co_await wil::resume_foreground(_control.Dispatcher());
-        if (auto pane{ weakThis.get() })
-        {
-            if (!_bellPlayerCreated)
-            {
-                // The MediaPlayer might not exist on Windows N SKU.
-                try
-                {
-                    _bellPlayerCreated = true;
-                    _bellPlayer = winrt::Windows::Media::Playback::MediaPlayer();
-                    // GH#12258: The media keys (like play/pause) should have no effect on our bell sound.
-                    _bellPlayer.CommandManager().IsEnabled(false);
-                }
-                CATCH_LOG();
-            }
-            if (_bellPlayer)
-            {
-                const auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) };
-                const auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) };
-                _bellPlayer.Source(item);
-                _bellPlayer.Play();
-            }
-        }
-    }
     void TerminalPaneContent::_closeTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,
                                                              const winrt::Windows::Foundation::IInspectable& /*args*/)
     {

src/cascadia/TerminalApp/TerminalPaneContent.h

@@ -76,9 +76,6 @@ namespace winrt::TerminalApp::implementation
         std::shared_ptr<TerminalSettingsCache> _cache{};
         bool _isDefTermSession{ false };
 
-        winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr };
-        bool _bellPlayerCreated{ false };
-
         struct ControlEventTokens
         {
             winrt::Microsoft::Terminal::Control::TermControl::ConnectionStateChanged_revoker _ConnectionStateChanged;
@@ -96,8 +93,6 @@ namespace winrt::TerminalApp::implementation
         void _setupControlEvents();
         void _removeControlEvents();
 
-        safe_void_coroutine _playBellSound(winrt::Windows::Foundation::Uri uri);
-
         safe_void_coroutine _controlConnectionStateChangedHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& /*args*/);
         void _controlWarningBellHandler(const winrt::Windows::Foundation::IInspectable& sender,
                                         const winrt::Windows::Foundation::IInspectable& e);

src/cascadia/TerminalControl/ControlCore.cpp

@@ -925,7 +925,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // Manually turn off acrylic if they turn off transparency.
         _runtimeUseAcrylic = _settings.Opacity() < 1.0 && _settings.UseAcrylic();
 
-        const auto sizeChanged = _setFontSizeUnderLock(_settings.FontSize());
+        const auto sizeChanged = _setFontSizeUnderLock(_settings.FontSize() + _accumulatedFontSizeDelta);
 
         // Update the terminal core with its new Core settings
         _terminal->UpdateSettings(_settings);
@@ -1163,11 +1163,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - none
     void ControlCore::ResetFontSize()
     {
-        const auto lock = _terminal->LockForWriting();
-
-        if (_setFontSizeUnderLock(_settings.FontSize()))
+        if (std::exchange(_accumulatedFontSizeDelta, 0.f) != 0.f)
         {
-            _refreshSizeUnderLock();
+            // No point in doing this if there was no delta.
+            AdjustFontSize(0);
         }
     }
 
@@ -1177,9 +1176,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - fontSizeDelta: The amount to increase or decrease the font size by.
     void ControlCore::AdjustFontSize(float fontSizeDelta)
     {
+        _accumulatedFontSizeDelta += fontSizeDelta;
+
         const auto lock = _terminal->LockForWriting();
 
-        if (_setFontSizeUnderLock(_desiredFont.GetFontSize() + fontSizeDelta))
+        if (_setFontSizeUnderLock(_settings.FontSize() + _accumulatedFontSizeDelta))
         {
             _refreshSizeUnderLock();
         }

src/cascadia/TerminalControl/ControlCore.h

@@ -391,6 +391,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         bool _colorGlyphs = true;
         CSSLengthPercentage _cellWidth;
         CSSLengthPercentage _cellHeight;
+        float _accumulatedFontSizeDelta = 0.f; // Preserved across reloads to prevent user zoom from being overwritten.
 
         // Rendering stuff.
         winrt::handle _lastSwapChainHandle{ nullptr };

src/host/ut_host/ScreenBufferTests.cpp

@@ -123,6 +123,7 @@ class ScreenBufferTests
     TEST_METHOD(VtResizePreservingAttributes);
 
     TEST_METHOD(VtSoftResetCursorPosition);
+    TEST_METHOD(VtSoftResetAltBufferCursorState);
 
     TEST_METHOD(VtScrollMarginsNewlineColor);
 
@@ -1510,6 +1511,30 @@ void ScreenBufferTests::VtSoftResetCursorPosition()
     VERIFY_ARE_EQUAL(til::point(1, 1), cursor.GetPosition());
 }
 
+void ScreenBufferTests::VtSoftResetAltBufferCursorState()
+{
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    gci.LockConsole(); // Lock must be taken to manipulate buffer.
+    auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });
+
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& stateMachine = si.GetStateMachine();
+
+    Log::Comment(L"Move cursor on the main buffer.");
+    stateMachine.ProcessString(L"\x1b[4;7H");
+    VERIFY_ARE_EQUAL(til::point(6, 3), si.GetTextBuffer().GetCursor().GetPosition());
+
+    Log::Comment(L"Enter alt buffer, soft reset, and return to main buffer.");
+    stateMachine.ProcessString(L"\x1b[?1049h");
+    VERIFY_IS_TRUE(gci.GetActiveOutputBuffer()._IsAltBuffer());
+    stateMachine.ProcessString(L"\x1b[!p");
+    stateMachine.ProcessString(L"\x1b[?1049l");
+    VERIFY_IS_FALSE(gci.GetActiveOutputBuffer()._IsAltBuffer());
+
+    Log::Comment(L"Returning from alt buffer should restore the main cursor position.");
+    VERIFY_ARE_EQUAL(til::point(6, 3), gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition());
+}
+
 void ScreenBufferTests::VtScrollMarginsNewlineColor()
 {
     auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();

src/terminal/adapter/adaptDispatch.cpp

@@ -3002,17 +3002,15 @@ void AdaptDispatch::SoftReset()
     SetGraphicsRendition({}); // Normal rendition.
     SetCharacterProtectionAttribute({}); // Default (unprotected)
 
-    // Reset the saved cursor state.
-    // Note that XTerm only resets the main buffer state, but that
-    // seems likely to be a bug. Most other terminals reset both.
-    _savedCursorState.at(0) = {}; // Main buffer
-    _savedCursorState.at(1) = {}; // Alt buffer
+    // Reset only the active saved cursor state.
+    // This matches xterm behavior when DECSTR is processed while using
+    // the alternate screen buffer (GH#19918).
+    _savedCursorState.at(_usingAltBuffer ? 1 : 0) = {};
 
-    // The TerminalOutput state in these buffers must be reset to
+    // The TerminalOutput state in this buffer must be reset to
     // the same state as the _termOutput instance, which is not
     // necessarily equivalent to a full reset.
-    _savedCursorState.at(0).TermOutput = _termOutput;
-    _savedCursorState.at(1).TermOutput = _termOutput;
+    _savedCursorState.at(_usingAltBuffer ? 1 : 0).TermOutput = _termOutput;
 
     // Soft reset the Sixel parser if in use.
     if (_sixelParser)