From cccaab8545f4d9b8918b08c2fd7d6d2d4d79639e Mon Sep 17 00:00:00 2001 From: Ian O'Neill Date: Tue, 3 Aug 2021 19:16:07 +0100 Subject: [PATCH 01/16] Fix drag and drop on '+' button for drive letters (#10842) Fixes dragging and dropping drive letters onto the '+' button. Manually tested - dragging and dropping the `C:\` drive onto the '+' button works when creating a new tab, splitting or creating a new window. Dragging and dropping a regular directory still works. Closes #10723 --- .../LocalTests_SettingsModel/CommandTests.cpp | 21 ++++++++++++++++++- src/cascadia/TerminalApp/TerminalPage.cpp | 10 +-------- .../TerminalSettingsModel/ActionArgs.cpp | 5 ++++- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp index ce5f06014..bdc4d5692 100644 --- a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp @@ -397,6 +397,10 @@ namespace SettingsModelLocalTests "name":"action6", "command": { "action": "newWindow", "startingDirectory":"C:\\foo", "commandline": "bar.exe" } }, + { + "name":"action7_startingDirectoryWithTrailingSlash", + "command": { "action": "newWindow", "startingDirectory":"C:\\", "commandline": "bar.exe" } + }, ])" }; const auto commands0Json = VerifyParseSucceeded(commands0String); @@ -405,7 +409,7 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(0u, commands.Size()); auto warnings = implementation::Command::LayerJson(commands, commands0Json); VERIFY_ARE_EQUAL(0u, warnings.size()); - VERIFY_ARE_EQUAL(7u, commands.Size()); + VERIFY_ARE_EQUAL(8u, commands.Size()); { auto command = commands.Lookup(L"action0"); @@ -503,5 +507,20 @@ namespace SettingsModelLocalTests L"cmdline: \"%s\"", cmdline.c_str())); VERIFY_ARE_EQUAL(L"--startingDirectory \"C:\\foo\" -- \"bar.exe\"", terminalArgs.ToCommandline()); } + + { + auto command = commands.Lookup(L"action7_startingDirectoryWithTrailingSlash"); + VERIFY_IS_NOT_NULL(command); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); + VERIFY_IS_NOT_NULL(realArgs); + const auto& terminalArgs = realArgs.TerminalArgs(); + VERIFY_IS_NOT_NULL(terminalArgs); + auto cmdline = terminalArgs.ToCommandline(); + Log::Comment(NoThrowString().Format( + L"cmdline: \"%s\"", cmdline.c_str())); + VERIFY_ARE_EQUAL(L"--startingDirectory \"C:\\\\\" -- \"bar.exe\"", terminalArgs.ToCommandline()); + } } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index fc552d767..281189920 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -253,16 +253,8 @@ namespace winrt::TerminalApp::implementation path = path.parent_path(); } - std::wstring pathText = path.wstring(); - - // Handle edge case of "C:\\", seems like the "StartingDirectory" doesn't like path which ends with '\' - if (pathText.back() == L'\\') - { - pathText.erase(std::prev(pathText.end())); - } - NewTerminalArgs args; - args.StartingDirectory(winrt::hstring{ pathText }); + args.StartingDirectory(winrt::hstring{ path.wstring() }); this->_OpenNewTerminal(args); TraceLoggingWrite( diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp index 7bc057742..9baaaf240 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp @@ -118,7 +118,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation if (!StartingDirectory().empty()) { - ss << fmt::format(L"--startingDirectory \"{}\" ", StartingDirectory()); + // If the directory ends in a '\', we need to add another one on so that the enclosing quote added + // afterwards isn't escaped + const auto trailingBackslashEscape = StartingDirectory().back() == L'\\' ? L"\\" : L""; + ss << fmt::format(L"--startingDirectory \"{}{}\" ", StartingDirectory(), trailingBackslashEscape); } if (!TabTitle().empty()) From 8ab3422b57231451eacf2e32c437478f44975b83 Mon Sep 17 00:00:00 2001 From: Marcel Wagner Date: Wed, 4 Aug 2021 00:25:23 +0200 Subject: [PATCH 02/16] [settings-editor] Switch to function bindings instead of Converter objects (#10846) ## Validation Steps Performed Clicked around, validated that settings still behave the same (as far as I can tell with my limited terminal configuration expertise) Closes #10387 --- .../TerminalSettingsEditor/Actions.xaml | 7 +- .../TerminalSettingsEditor/Appearances.h | 13 +++ .../TerminalSettingsEditor/Appearances.idl | 5 + .../TerminalSettingsEditor/Appearances.xaml | 24 ++-- .../ColorLightenConverter.cpp | 33 ------ .../ColorLightenConverter.h | 9 -- .../TerminalSettingsEditor/ColorSchemes.xaml | 13 +-- .../ColorToBrushConverter.cpp | 28 ----- .../ColorToBrushConverter.h | 30 ----- .../ColorToHexConverter.cpp | 30 ----- .../ColorToHexConverter.h | 30 ----- .../TerminalSettingsEditor/Converters.cpp | 106 ++++++++++++++++++ .../TerminalSettingsEditor/Converters.h | 33 ++++++ .../TerminalSettingsEditor/Converters.idl | 73 +++--------- .../FontWeightConverter.cpp | 32 ------ .../FontWeightConverter.h | 30 ----- .../GlobalAppearance.xaml | 4 +- .../InvertedBooleanConverter.cpp | 28 ----- .../InvertedBooleanConverter.h | 9 -- .../InvertedBooleanToVisibilityConverter.cpp | 28 ----- .../InvertedBooleanToVisibilityConverter.h | 9 -- ...Microsoft.Terminal.Settings.Editor.vcxproj | 81 ++----------- ...t.Terminal.Settings.Editor.vcxproj.filters | 13 +-- .../PaddingConverter.cpp | 65 ----------- .../TerminalSettingsEditor/PaddingConverter.h | 9 -- .../PercentageConverter.cpp | 32 ------ .../PercentageConverter.h | 30 ----- .../TerminalSettingsEditor/Profiles.h | 10 ++ .../TerminalSettingsEditor/Profiles.idl | 5 + .../TerminalSettingsEditor/Profiles.xaml | 25 ++--- .../ReadOnlyActions.xaml | 3 - .../StringIsEmptyConverter.cpp | 30 ----- .../StringIsEmptyConverter.h | 9 -- .../StringIsNotDesktopConverter.cpp | 50 --------- .../StringIsNotDesktopConverter.h | 11 -- .../TerminalSettingsModel/GlobalAppSettings.h | 6 + .../GlobalAppSettings.idl | 3 + 37 files changed, 231 insertions(+), 725 deletions(-) delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorLightenConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorLightenConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorToHexConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/ColorToHexConverter.h create mode 100644 src/cascadia/TerminalSettingsEditor/Converters.cpp create mode 100644 src/cascadia/TerminalSettingsEditor/Converters.h delete mode 100644 src/cascadia/TerminalSettingsEditor/FontWeightConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/FontWeightConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/PaddingConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/PaddingConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/PercentageConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/PercentageConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/StringIsEmptyConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/StringIsEmptyConverter.h delete mode 100644 src/cascadia/TerminalSettingsEditor/StringIsNotDesktopConverter.cpp delete mode 100644 src/cascadia/TerminalSettingsEditor/StringIsNotDesktopConverter.h diff --git a/src/cascadia/TerminalSettingsEditor/Actions.xaml b/src/cascadia/TerminalSettingsEditor/Actions.xaml index e4dd4ccc7..11c1f65e4 100644 --- a/src/cascadia/TerminalSettingsEditor/Actions.xaml +++ b/src/cascadia/TerminalSettingsEditor/Actions.xaml @@ -158,7 +158,6 @@ - + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(IsInEditMode), Mode=OneWay}" /> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(IsInEditMode), Mode=OneWay}"> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(IsNewlyAdded), Mode=OneWay}"> diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.h b/src/cascadia/TerminalSettingsEditor/Appearances.h index db00e79c4..6860b0361 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.h +++ b/src/cascadia/TerminalSettingsEditor/Appearances.h @@ -51,6 +51,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation public: AppearanceViewModel(const Model::AppearanceConfig& appearance); + void SetFontWeightFromDouble(double fontWeight) + { + FontWeight(winrt::Microsoft::Terminal::Settings::Editor::Converters::DoubleToFontWeight(fontWeight)); + } + void SetBackgroundImageOpacityFromPercentageValue(double percentageValue) + { + BackgroundImageOpacity(winrt::Microsoft::Terminal::Settings::Editor::Converters::PercentageValueToPercentage(percentageValue)); + } + void SetBackgroundImagePath(winrt::hstring path) + { + BackgroundImagePath(path); + } + // background image bool UseDesktopBGImage(); void UseDesktopBGImage(const bool useDesktop); diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.idl b/src/cascadia/TerminalSettingsEditor/Appearances.idl index a4faa7a30..ddc83abed 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.idl +++ b/src/cascadia/TerminalSettingsEditor/Appearances.idl @@ -23,6 +23,10 @@ namespace Microsoft.Terminal.Settings.Editor { Boolean IsDefault; + void SetFontWeightFromDouble(Double fontWeight); + void SetBackgroundImageOpacityFromPercentageValue(Double percentageValue); + void SetBackgroundImagePath(String path); + Boolean UseDesktopBGImage; Boolean BackgroundImageSettingsVisible { get; }; @@ -68,5 +72,6 @@ namespace Microsoft.Terminal.Settings.Editor Windows.Foundation.Collections.IObservableVector FontWeightList { get; }; IInspectable CurrentFontFace { get; }; + Windows.UI.Xaml.Controls.Slider BIOpacitySlider { get; }; } } diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.xaml b/src/cascadia/TerminalSettingsEditor/Appearances.xaml index f31e625ec..d49de2051 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.xaml +++ b/src/cascadia/TerminalSettingsEditor/Appearances.xaml @@ -34,16 +34,6 @@ - - - - - - - - - - @@ -87,7 +77,7 @@ SelectedItem="{x:Bind CurrentFontFace, Mode=OneWay}" SelectionChanged="FontFace_SelectionChanged" Style="{StaticResource ComboBoxSettingStyle}" - Visibility="{x:Bind ShowAllFonts, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}" /> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(ShowAllFonts), Mode=OneWay}" /> + Value="{x:Bind local:Converters.FontWeightToDouble(Appearance.FontWeight), BindBack=Appearance.SetFontWeightFromDouble, Mode=TwoWay}" /> - + Text="{x:Bind local:Converters.StringFallBackToEmptyString('desktopWallpaper', Appearance.BackgroundImagePath), Mode=TwoWay, BindBack=Appearance.SetBackgroundImagePath}" /> - - - - - @@ -106,7 +101,7 @@ + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(IsRenaming), Mode=OneWay}"> (value))); - } - - Foundation::IInspectable ColorToBrushConverter::ConvertBack(Foundation::IInspectable const& /*value*/, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - throw hresult_not_implemented(); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.h b/src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.h deleted file mode 100644 index 94c20db6d..000000000 --- a/src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "ColorToBrushConverter.g.h" -#include "Utils.h" - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - struct ColorToBrushConverter : ColorToBrushConverterT - { - ColorToBrushConverter() = default; - - Windows::Foundation::IInspectable Convert(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - - Windows::Foundation::IInspectable ConvertBack(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - }; -} - -namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation -{ - BASIC_FACTORY(ColorToBrushConverter); -} diff --git a/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.cpp b/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.cpp deleted file mode 100644 index 1d9379ab3..000000000 --- a/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.cpp +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "ColorToHexConverter.h" -#include "ColorToHexConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable ColorToHexConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - til::color color{ winrt::unbox_value(value) }; - auto hex = winrt::to_hstring(color.ToHexString().data()); - return winrt::box_value(hex); - } - - Foundation::IInspectable ColorToHexConverter::ConvertBack(Foundation::IInspectable const& /*value*/, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - throw hresult_not_implemented(); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.h b/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.h deleted file mode 100644 index 53e2963df..000000000 --- a/src/cascadia/TerminalSettingsEditor/ColorToHexConverter.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "ColorToHexConverter.g.h" -#include "Utils.h" - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - struct ColorToHexConverter : ColorToHexConverterT - { - ColorToHexConverter() = default; - - Windows::Foundation::IInspectable Convert(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - - Windows::Foundation::IInspectable ConvertBack(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - }; -} - -namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation -{ - BASIC_FACTORY(ColorToHexConverter); -} diff --git a/src/cascadia/TerminalSettingsEditor/Converters.cpp b/src/cascadia/TerminalSettingsEditor/Converters.cpp new file mode 100644 index 000000000..d9cff642e --- /dev/null +++ b/src/cascadia/TerminalSettingsEditor/Converters.cpp @@ -0,0 +1,106 @@ +#include "pch.h" +#include "Converters.h" +#if __has_include("Converters.g.cpp") +#include "Converters.g.cpp" +#endif + +namespace winrt::Microsoft::Terminal::Settings::Editor::implementation +{ + winrt::hstring Converters::AppendPercentageSign(double value) + { + const auto number{ value }; + return to_hstring((int)number) + L"%"; + } + + winrt::Windows::UI::Xaml::Media::SolidColorBrush Converters::ColorToBrush(winrt::Windows::UI::Color color) + { + return Windows::UI::Xaml::Media::SolidColorBrush(color); + } + + winrt::Windows::UI::Text::FontWeight Converters::DoubleToFontWeight(double value) + { + return winrt::Windows::UI::Text::FontWeight{ base::ClampedNumeric(value) }; + } + + double Converters::FontWeightToDouble(winrt::Windows::UI::Text::FontWeight fontWeight) + { + return fontWeight.Weight; + } + + bool Converters::InvertBoolean(bool value) + { + return !value; + } + + winrt::Windows::UI::Xaml::Visibility Converters::InvertedBooleanToVisibility(bool value) + { + return value ? winrt::Windows::UI::Xaml::Visibility::Collapsed : winrt::Windows::UI::Xaml::Visibility::Visible; + } + + winrt::Windows::UI::Color Converters::LightenColor(winrt::Windows::UI::Color color) + { + color.A = 128; // halfway transparent + return color; + } + + double Converters::MaxValueFromPaddingString(winrt::hstring paddingString) + { + const wchar_t singleCharDelim = L','; + std::wstringstream tokenStream(paddingString.c_str()); + std::wstring token; + double maxVal = 0; + size_t* idx = nullptr; + + // Get padding values till we run out of delimiter separated values in the stream + // Non-numeral values detected will default to 0 + // std::getline will not throw exception unless flags are set on the wstringstream + // std::stod will throw invalid_argument exception if the input is an invalid double value + // std::stod will throw out_of_range exception if the input value is more than DBL_MAX + try + { + while (std::getline(tokenStream, token, singleCharDelim)) + { + // std::stod internally calls wcstod which handles whitespace prefix (which is ignored) + // & stops the scan when first char outside the range of radix is encountered + // We'll be permissive till the extent that stod function allows us to be by default + // Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail + const auto curVal = std::stod(token, idx); + if (curVal > maxVal) + { + maxVal = curVal; + } + } + } + catch (...) + { + // If something goes wrong, even if due to a single bad padding value, we'll return default 0 padding + maxVal = 0; + LOG_CAUGHT_EXCEPTION(); + } + + return maxVal; + } + + int Converters::PercentageToPercentageValue(double value) + { + return base::ClampMul(value, 100u); + } + + double Converters::PercentageValueToPercentage(double value) + { + return base::ClampDiv(value, 100); + } + + bool Converters::StringsAreNotEqual(winrt::hstring expected, winrt::hstring actual) + { + return expected != actual; + } + winrt::Windows::UI::Xaml::Visibility Converters::StringNotEmptyToVisibility(winrt::hstring value) + { + return value.empty() ? winrt::Windows::UI::Xaml::Visibility::Collapsed : winrt::Windows::UI::Xaml::Visibility::Visible; + } + winrt::hstring Converters::StringFallBackToEmptyString(winrt::hstring expected, winrt::hstring actual) + { + return expected == actual ? expected : L""; + } +} diff --git a/src/cascadia/TerminalSettingsEditor/Converters.h b/src/cascadia/TerminalSettingsEditor/Converters.h new file mode 100644 index 000000000..d7f99cbf2 --- /dev/null +++ b/src/cascadia/TerminalSettingsEditor/Converters.h @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "Converters.g.h" + +namespace winrt::Microsoft::Terminal::Settings::Editor::implementation +{ + struct Converters : ConvertersT + { + static winrt::hstring AppendPercentageSign(double value); + static winrt::Windows::UI::Text::FontWeight DoubleToFontWeight(double value); + static winrt::Windows::UI::Xaml::Media::SolidColorBrush ColorToBrush(winrt::Windows::UI::Color color); + static double FontWeightToDouble(winrt::Windows::UI::Text::FontWeight fontWeight); + static bool InvertBoolean(bool value); + static winrt::Windows::UI::Xaml::Visibility InvertedBooleanToVisibility(bool value); + static winrt::Windows::UI::Color LightenColor(winrt::Windows::UI::Color color); + static double MaxValueFromPaddingString(winrt::hstring paddingString); + static int PercentageToPercentageValue(double value); + static double PercentageValueToPercentage(double value); + static bool StringsAreNotEqual(winrt::hstring expected, winrt::hstring actual); + static winrt::Windows::UI::Xaml::Visibility StringNotEmptyToVisibility(winrt::hstring value); + static winrt::hstring StringFallBackToEmptyString(winrt::hstring expected, winrt::hstring actual); + }; +} + +namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation +{ + struct Converters : ConvertersT + { + }; +} diff --git a/src/cascadia/TerminalSettingsEditor/Converters.idl b/src/cascadia/TerminalSettingsEditor/Converters.idl index 89eb05791..a5b79d5fc 100644 --- a/src/cascadia/TerminalSettingsEditor/Converters.idl +++ b/src/cascadia/TerminalSettingsEditor/Converters.idl @@ -4,63 +4,22 @@ namespace Microsoft.Terminal.Settings.Editor { - runtimeclass ColorLightenConverter : [default] Windows.UI.Xaml.Data.IValueConverter + [bindable] + [default_interface] static runtimeclass Converters { - ColorLightenConverter(); - }; - runtimeclass FontWeightConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - FontWeightConverter(); - }; - - runtimeclass InvertedBooleanConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - InvertedBooleanConverter(); - }; - - runtimeclass InvertedBooleanToVisibilityConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - InvertedBooleanToVisibilityConverter(); - }; - - runtimeclass ColorToBrushConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - ColorToBrushConverter(); - }; - - runtimeclass ColorToHexConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - ColorToHexConverter(); - }; - - runtimeclass PercentageConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - PercentageConverter(); - }; - - runtimeclass PercentageSignConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - PercentageSignConverter(); - }; - - runtimeclass StringIsEmptyConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - StringIsEmptyConverter(); - }; - - runtimeclass PaddingConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - PaddingConverter(); - }; - - runtimeclass StringIsNotDesktopConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - StringIsNotDesktopConverter(); - }; - - runtimeclass DesktopWallpaperToEmptyStringConverter : [default] Windows.UI.Xaml.Data.IValueConverter - { - DesktopWallpaperToEmptyStringConverter(); - }; + static String AppendPercentageSign(Double value); + static Windows.UI.Text.FontWeight DoubleToFontWeight(Double value); + static Windows.UI.Xaml.Media.SolidColorBrush ColorToBrush(Windows.UI.Color color); + static Double FontWeightToDouble(Windows.UI.Text.FontWeight fontWeight); + static Boolean InvertBoolean(Boolean value); + static Windows.UI.Xaml.Visibility InvertedBooleanToVisibility(Boolean value); + static Windows.UI.Color LightenColor(Windows.UI.Color color); + static Double MaxValueFromPaddingString(String paddingString); + static Int32 PercentageToPercentageValue(Double value); + static Double PercentageValueToPercentage(Double value); + static Boolean StringsAreNotEqual(String expected, String actual); + static Windows.UI.Xaml.Visibility StringNotEmptyToVisibility(String value); + static String StringFallBackToEmptyString(String expected, String actual); + } } diff --git a/src/cascadia/TerminalSettingsEditor/FontWeightConverter.cpp b/src/cascadia/TerminalSettingsEditor/FontWeightConverter.cpp deleted file mode 100644 index 5b39eded1..000000000 --- a/src/cascadia/TerminalSettingsEditor/FontWeightConverter.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "FontWeightConverter.h" -#include "FontWeightConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; -using namespace winrt::Windows::UI::Text; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable FontWeightConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - const auto weight{ winrt::unbox_value(value) }; - return winrt::box_value(weight.Weight); - } - - Foundation::IInspectable FontWeightConverter::ConvertBack(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - const auto sliderVal{ winrt::unbox_value(value) }; - FontWeight weight{ base::ClampedNumeric(sliderVal) }; - return winrt::box_value(weight); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/FontWeightConverter.h b/src/cascadia/TerminalSettingsEditor/FontWeightConverter.h deleted file mode 100644 index 249e7987d..000000000 --- a/src/cascadia/TerminalSettingsEditor/FontWeightConverter.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "FontWeightConverter.g.h" -#include "Utils.h" - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - struct FontWeightConverter : FontWeightConverterT - { - FontWeightConverter() = default; - - Windows::Foundation::IInspectable Convert(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - - Windows::Foundation::IInspectable ConvertBack(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - }; -} - -namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation -{ - BASIC_FACTORY(FontWeightConverter); -} diff --git a/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml b/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml index ae030da7c..c4f4bf2b4 100644 --- a/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml +++ b/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml @@ -21,8 +21,6 @@ x:DataType="local:EnumEntry"> - - @@ -79,7 +77,7 @@ - + diff --git a/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.cpp b/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.cpp deleted file mode 100644 index d0bb98075..000000000 --- a/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.cpp +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "InvertedBooleanConverter.h" -#include "InvertedBooleanConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable InvertedBooleanConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - return winrt::box_value(!winrt::unbox_value(value)); - } - - Foundation::IInspectable InvertedBooleanConverter::ConvertBack(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - return winrt::box_value(!winrt::unbox_value(value)); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.h b/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.h deleted file mode 100644 index 9da6768fe..000000000 --- a/src/cascadia/TerminalSettingsEditor/InvertedBooleanConverter.h +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "InvertedBooleanConverter.g.h" -#include "../inc/cppwinrt_utils.h" - -DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, InvertedBooleanConverter); diff --git a/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.cpp b/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.cpp deleted file mode 100644 index fd3c9e6fc..000000000 --- a/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.cpp +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "InvertedBooleanToVisibilityConverter.h" -#include "InvertedBooleanToVisibilityConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable InvertedBooleanToVisibilityConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - return winrt::box_value(winrt::unbox_value(value) ? Visibility::Collapsed : Visibility::Visible); - } - - Foundation::IInspectable InvertedBooleanToVisibilityConverter::ConvertBack(Foundation::IInspectable const& /*value*/, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - throw hresult_not_implemented(); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.h b/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.h deleted file mode 100644 index 90c6bbb74..000000000 --- a/src/cascadia/TerminalSettingsEditor/InvertedBooleanToVisibilityConverter.h +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "InvertedBooleanToVisibilityConverter.g.h" -#include "../inc/cppwinrt_utils.h" - -DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, InvertedBooleanToVisibilityConverter); diff --git a/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj b/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj index 4861cb1f4..e60fccb32 100644 --- a/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj +++ b/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj @@ -42,41 +42,9 @@ AddProfile.xaml - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - KeyChordListener.xaml - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - + Converters.idl + Code EnumEntry.idl @@ -91,6 +59,9 @@ Interaction.xaml + + KeyChordListener.xaml + Launch.xaml @@ -171,41 +142,9 @@ AddProfile.xaml - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - KeyChordListener.xaml - - - Converters.idl - - - Converters.idl - - - Converters.idl - - - Converters.idl - - + Converters.idl + Code GlobalAppearance.xaml @@ -217,6 +156,9 @@ Interaction.xaml + + KeyChordListener.xaml + Launch.xaml @@ -339,7 +281,6 @@ true false - false @@ -379,4 +320,4 @@ - + \ No newline at end of file diff --git a/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj.filters b/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj.filters index 66e8b687d..3b0b9186e 100644 --- a/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj.filters +++ b/src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj.filters @@ -10,16 +10,10 @@ - - Converters - - - Converters - @@ -49,9 +43,4 @@ - - - {00f725c8-41b4-40a8-995e-8ee2e49a4a4c} - - - + \ No newline at end of file diff --git a/src/cascadia/TerminalSettingsEditor/PaddingConverter.cpp b/src/cascadia/TerminalSettingsEditor/PaddingConverter.cpp deleted file mode 100644 index 6c0e3472c..000000000 --- a/src/cascadia/TerminalSettingsEditor/PaddingConverter.cpp +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "PaddingConverter.h" -#include "PaddingConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; -using namespace winrt::Windows::UI::Text; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable PaddingConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - const auto& padding = winrt::unbox_value(value); - - const wchar_t singleCharDelim = L','; - std::wstringstream tokenStream(padding.c_str()); - std::wstring token; - double maxVal = 0; - size_t* idx = nullptr; - - // Get padding values till we run out of delimiter separated values in the stream - // Non-numeral values detected will default to 0 - // std::getline will not throw exception unless flags are set on the wstringstream - // std::stod will throw invalid_argument exception if the input is an invalid double value - // std::stod will throw out_of_range exception if the input value is more than DBL_MAX - try - { - while (std::getline(tokenStream, token, singleCharDelim)) - { - // std::stod internally calls wcstod which handles whitespace prefix (which is ignored) - // & stops the scan when first char outside the range of radix is encountered - // We'll be permissive till the extent that stod function allows us to be by default - // Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail - const auto curVal = std::stod(token, idx); - if (curVal > maxVal) - { - maxVal = curVal; - } - } - } - catch (...) - { - // If something goes wrong, even if due to a single bad padding value, we'll return default 0 padding - maxVal = 0; - LOG_CAUGHT_EXCEPTION(); - } - - return winrt::box_value(maxVal); - } - - Foundation::IInspectable PaddingConverter::ConvertBack(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - const auto padding{ winrt::unbox_value(value) }; - return winrt::box_value(winrt::to_hstring(padding)); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/PaddingConverter.h b/src/cascadia/TerminalSettingsEditor/PaddingConverter.h deleted file mode 100644 index c56dd6d9f..000000000 --- a/src/cascadia/TerminalSettingsEditor/PaddingConverter.h +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "PaddingConverter.g.h" -#include "../inc/cppwinrt_utils.h" - -DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, PaddingConverter); diff --git a/src/cascadia/TerminalSettingsEditor/PercentageConverter.cpp b/src/cascadia/TerminalSettingsEditor/PercentageConverter.cpp deleted file mode 100644 index b414532bf..000000000 --- a/src/cascadia/TerminalSettingsEditor/PercentageConverter.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "PercentageConverter.h" -#include "PercentageConverter.g.cpp" - -using namespace winrt::Windows; -using namespace winrt::Windows::UI::Xaml; - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - Foundation::IInspectable PercentageConverter::Convert(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /* parameter */, - hstring const& /* language */) - { - const auto decimal{ winrt::unbox_value(value) }; - const unsigned int number{ base::ClampMul(decimal, 100u) }; - return winrt::box_value(number); - } - - Foundation::IInspectable PercentageConverter::ConvertBack(Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& /* targetType */, - Foundation::IInspectable const& /*parameter*/, - hstring const& /* language */) - { - const auto number{ winrt::unbox_value(value) }; - const auto decimal{ base::ClampDiv(number, 100) }; - return winrt::box_value(decimal); - } -} diff --git a/src/cascadia/TerminalSettingsEditor/PercentageConverter.h b/src/cascadia/TerminalSettingsEditor/PercentageConverter.h deleted file mode 100644 index e3aa16bbf..000000000 --- a/src/cascadia/TerminalSettingsEditor/PercentageConverter.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "PercentageConverter.g.h" -#include "Utils.h" - -namespace winrt::Microsoft::Terminal::Settings::Editor::implementation -{ - struct PercentageConverter : PercentageConverterT - { - PercentageConverter() = default; - - Windows::Foundation::IInspectable Convert(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - - Windows::Foundation::IInspectable ConvertBack(Windows::Foundation::IInspectable const& value, - Windows::UI::Xaml::Interop::TypeName const& targetType, - Windows::Foundation::IInspectable const& parameter, - hstring const& language); - }; -} - -namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation -{ - BASIC_FACTORY(PercentageConverter); -} diff --git a/src/cascadia/TerminalSettingsEditor/Profiles.h b/src/cascadia/TerminalSettingsEditor/Profiles.h index c39786bfd..65a151cb0 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles.h @@ -20,6 +20,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation Model::TerminalSettings TermSettings() const; + void SetAcrylicOpacityPercentageValue(double value) + { + AcrylicOpacity(winrt::Microsoft::Terminal::Settings::Editor::Converters::PercentageValueToPercentage(value)); + }; + + void SetPadding(double value) + { + Padding(to_hstring(value)); + } + // starting directory bool UseParentProcessDirectory(); void UseParentProcessDirectory(const bool useParent); diff --git a/src/cascadia/TerminalSettingsEditor/Profiles.idl b/src/cascadia/TerminalSettingsEditor/Profiles.idl index ff8fc6565..4ecc48899 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles.idl +++ b/src/cascadia/TerminalSettingsEditor/Profiles.idl @@ -19,6 +19,9 @@ namespace Microsoft.Terminal.Settings.Editor Windows.Foundation.Collections.IObservableVector MonospaceFontList { get; }; Microsoft.Terminal.Settings.Model.TerminalSettings TermSettings { get; }; + void SetAcrylicOpacityPercentageValue(Double value); + void SetPadding(Double value); + Boolean CanDeleteProfile { get; }; Boolean IsBaseLayer; Boolean UseParentProcessDirectory; @@ -105,5 +108,7 @@ namespace Microsoft.Terminal.Settings.Editor IInspectable CurrentScrollState; Windows.Foundation.Collections.IObservableVector ScrollStateList { get; }; + + Windows.UI.Xaml.Controls.Slider AcrylicOpacitySlider { get; }; } } diff --git a/src/cascadia/TerminalSettingsEditor/Profiles.xaml b/src/cascadia/TerminalSettingsEditor/Profiles.xaml index e2fb38987..763532256 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles.xaml +++ b/src/cascadia/TerminalSettingsEditor/Profiles.xaml @@ -33,17 +33,6 @@ - - - - - - - - - - - @@ -78,7 +67,7 @@ --> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(State.Profile.IsBaseLayer), Mode=OneWay}"> @@ -90,7 +79,7 @@ ClearSettingValue="{x:Bind State.Profile.ClearCommandline}" HasSettingValue="{x:Bind State.Profile.HasCommandline, Mode=OneWay}" SettingOverrideSource="{x:Bind State.Profile.CommandlineOverrideSource, Mode=OneWay}" - Visibility="{x:Bind State.Profile.IsBaseLayer, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}"> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(State.Profile.IsBaseLayer), Mode=OneWay}"> @@ -150,7 +139,7 @@ + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(State.Profile.IsBaseLayer), Mode=OneWay}"> @@ -280,10 +269,10 @@ + Value="{x:Bind local:Converters.PercentageToPercentageValue(State.Profile.AcrylicOpacity), BindBack=State.Profile.SetAcrylicOpacityPercentageValue, Mode=TwoWay}" /> + Text="{x:Bind local:Converters.AppendPercentageSign(AcrylicOpacitySlider.Value), Mode=OneWay}" /> @@ -307,7 +296,7 @@ + Value="{x:Bind local:Converters.MaxValueFromPaddingString(State.Profile.Padding), BindBack=State.Profile.SetPadding, Mode=TwoWay}" /> @@ -333,7 +322,7 @@ Margin="32,0,0,0" Click="CreateUnfocusedAppearance_Click" Style="{StaticResource BaseButtonStyle}" - Visibility="{x:Bind State.Profile.HasUnfocusedAppearance, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}"> + Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(State.Profile.HasUnfocusedAppearance), Mode=OneWay}"> diff --git a/src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml b/src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml index ec1a685fb..eef40cd8d 100644 --- a/src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml +++ b/src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml @@ -17,9 +17,6 @@ - - - the index returned by _BitScanForward must be divided by 2. const auto haystack1 = _mm_loadu_si128(reinterpret_cast(colorTable.data() + 0)); const auto haystack2 = _mm_loadu_si128(reinterpret_cast(colorTable.data() + 4)); const auto needle = _mm_set1_epi32(__builtin_bit_cast(int, defaultColor)); const auto result1 = _mm_cmpeq_epi32(haystack1, needle); const auto result2 = _mm_cmpeq_epi32(haystack2, needle); const auto result = _mm_packs_epi32(result1, result2); // 3.5 - const auto mask = _mm_movemask_ps(_mm_castsi128_ps(result)); + const auto mask = _mm_movemask_epi8(result); unsigned long index; - return _BitScanForward(&index, mask) ? til::at(colorTable, static_cast(index) + 8) : defaultColor; + return _BitScanForward(&index, mask) ? til::at(colorTable, static_cast(index / 2) + 8) : defaultColor; #else for (size_t i = 0; i < 8; i++) { From 2eb659717c0941626610faa4f3326f7f6a309a94 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 4 Aug 2021 10:00:41 -0700 Subject: [PATCH 04/16] Move to 1ES engineering pools (#10854) Move to 1ES engineering pools ## PR Checklist * [x] Closes #10734 * [x] I work here * [x] If the builds still work, the tests pass. (release and PR builds...) ## Validation Steps Performed - [x] Run the builds associated with this PR - [x] Force run a release build off this branch - [x] Force run a PGO training build off this branch --- build/pipelines/release.yml | 19 +++++++------------ .../templates/build-console-audit-job.yml | 9 ++++++--- .../pipelines/templates/build-console-ci.yml | 9 ++++++--- .../pipelines/templates/build-console-pgo.yml | 9 ++++++--- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/build/pipelines/release.yml b/build/pipelines/release.yml index ff6d49cf7..e200db166 100644 --- a/build/pipelines/release.yml +++ b/build/pipelines/release.yml @@ -2,8 +2,9 @@ trigger: none pr: none -pool: - name: Package ES Standard Build +pool: + name: WinDevPool-L + demands: ImageOverride -equals WinDevVS16-latest parameters: - name: branding @@ -70,11 +71,9 @@ jobs: clean: true submodules: true persistCredentials: True - - task: PkgESSetupBuild@10 + - task: PkgESSetupBuild@12 displayName: Package ES - Setup Build inputs: - useDfs: false - productName: OpenConsole disableOutputRedirect: true - task: PowerShell@2 displayName: Rationalize Build Platform @@ -275,11 +274,9 @@ jobs: clean: true submodules: true persistCredentials: True - - task: PkgESSetupBuild@10 + - task: PkgESSetupBuild@12 displayName: Package ES - Setup Build inputs: - useDfs: false - productName: OpenConsole disableOutputRedirect: true - task: DownloadBuildArtifacts@0 displayName: Download Artifacts (*.appx, *.msix) @@ -354,11 +351,9 @@ jobs: clean: true submodules: true persistCredentials: True - - task: PkgESSetupBuild@10 + - task: PkgESSetupBuild@12 displayName: Package ES - Setup Build inputs: - useDfs: false - productName: OpenConsole disableOutputRedirect: true - task: DownloadBuildArtifacts@0 displayName: Download x86 PublicTerminalCore @@ -480,7 +475,7 @@ jobs: mv Microsoft.WindowsTerminal_8wekyb3d8bbwe.msixbundle .\WindowsTerminal.app\ workingDirectory: $(System.ArtifactsDirectory)\appxbundle-signed - - task: PkgESVPack@10 + - task: PkgESVPack@12 displayName: 'Package ES - VPack' env: SYSTEM_ACCESSTOKEN: $(System.AccessToken) diff --git a/build/pipelines/templates/build-console-audit-job.yml b/build/pipelines/templates/build-console-audit-job.yml index 1c9a90d6e..2f1283f34 100644 --- a/build/pipelines/templates/build-console-audit-job.yml +++ b/build/pipelines/templates/build-console-audit-job.yml @@ -8,9 +8,12 @@ jobs: variables: BuildConfiguration: AuditMode BuildPlatform: ${{ parameters.platform }} - pool: "windevbuildagents" - # The public pool is also an option! - # pool: { vmImage: windows-2019 } + pool: + ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPoolOSS-L + ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPool-L + demands: ImageOverride -equals WinDevVS16-latest steps: - checkout: self diff --git a/build/pipelines/templates/build-console-ci.yml b/build/pipelines/templates/build-console-ci.yml index 0b9ce8685..0ff8b6b54 100644 --- a/build/pipelines/templates/build-console-ci.yml +++ b/build/pipelines/templates/build-console-ci.yml @@ -11,9 +11,12 @@ jobs: variables: BuildConfiguration: ${{ parameters.configuration }} BuildPlatform: ${{ parameters.platform }} - pool: "windevbuildagents" - # The public pool is also an option! - # pool: { vmImage: windows-2019 } + pool: + ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPoolOSS-L + ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPool-L + demands: ImageOverride -equals WinDevVS16-latest steps: - template: build-console-steps.yml diff --git a/build/pipelines/templates/build-console-pgo.yml b/build/pipelines/templates/build-console-pgo.yml index 8af4ec5d8..1e33e82c8 100644 --- a/build/pipelines/templates/build-console-pgo.yml +++ b/build/pipelines/templates/build-console-pgo.yml @@ -12,9 +12,12 @@ jobs: BuildConfiguration: ${{ parameters.configuration }} BuildPlatform: ${{ parameters.platform }} PGOBuildMode: 'Instrument' - pool: "windevbuildagents" - # The public pool is also an option! - # pool: { vmImage: windows-2019 } + pool: + ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPoolOSS-L + ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: + name: WinDevPool-L + demands: ImageOverride -equals WinDevVS16-latest steps: - template: build-console-steps.yml From 2bd46701005609a500a6e68b6089b4c8264a3006 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 5 Aug 2021 14:08:51 +0100 Subject: [PATCH 05/16] Fix a use-after-free crash when returning from the alt buffer (#10878) ## Summary of the Pull Request When switching from the alt buffer back to the main buffer, we need to copy certain cursor attributes from the one to the other. However, this copying was taking place after the alt buffer had been freed, and thus could result in the app crashing. This PR simply moves that code up a bit so it's prior to the buffer being freed. ## References PR #10843 added the code that introduced this problem. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Validation Steps Performed I was able to reproduce the crash when using a debug build, and confirmed that the crash no longer occurred after this PR was applied. I also checked that the cursor attributes were still being correctly copied back when returning from the alt buffer. --- src/host/screenInfo.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 2506bf450..5323cb574 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -2002,8 +2002,6 @@ void SCREEN_INFORMATION::UseMainScreenBuffer() SCREEN_INFORMATION* psiAlt = psiMain->_psiAlternateBuffer; psiMain->_psiAlternateBuffer = nullptr; - s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer - // deleting the alt buffer will give the GetSet back to its main // Copy the alt buffer's cursor style and visibility back to the main buffer. const auto& altCursor = psiAlt->GetTextBuffer().GetCursor(); @@ -2012,6 +2010,9 @@ void SCREEN_INFORMATION::UseMainScreenBuffer() mainCursor.SetIsVisible(altCursor.IsVisible()); mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed()); + s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer + // deleting the alt buffer will give the GetSet back to its main + // Tell the VT MouseInput handler that we're in the main buffer now gci.GetActiveInputBuffer()->GetTerminalInput().UseMainScreenBuffer(); } From 0b4839d94d4429a34275bc4aafb96484791ec5f1 Mon Sep 17 00:00:00 2001 From: Kayla Cinnamon Date: Thu, 5 Aug 2021 06:46:24 -0700 Subject: [PATCH 06/16] Add Split Tab option to tab context menu (#10832) ## Summary of the Pull Request Adds the Split Tab option to the tab context menu. Clicking this option will `auto` split the active pane of the tab into a duplicate pane. Clicking on an unfocused tab and splitting it will bring that tab into focus and split its active pane. We could make this a flyout from the context menu to let people choose horizontal/vertical split in the future if it's requested. I'm also wondering if this should be called Split Pane instead of Split Tab? ## References #1912 ## PR Checklist * [x] Closes #5025 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments https://user-images.githubusercontent.com/48369326/127691919-aae4683a-212a-4525-a0eb-a61c877461ed.mp4 ## Validation Steps Performed --- .../Resources/en-US/Resources.resw | 5 ++- src/cascadia/TerminalApp/TabManagement.cpp | 24 ++++++++++++ src/cascadia/TerminalApp/TerminalPage.cpp | 39 ++++++++++++++++--- src/cascadia/TerminalApp/TerminalPage.h | 7 ++++ src/cascadia/TerminalApp/TerminalTab.cpp | 19 +++++++++ src/cascadia/TerminalApp/TerminalTab.h | 1 + 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 9fa780a3a..5f325b1a6 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -662,10 +662,13 @@ Open a new tab in given starting directory + + Split Tab + Open a new window with given starting directory Split the window and start in given directory - \ No newline at end of file + diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index dd3a7aa7c..b93f50cab 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -192,6 +192,16 @@ namespace winrt::TerminalApp::implementation } }); + newTabImpl->SplitTabRequested([weakTab, weakThis{ get_weak() }]() { + auto page{ weakThis.get() }; + auto tab{ weakTab.get() }; + + if (page && tab) + { + page->_SplitTab(*tab); + } + }); + auto tabViewItem = newTabImpl->TabViewItem(); _tabView.TabItems().Append(tabViewItem); @@ -357,6 +367,20 @@ namespace winrt::TerminalApp::implementation CATCH_LOG(); } + // Method Description: + // - Sets the specified tab as the focused tab and splits its active pane + // Arguments: + // - tab: tab to split + void TerminalPage::_SplitTab(TerminalTab& tab) + { + try + { + _SetFocusedTab(tab); + _SplitPane(tab, SplitState::Automatic, SplitType::Duplicate); + } + CATCH_LOG(); + } + // Method Description: // - Removes the tab (both TerminalControl and XAML) after prompting for approval // Arguments: diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 281189920..96e5f38e0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1238,6 +1238,33 @@ namespace winrt::TerminalApp::implementation return; } + _SplitPane(*focusedTab, splitType, splitMode, splitSize, newTerminalArgs); + } + + // Method Description: + // - Split the focused pane of the given tab, either horizontally or vertically, and place the + // given TermControl into the newly created pane. + // - If splitType == SplitState::None, this method does nothing. + // Arguments: + // - tab: The tab that is going to be split. + // - splitType: one value from the TerminalApp::SplitState enum, indicating how the + // new pane should be split from its parent. + // - splitMode: value from TerminalApp::SplitType enum, indicating the profile to be used in the newly split pane. + // - newTerminalArgs: An object that may contain a blob of parameters to + // control which profile is created and with possible other + // configurations. See CascadiaSettings::BuildSettings for more details. + void TerminalPage::_SplitPane(TerminalTab& tab, + const SplitState splitType, + const SplitType splitMode, + const float splitSize, + const NewTerminalArgs& newTerminalArgs) + { + // Do nothing if we're requesting no split. + if (splitType == SplitState::None) + { + return; + } + try { TerminalSettingsCreateResult controlSettings{ nullptr }; @@ -1246,12 +1273,12 @@ namespace winrt::TerminalApp::implementation if (splitMode == SplitType::Duplicate) { - std::optional current_guid = focusedTab->GetFocusedProfile(); + std::optional current_guid = tab.GetFocusedProfile(); if (current_guid) { profileFound = true; controlSettings = TerminalSettings::CreateWithProfileByID(_settings, current_guid.value(), *_bindings); - const auto workingDirectory = focusedTab->GetActiveTerminalControl().WorkingDirectory(); + const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory(); const auto validWorkingDirectory = !workingDirectory.empty(); if (validWorkingDirectory) { @@ -1287,10 +1314,10 @@ namespace winrt::TerminalApp::implementation auto realSplitType = splitType; if (realSplitType == SplitState::Automatic) { - realSplitType = focusedTab->PreCalculateAutoSplit(availableSpace); + realSplitType = tab.PreCalculateAutoSplit(availableSpace); } - const auto canSplit = focusedTab->PreCalculateCanSplit(realSplitType, splitSize, availableSpace); + const auto canSplit = tab.PreCalculateCanSplit(realSplitType, splitSize, availableSpace); if (!canSplit) { return; @@ -1299,11 +1326,11 @@ namespace winrt::TerminalApp::implementation auto newControl = _InitControl(controlSettings, controlConnection); // Hookup our event handlers to the new terminal - _RegisterTerminalEvents(newControl, *focusedTab); + _RegisterTerminalEvents(newControl, tab); _UnZoomIfNeeded(); - focusedTab->SplitPane(realSplitType, splitSize, realGuid, newControl); + tab.SplitPane(realSplitType, splitSize, realGuid, newControl); } CATCH_LOG(); } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index aa2db9529..d6476b5ad 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -219,6 +219,8 @@ namespace winrt::TerminalApp::implementation void _DuplicateFocusedTab(); void _DuplicateTab(const TerminalTab& tab); + void _SplitTab(TerminalTab& tab); + winrt::Windows::Foundation::IAsyncAction _HandleCloseTabRequested(winrt::TerminalApp::TabBase tab); void _CloseTabAtIndex(uint32_t index); void _RemoveTab(const winrt::TerminalApp::TabBase& tab); @@ -254,6 +256,11 @@ namespace winrt::TerminalApp::implementation const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual, const float splitSize = 0.5f, const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr); + void _SplitPane(TerminalTab& tab, + const Microsoft::Terminal::Settings::Model::SplitState splitType, + const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual, + const float splitSize = 0.5f, + const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr); void _ResizePane(const Microsoft::Terminal::Settings::Model::ResizeDirection& direction); void _ToggleSplitOrientation(); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 4e420c5c2..ad4595a1b 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -936,12 +936,30 @@ namespace winrt::TerminalApp::implementation duplicateTabMenuItem.Icon(duplicateTabSymbol); } + Controls::MenuFlyoutItem splitTabMenuItem; + { + // "Split Tab" + Controls::FontIcon splitTabSymbol; + splitTabSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" }); + splitTabSymbol.Glyph(L"\xF246"); // ViewDashboard + + splitTabMenuItem.Click([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_SplitTabRequestedHandlers(); + } + }); + splitTabMenuItem.Text(RS_(L"SplitTabText")); + splitTabMenuItem.Icon(splitTabSymbol); + } + // Build the menu Controls::MenuFlyout contextMenuFlyout; Controls::MenuFlyoutSeparator menuSeparator; contextMenuFlyout.Items().Append(chooseColorMenuItem); contextMenuFlyout.Items().Append(renameTabMenuItem); contextMenuFlyout.Items().Append(duplicateTabMenuItem); + contextMenuFlyout.Items().Append(splitTabMenuItem); contextMenuFlyout.Items().Append(menuSeparator); // GH#5750 - When the context menu is dismissed with ESC, toss the focus @@ -1315,4 +1333,5 @@ namespace winrt::TerminalApp::implementation DEFINE_EVENT(TerminalTab, ColorCleared, _colorCleared, winrt::delegate<>); DEFINE_EVENT(TerminalTab, TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>); DEFINE_EVENT(TerminalTab, DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>); + DEFINE_EVENT(TerminalTab, SplitTabRequested, _SplitTabRequestedHandlers, winrt::delegate<>); } diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index 3ef935364..f3cee7555 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -94,6 +94,7 @@ namespace winrt::TerminalApp::implementation DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>); DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>); DECLARE_EVENT(DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>); + DECLARE_EVENT(SplitTabRequested, _SplitTabRequestedHandlers, winrt::delegate<>); TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable); private: From 76793b1e3fd303d681016d019407c509e91575aa Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Thu, 5 Aug 2021 10:05:21 -0700 Subject: [PATCH 07/16] [DefApp] Move from Monarch multi instance servers to Peasant single instance servers (#10823) - Monarch no longer sets itself up as a `CTerminalHandoff` multi instance server by default - In fact, `CTerminalHandoff` will only ever be a single instance server - When COM needs a `CTerminalHandoff`, it launches `wt.exe -embedding`, which gets picked up by the Monarch and then gets handed off to itself/peasant depending on user settings. - Peasant now recognizes the `-embedding` commandline and will start a `CTerminalHandoff` single instance listener, and receives the connection into a new tab. Closes #10358 --- .../TerminalApp/AppActionHandlers.cpp | 4 +- src/cascadia/TerminalApp/AppLogic.cpp | 5 ++ src/cascadia/TerminalApp/TabManagement.cpp | 6 +- src/cascadia/TerminalApp/TerminalPage.cpp | 69 ++++++++++--------- src/cascadia/TerminalApp/TerminalPage.h | 4 +- .../TerminalConnection/CTerminalHandoff.cpp | 27 ++++++-- .../TerminalConnection/CTerminalHandoff.h | 6 +- src/cascadia/WindowsTerminal/AppHost.cpp | 3 - 8 files changed, 74 insertions(+), 50 deletions(-) diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 60401122c..f88017e1e 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -264,12 +264,12 @@ namespace winrt::TerminalApp::implementation { if (args == nullptr) { - _OpenNewTab(nullptr); + LOG_IF_FAILED(_OpenNewTab(nullptr)); args.Handled(true); } else if (const auto& realArgs = args.ActionArgs().try_as()) { - _OpenNewTab(realArgs.TerminalArgs()); + LOG_IF_FAILED(_OpenNewTab(realArgs.TerminalArgs())); args.Handled(true); } } diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 441fbd4de..e9350ef95 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -1229,6 +1229,11 @@ namespace winrt::TerminalApp::implementation auto actions = winrt::single_threaded_vector(std::move(appArgs.GetStartupActions())); _root->ProcessStartupActions(actions, false, cwd); + + if (appArgs.IsHandoffListener()) + { + _root->SetInboundListener(true); + } } // Return the result of parsing with commandline, though it may or may not be used. return result; diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index b93f50cab..dd9e9dc96 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -56,7 +56,7 @@ namespace winrt::TerminalApp::implementation // - existingConnection: An optional connection that is already established to a PTY // for this tab to host instead of creating one. // If not defined, the tab will create the connection. - void TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection) + HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection) try { const auto profileGuid{ _settings.GetProfileForArgs(newTerminalArgs) }; @@ -89,8 +89,10 @@ namespace winrt::TerminalApp::implementation TraceLoggingWideString(schemeName.data(), "SchemeName", "Color scheme set in the settings"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + + return S_OK; } - CATCH_LOG(); + CATCH_RETURN(); // Method Description: // - Creates a new tab with the given settings. If the tab bar is not being diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 96e5f38e0..3bbf27eab 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -19,6 +19,8 @@ #include "RenameWindowRequestedArgs.g.cpp" #include "../inc/WindowingBehavior.h" +#include + using namespace winrt; using namespace winrt::Windows::Foundation::Collections; using namespace winrt::Windows::UI::Xaml; @@ -348,34 +350,12 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::StartInboundListener(); } // If we failed to start the listener, it will throw. - // We should fail fast here or the Terminal will be in a very strange state. - // We only start the listener if the Terminal was started with the COM server - // `-Embedding` flag and we make no tabs as a result. - // Therefore, if the listener cannot start itself up to make that tab with - // the inbound connection that caused the COM activation in the first place... - // we would be left with an empty terminal frame with no tabs. - // Instead, crash out so COM sees the server die and things unwind - // without a weird empty frame window. + // We don't want to fail fast here because if a peasant has some trouble with + // starting the listener, we don't want it to crash and take all its tabs down + // with it. catch (...) { - // However, we cannot always fail fast because of MSFT:33501832. Sometimes the COM catalog - // tears the state between old and new versions and fails here for that reason. - // As we're always becoming an inbound server in the monarch, even when COM didn't strictly - // ask us yet...we might just crash always. - // Instead... we're going to differentiate. If COM started us... we will fail fast - // so it sees the process die and falls back. - // If we were just starting normally as a Monarch and opportunistically listening for - // inbound connections... then we'll just log the failure and move on assuming - // the version state is torn and will fix itself whenever the packaging upgrade - // tasks decide to clean up. - if (_isEmbeddingInboundListener) - { - FAIL_FAST_CAUGHT_EXCEPTION(); - } - else - { - LOG_CAUGHT_EXCEPTION(); - } + LOG_CAUGHT_EXCEPTION(); } } } @@ -759,7 +739,7 @@ namespace winrt::TerminalApp::implementation } else { - this->_OpenNewTab(newTerminalArgs); + LOG_IF_FAILED(this->_OpenNewTab(newTerminalArgs)); } } @@ -2405,13 +2385,38 @@ namespace winrt::TerminalApp::implementation return _isAlwaysOnTop; } - void TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection) + HRESULT TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection) { - // TODO: GH 9458 will give us more context so we can try to choose a better profile. - _OpenNewTab(nullptr, connection); + // We need to be on the UI thread in order for _OpenNewTab to run successfully. + // HasThreadAccess will return true if we're currently on a UI thread and false otherwise. + // When we're on a COM thread, we'll need to dispatch the calls to the UI thread + // and wait on it hence the locking mechanism. + if (Dispatcher().HasThreadAccess()) + { + // TODO: GH 9458 will give us more context so we can try to choose a better profile. + auto hr = _OpenNewTab(nullptr, connection); - // Request a summon of this window to the foreground - _SummonWindowRequestedHandlers(*this, nullptr); + // Request a summon of this window to the foreground + _SummonWindowRequestedHandlers(*this, nullptr); + + return hr; + } + else + { + til::latch latch{ 1 }; + HRESULT finalVal = S_OK; + + Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [&]() { + finalVal = _OpenNewTab(nullptr, connection); + + _SummonWindowRequestedHandlers(*this, nullptr); + + latch.count_down(); + }); + + latch.wait(); + return finalVal; + } } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index d6476b5ad..bb1a6f13e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -188,7 +188,7 @@ namespace winrt::TerminalApp::implementation void _CreateNewTabFlyout(); void _OpenNewTabDropdown(); - void _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr); + HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr); void _CreateNewTabFromSettings(GUID profileGuid, const Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr); winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(GUID profileGuid, Microsoft::Terminal::Settings::Model::TerminalSettings settings); @@ -344,7 +344,7 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::Settings::Model::Command _lastPreviewedCommand{ nullptr }; winrt::Microsoft::Terminal::Settings::Model::TerminalSettings _originalSettings{ nullptr }; - void _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection); + HRESULT _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection); void _HandleToggleInboundPty(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args); void _WindowRenamerActionClick(const IInspectable& sender, const IInspectable& eventArgs); diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp index 0138921fd..5023a8188 100644 --- a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp +++ b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp @@ -11,6 +11,8 @@ using namespace Microsoft::WRL; static NewHandoffFunction _pfnHandoff = nullptr; // The registration ID of the class object for clean up later static DWORD g_cTerminalHandoffRegistration = 0; +// Mutex so we only do start/stop/establish one at a time. +static std::shared_mutex _mtx; // Routine Description: // - Starts listening for TerminalHandoff requests by registering @@ -19,9 +21,11 @@ static DWORD g_cTerminalHandoffRegistration = 0; // - pfnHandoff - Function to callback when a handoff is received // Return Value: // - S_OK, E_NOT_VALID_STATE (start called when already started) or relevant COM registration error. -HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff) noexcept +HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff) try { + std::unique_lock lock{ _mtx }; + RETURN_HR_IF(E_NOT_VALID_STATE, _pfnHandoff != nullptr); const auto classFactory = Make>(); @@ -31,7 +35,7 @@ try ComPtr unk; RETURN_IF_FAILED(classFactory.As(&unk)); - RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_MULTIPLEUSE, &g_cTerminalHandoffRegistration)); + RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_SINGLEUSE, &g_cTerminalHandoffRegistration)); _pfnHandoff = pfnHandoff; @@ -46,8 +50,10 @@ CATCH_RETURN() // - // Return Value: // - S_OK, E_NOT_VALID_STATE (stop called when not started), or relevant COM class revoke error -HRESULT CTerminalHandoff::s_StopListening() noexcept +HRESULT CTerminalHandoff::s_StopListening() { + std::unique_lock lock{ _mtx }; + RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff); _pfnHandoff = nullptr; @@ -91,10 +97,19 @@ static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept // - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle` // error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error // from the registered handler event function. -HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept +HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) { + // Stash a local copy of _pfnHandoff before we stop listening. + auto localPfnHandoff = _pfnHandoff; + + // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call. + // COM does not automatically clean that up for us. We must do it. + s_StopListening(); + + std::unique_lock lock{ _mtx }; + // Report an error if no one registered a handoff function before calling this. - RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff); + RETURN_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff); // Duplicate the handles from what we received. // The contract with COM specifies that any HANDLEs we receive from the caller belong @@ -108,5 +123,5 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign RETURN_IF_FAILED(_duplicateHandle(client, client)); // Call registered handler from when we started listening. - return _pfnHandoff(in, out, signal, ref, server, client); + return localPfnHandoff(in, out, signal, ref, server, client); } diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.h b/src/cascadia/TerminalConnection/CTerminalHandoff.h index 2e173e0a3..2ba644f41 100644 --- a/src/cascadia/TerminalConnection/CTerminalHandoff.h +++ b/src/cascadia/TerminalConnection/CTerminalHandoff.h @@ -37,12 +37,12 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff)) HANDLE signal, HANDLE ref, HANDLE server, - HANDLE client) noexcept override; + HANDLE client) override; #pragma endregion - static HRESULT s_StartListening(NewHandoffFunction pfnHandoff) noexcept; - static HRESULT s_StopListening() noexcept; + static HRESULT s_StartListening(NewHandoffFunction pfnHandoff); + static HRESULT s_StopListening(); }; // Disable warnings from the CoCreatableClass macro as the value it provides for diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 53e21ccdd..092fe8eae 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -656,9 +656,6 @@ void AppHost::_BecomeMonarch(const winrt::Windows::Foundation::IInspectable& /*s const winrt::Windows::Foundation::IInspectable& /*args*/) { _setupGlobalHotkeys(); - - // The monarch is just going to be THE listener for inbound connections. - _listenForInboundConnections(); } void AppHost::_listenForInboundConnections() From dcbf7c74f18a344ae255b62dbec31c2965a0433f Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 5 Aug 2021 23:33:44 +0200 Subject: [PATCH 08/16] Reload settings when the input method changes (#10876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `VkKeyScanW` as well as `MapVirtualKeyW` are used throughout the project, but are input method sensitive functions. Since #10666 `win+sc(41)` is used as the quake mode keybinding, which is then mapped to a virtual key in order to call `RegisterHotKey`. This mapping is highly dependent on the input method and the quake mode key binding will fail to work once the input method was changed. ## PR Checklist * [x] Closes #10729 * [x] I work here * [ ] Tests added/passed ## Validation Steps Performed * win+` opens quake window before & after changing keyboard layout ✔️ * keyboard layout changes while WT is minimized trigger reloaded ✔️ --- src/cascadia/TerminalApp/AppLogic.cpp | 4 ++ src/cascadia/TerminalApp/AppLogic.h | 16 ++++--- .../TerminalApp/LanguageProfileNotifier.cpp | 42 +++++++++++++++++++ .../TerminalApp/LanguageProfileNotifier.h | 21 ++++++++++ .../TerminalApp/TerminalAppLib.vcxproj | 12 +++--- .../TerminalAppLib.vcxproj.filters | 25 ++++------- src/cascadia/TerminalApp/pch.h | 1 + 7 files changed, 93 insertions(+), 28 deletions(-) create mode 100644 src/cascadia/TerminalApp/LanguageProfileNotifier.cpp create mode 100644 src/cascadia/TerminalApp/LanguageProfileNotifier.h diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index e9350ef95..a9ac10d8f 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -209,6 +209,10 @@ namespace winrt::TerminalApp::implementation self->_ReloadSettings(); } }); + + _languageProfileNotifier = winrt::make_self([this]() { + _reloadSettings->Run(); + }); } // Method Description: diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 3d7782039..6d2e36d9e 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -5,8 +5,9 @@ #include "AppLogic.g.h" #include "FindTargetWindowResult.g.h" -#include "TerminalPage.h" #include "Jumplist.h" +#include "LanguageProfileNotifier.h" +#include "TerminalPage.h" #include #include @@ -110,12 +111,8 @@ namespace winrt::TerminalApp::implementation // ALSO: If you add any UIElements as roots here, make sure they're // updated in _ApplyTheme. The root currently is _root. winrt::com_ptr _root{ nullptr }; - Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr }; - wil::unique_folder_change_reader_nothrow _reader; - std::shared_ptr> _reloadSettings; - til::throttled_func_trailing<> _reloadState; winrt::hstring _settingsLoadExceptionText; HRESULT _settingsLoadedResult = S_OK; bool _loadedInitialSettings = false; @@ -124,6 +121,15 @@ namespace winrt::TerminalApp::implementation ::TerminalApp::AppCommandlineArgs _appArgs; ::TerminalApp::AppCommandlineArgs _settingsAppArgs; + + std::shared_ptr> _reloadSettings; + til::throttled_func_trailing<> _reloadState; + + // These fields invoke _reloadSettings and must be destroyed before _reloadSettings. + // (C++ destroys members in reverse-declaration-order.) + winrt::com_ptr _languageProfileNotifier; + wil::unique_folder_change_reader_nothrow _reader; + static TerminalApp::FindTargetWindowResult _doFindTargetWindow(winrt::array_view args, const Microsoft::Terminal::Settings::Model::WindowingMode& windowingBehavior); diff --git a/src/cascadia/TerminalApp/LanguageProfileNotifier.cpp b/src/cascadia/TerminalApp/LanguageProfileNotifier.cpp new file mode 100644 index 000000000..9655947a4 --- /dev/null +++ b/src/cascadia/TerminalApp/LanguageProfileNotifier.cpp @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "pch.h" +#include "LanguageProfileNotifier.h" + +using namespace winrt::TerminalApp::implementation; + +LanguageProfileNotifier::LanguageProfileNotifier(std::function&& callback) : + _callback{ std::move(callback) }, + _currentKeyboardLayout{ GetKeyboardLayout(0) } +{ + const auto manager = wil::CoCreateInstance(CLSID_TF_ThreadMgr); + _source = manager.query(); + if (FAILED(_source->AdviseSink(IID_ITfInputProcessorProfileActivationSink, static_cast(this), &_cookie))) + { + _cookie = TF_INVALID_COOKIE; + THROW_LAST_ERROR(); + } +} + +LanguageProfileNotifier::~LanguageProfileNotifier() +{ + if (_cookie != TF_INVALID_COOKIE) + { + _source->UnadviseSink(_cookie); + } +} + +STDMETHODIMP LanguageProfileNotifier::OnActivated(DWORD /*dwProfileType*/, LANGID /*langid*/, REFCLSID /*clsid*/, REFGUID /*catid*/, REFGUID /*guidProfile*/, HKL hkl, DWORD /*dwFlags*/) +{ + if (hkl && hkl != _currentKeyboardLayout) + { + _currentKeyboardLayout = hkl; + try + { + _callback(); + } + CATCH_RETURN(); + } + return S_OK; +} diff --git a/src/cascadia/TerminalApp/LanguageProfileNotifier.h b/src/cascadia/TerminalApp/LanguageProfileNotifier.h new file mode 100644 index 000000000..865330455 --- /dev/null +++ b/src/cascadia/TerminalApp/LanguageProfileNotifier.h @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +namespace winrt::TerminalApp::implementation +{ + class LanguageProfileNotifier : public winrt::implements + { + public: + explicit LanguageProfileNotifier(std::function&& callback); + ~LanguageProfileNotifier(); + STDMETHODIMP OnActivated(DWORD dwProfileType, LANGID langid, REFCLSID clsid, REFGUID catid, REFGUID guidProfile, HKL hkl, DWORD dwFlags); + + private: + std::function _callback; + wil::com_ptr _source; + DWORD _cookie = TF_INVALID_COOKIE; + HKL _currentKeyboardLayout; + }; +} diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj index 67317f4e4..25cd7cfa3 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj @@ -53,7 +53,7 @@ Designer - + Designer @@ -74,6 +74,7 @@ + MinMaxCloseControl.xaml @@ -112,7 +113,7 @@ HighlightedTextControl.xaml - + ColorPickupFlyout.xaml @@ -139,7 +140,7 @@ AppLogic.idl - + @@ -149,6 +150,7 @@ + MinMaxCloseControl.xaml @@ -195,7 +197,7 @@ HighlightedTextControl.xaml - + ColorPickupFlyout.xaml @@ -280,7 +282,7 @@ HighlightedTextControl.xaml Code - + ColorPickupFlyout.xaml Code diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters index 1511e14d9..f041b68d7 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters @@ -13,9 +13,6 @@ pane - - tab - pane @@ -23,7 +20,6 @@ - tab @@ -49,10 +45,10 @@ highlightedText + + - - app @@ -60,9 +56,6 @@ pane - - tab - @@ -92,14 +85,13 @@ highlightedText + + app - - settings - settings @@ -107,9 +99,6 @@ settings - - tab - tab @@ -125,6 +114,9 @@ tab + + + @@ -160,9 +152,6 @@ commandPalette - - commandPalette - commandPalette diff --git a/src/cascadia/TerminalApp/pch.h b/src/cascadia/TerminalApp/pch.h index 299d7de63..1f84f821e 100644 --- a/src/cascadia/TerminalApp/pch.h +++ b/src/cascadia/TerminalApp/pch.h @@ -66,6 +66,7 @@ TRACELOGGING_DECLARE_PROVIDER(g_hTerminalAppProvider); #include #include +#include #include #include From 90ff261c35a1b3e50267576f5cbdc66733731101 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Fri, 6 Aug 2021 21:41:02 +0100 Subject: [PATCH 09/16] Add support for downloadable soft fonts (#10011) This PR adds conhost support for downloadable soft fonts - also known as dynamically redefinable character sets (DRCS) - using the `DECDLD` escape sequence. These fonts are typically designed to work on a specific terminal model, and each model tends to have a different character cell size. So in order to support as many models as possible, the code attempts to detect the original target size of the font, and then scale the glyphs to fit our current cell size. Once a font has been downloaded to the terminal, it can be designated in the same way you would a standard character set, using an `SCS` escape sequence. The identification string for the set is defined by the `DECDLD` sequence. Internally we map the characters in this set to code points `U+EF20` to `U+EF7F` in the Unicode private use are (PUA). Then in the renderer, any characters in that range are split off into separate runs, which get painted with a special font. The font itself is dynamically generated as an in-memory resource, constructed from the downloaded character bitmaps which have been scaled to the appropriate size. If no soft fonts are in use, then no mapping of the PUA code points will take place, so this shouldn't interfere with anyone using those code points for something else, as along as they aren't also trying to use soft fonts. I also tried to pick a PUA range that hadn't already been snatched up by Nerd Fonts, but if we do receive reports of a conflict, it's easy enough to change. ## Validation Steps Performed I added an adapter test that runs through a bunch of parameter variations for the `DECDLD` sequence, to make sure we're correctly detecting the font sizes for most of the known DEC terminal models. I've also tested manually on a wide range of existing fonts, of varying dimensions, and from multiple sources, and made sure they all worked reasonably well. Closes #9164 --- .github/actions/spelling/allow/apis.txt | 1 + .github/actions/spelling/expect/expect.txt | 14 + src/host/getset.cpp | 24 + src/host/getset.h | 4 + src/host/outputStream.cpp | 15 + src/host/outputStream.hpp | 4 + src/host/ut_host/VtRendererTests.cpp | 50 +- src/interactivity/onecore/BgfxEngine.cpp | 1 + src/interactivity/onecore/BgfxEngine.hpp | 1 + src/renderer/base/FontResource.cpp | 285 +++++++++ src/renderer/base/RenderEngineBase.cpp | 7 + src/renderer/base/lib/base.vcxproj | 2 + src/renderer/base/lib/base.vcxproj.filters | 6 + src/renderer/base/renderer.cpp | 52 +- src/renderer/base/renderer.hpp | 13 +- src/renderer/base/sources.inc | 1 + src/renderer/dx/DxRenderer.cpp | 2 + src/renderer/dx/DxRenderer.hpp | 1 + src/renderer/gdi/gdirenderer.hpp | 15 +- src/renderer/gdi/paint.cpp | 4 + src/renderer/gdi/state.cpp | 63 +- src/renderer/inc/FontResource.hpp | 45 ++ src/renderer/inc/IRenderEngine.hpp | 4 + src/renderer/inc/IRenderer.hpp | 4 + src/renderer/inc/RenderEngineBase.hpp | 4 + src/renderer/uia/UiaRenderer.cpp | 2 + src/renderer/uia/UiaRenderer.hpp | 1 + src/renderer/vt/Xterm256Engine.cpp | 2 + src/renderer/vt/Xterm256Engine.hpp | 1 + src/renderer/vt/XtermEngine.cpp | 2 + src/renderer/vt/XtermEngine.hpp | 1 + src/renderer/vt/vtrenderer.hpp | 1 + src/renderer/wddmcon/WddmConRenderer.cpp | 1 + src/renderer/wddmcon/WddmConRenderer.hpp | 1 + src/terminal/adapter/DispatchTypes.hpp | 40 ++ src/terminal/adapter/FontBuffer.cpp | 603 ++++++++++++++++++ src/terminal/adapter/FontBuffer.hpp | 95 +++ src/terminal/adapter/ITermDispatch.hpp | 11 + src/terminal/adapter/adaptDispatch.cpp | 87 +++ src/terminal/adapter/adaptDispatch.hpp | 11 + src/terminal/adapter/charsets.hpp | 11 + src/terminal/adapter/conGetSet.hpp | 4 + src/terminal/adapter/lib/adapter.vcxproj | 2 + .../adapter/lib/adapter.vcxproj.filters | 6 + src/terminal/adapter/sources.inc | 1 + src/terminal/adapter/termDispatch.hpp | 9 + src/terminal/adapter/terminalOutput.cpp | 234 ++++--- src/terminal/adapter/terminalOutput.hpp | 7 + .../adapter/ut_adapter/adapterTest.cpp | 238 +++++++ .../parser/OutputStateMachineEngine.cpp | 19 +- .../parser/OutputStateMachineEngine.hpp | 7 +- 51 files changed, 1903 insertions(+), 116 deletions(-) create mode 100644 src/renderer/base/FontResource.cpp create mode 100644 src/renderer/inc/FontResource.hpp create mode 100644 src/terminal/adapter/FontBuffer.cpp create mode 100644 src/terminal/adapter/FontBuffer.hpp diff --git a/.github/actions/spelling/allow/apis.txt b/.github/actions/spelling/allow/apis.txt index 87d479f7f..85121e065 100644 --- a/.github/actions/spelling/allow/apis.txt +++ b/.github/actions/spelling/allow/apis.txt @@ -133,6 +133,7 @@ SRWLOCK STDCPP STDMETHOD strchr +strcpy streambuf strtoul Stubless diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 94f8fc911..05ccc895a 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -169,6 +169,7 @@ brandings BRK Browsable bsearch +Bspace bstr BTNFACE buf @@ -270,10 +271,12 @@ cmder CMDEXT Cmdlet cmdline +cmh CMOUSEBUTTONS cmp cmpeq cmt +cmw cmyk CNL cnt @@ -406,11 +409,13 @@ csbiex csharp CSHORT CSIDL +Cspace csproj Csr csrmsg CSRSS csrutil +css cstdarg cstddef cstdio @@ -509,6 +514,7 @@ DECAWM DECCKM DECCOLM DECDHL +DECDLD DECDWL DECEKBD DECID @@ -789,6 +795,7 @@ FONTENUMPROC FONTFACE FONTFAMILY FONTHEIGHT +FONTINFO fontlist FONTOK FONTSIZE @@ -902,6 +909,7 @@ github gitlab gle globals +GLYPHENTRY gmail GMEM GNUC @@ -950,6 +958,7 @@ hdrstop HEIGHTSCROLL hfile hfont +hfontresource hglobal hhh HHmm @@ -1272,6 +1281,7 @@ locsrc locstudio Loewen LOGFONT +LOGFONTA LOGFONTW logissue lowercased @@ -1935,6 +1945,7 @@ realloc reamapping rects redef +redefinable Redir redirector redist @@ -1980,6 +1991,7 @@ rfc rftp rgb rgba +RGBCOLOR rgbi rgci rgfae @@ -2149,6 +2161,7 @@ SIGDN SINGLEFLAG SINGLETHREADED siup +sixel SIZEBOX sizeof SIZESCROLL @@ -2754,6 +2767,7 @@ WTo wtof wtoi WTs +WTSOFTFONT wtw wtypes Wubi diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 6e9e70191..9c3ccde44 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1639,6 +1639,30 @@ void DoSrvEndHyperlink(SCREEN_INFORMATION& screenInfo) screenInfo.GetTextBuffer().SetCurrentAttributes(attr); } +// Routine Description: +// - A private API call for updating the active soft font. +// Arguments: +// - bitPattern - An array of scanlines representing all the glyphs in the font. +// - cellSize - The cell size for an individual glyph. +// - centeringHint - The horizontal extent that glyphs are offset from center. +// Return Value: +// - S_OK if we succeeded, otherwise the HRESULT of the failure. +[[nodiscard]] HRESULT DoSrvUpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept +{ + try + { + auto* pRender = ServiceLocator::LocateGlobals().pRender; + if (pRender) + { + pRender->UpdateSoftFont(bitPattern, cellSize, centeringHint); + } + return S_OK; + } + CATCH_RETURN(); +} + // Routine Description: // - A private API call for forcing the renderer to repaint the screen. If the // input screen buffer is not the active one, then just do nothing. We only diff --git a/src/host/getset.h b/src/host/getset.h index 43b8d5fef..1fb48e286 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -55,6 +55,10 @@ void DoSrvAddHyperlink(SCREEN_INFORMATION& screenInfo, void DoSrvEndHyperlink(SCREEN_INFORMATION& screenInfo); +[[nodiscard]] HRESULT DoSrvUpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept; + void DoSrvPrivateRefreshWindow(const SCREEN_INFORMATION& screenInfo); [[nodiscard]] HRESULT DoSrvSetConsoleOutputCodePage(const unsigned int codepage); diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 3129f0a1a..a70b48afd 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -818,3 +818,18 @@ bool ConhostInternalGetSet::PrivateEndHyperlink() const DoSrvEndHyperlink(_io.GetActiveOutputBuffer()); return true; } + +// Routine Description: +// - Replaces the active soft font with the given bit pattern. +// Arguments: +// - bitPattern - An array of scanlines representing all the glyphs in the font. +// - cellSize - The cell size for an individual glyph. +// - centeringHint - The horizontal extent that glyphs are offset from center. +// Return Value: +// - true if successful (see DoSrvUpdateSoftFont). false otherwise. +bool ConhostInternalGetSet::PrivateUpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept +{ + return SUCCEEDED(DoSrvUpdateSoftFont(bitPattern, cellSize, centeringHint)); +} diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index 34048e026..99889baca 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -147,6 +147,10 @@ public: bool PrivateAddHyperlink(const std::wstring_view uri, const std::wstring_view params) const override; bool PrivateEndHyperlink() const override; + bool PrivateUpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept override; + private: Microsoft::Console::IIoProvider& _io; }; diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index b17853805..a12dd8673 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -420,6 +420,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[48;2;5;6;7m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({ 0x00030201, 0x00070605 }, &renderData, + false, false)); TestPaint(*engine, [&]() { @@ -428,6 +429,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[48;2;7;8;9m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({ 0x00030201, 0x00090807 }, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -435,6 +437,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[38;2;10;11;12m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({ 0x000c0b0a, 0x00090807 }, &renderData, + false, false)); }); @@ -444,6 +447,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({ 0x000c0b0a, 0x00090807 }, &renderData, + false, false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); @@ -458,6 +462,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({}, &renderData, + false, false)); TestPaint(*engine, [&]() { @@ -469,6 +474,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -477,6 +483,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -485,6 +492,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[48;2;19;161;14m"); // Background RGB(19,161,14) VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -493,6 +501,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[38;2;193;156;0m"); // Foreground RGB(193,156,0) VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -501,6 +510,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[49m"); // Background default VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -509,6 +519,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[38;5;7m"); // Foreground DARK_WHITE (256-Color Index) VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -517,6 +528,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[48;5;1m"); // Background DARK_RED (256-Color Index) VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -525,6 +537,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[39m"); // Background default VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -533,6 +546,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back("\x1b[m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); }); @@ -542,6 +556,7 @@ void VtRendererTest::Xterm256TestColors() qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({}, &renderData, + false, false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); @@ -795,7 +810,7 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset() Log::Comment(L"----Start With All Attributes Reset----"); TextAttribute textAttributes = {}; qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); switch (renditionAttribute) { @@ -841,29 +856,29 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset() break; } qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Set Green Foreground----"); textAttributes.SetIndexedForeground(FOREGROUND_GREEN); qExpectedInput.push_back("\x1b[32m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Reset Default Foreground and Retain Rendition----"); textAttributes.SetDefaultForeground(); qExpectedInput.push_back("\x1b[m"); qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Set Green Background----"); textAttributes.SetIndexedBackground(FOREGROUND_GREEN); qExpectedInput.push_back("\x1b[42m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Reset Default Background and Retain Rendition----"); textAttributes.SetDefaultBackground(); qExpectedInput.push_back("\x1b[m"); qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); VerifyExpectedInputsDrained(); } @@ -1081,6 +1096,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({}, &renderData, + false, false)); TestPaint(*engine, [&]() { @@ -1092,6 +1108,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1100,6 +1117,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1108,6 +1126,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[42m"); // Background DARK_GREEN VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1116,6 +1135,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[33m"); // Foreground DARK_YELLOW VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1125,6 +1145,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[33m"); // Reapply foreground DARK_YELLOW VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1133,6 +1154,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1141,6 +1163,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1150,6 +1173,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[41m"); // Reapply background DARK_RED VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); Log::Comment(NoThrowString().Format( @@ -1158,6 +1182,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back("\x1b[m"); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, + false, false)); }); @@ -1167,6 +1192,7 @@ void VtRendererTest::XtermTestColors() qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes({}, &renderData, + false, false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); @@ -1304,7 +1330,7 @@ void VtRendererTest::XtermTestAttributesAcrossReset() Log::Comment(L"----Start With All Attributes Reset----"); TextAttribute textAttributes = {}; qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); switch (renditionAttribute) { @@ -1322,29 +1348,29 @@ void VtRendererTest::XtermTestAttributesAcrossReset() break; } qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Set Green Foreground----"); textAttributes.SetIndexedForeground(FOREGROUND_GREEN); qExpectedInput.push_back("\x1b[32m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Reset Default Foreground and Retain Rendition----"); textAttributes.SetDefaultForeground(); qExpectedInput.push_back("\x1b[m"); qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Set Green Background----"); textAttributes.SetIndexedBackground(FOREGROUND_GREEN); qExpectedInput.push_back("\x1b[42m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); Log::Comment(L"----Reset Default Background and Retain Rendition----"); textAttributes.SetDefaultBackground(); qExpectedInput.push_back("\x1b[m"); qExpectedInput.push_back(renditionSequence.str()); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(textAttributes, &renderData, false, false)); VerifyExpectedInputsDrained(); } diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index cf690ecd3..dcd3f923e 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -196,6 +196,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid [[nodiscard]] HRESULT BgfxEngine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null /*pData*/, + const bool /*usingSoftFont*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = textAttributes.GetLegacyAttributes(); diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 4fddcfb0b..2b4e787a3 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -60,6 +60,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/renderer/base/FontResource.cpp b/src/renderer/base/FontResource.cpp new file mode 100644 index 000000000..38263033a --- /dev/null +++ b/src/renderer/base/FontResource.cpp @@ -0,0 +1,285 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include "../inc/FontResource.hpp" + +using namespace Microsoft::Console::Render; + +namespace +{ + // The structures below are based on the Windows 3.0 font file format, which + // was documented in Microsoft Knowledge Base article Q65123. Although no + // longer hosted by Microsoft, it can still be found at the following URL: + // https://web.archive.org/web/20140820153410/http://support.microsoft.com/kb/65123 + + // For now we're only using fixed pitch single color fonts, but the rest + // of the flags are included here for completeness. + static constexpr DWORD DFF_FIXED = 0x0001; + static constexpr DWORD DFF_PROPORTIONAL = 0x0002; + static constexpr DWORD DFF_1COLOR = 0x0010; + static constexpr DWORD DFF_16COLOR = 0x0020; + static constexpr DWORD DFF_256COLOR = 0x0040; + static constexpr DWORD DFF_RGBCOLOR = 0x0080; + + // DRCS soft fonts only require 96 characters at most. + static constexpr size_t CHAR_COUNT = 96; + +#pragma pack(push, 1) + struct GLYPHENTRY + { + WORD geWidth; + DWORD geOffset; + }; + + struct FONTINFO + { + WORD dfVersion; + DWORD dfSize; + CHAR dfCopyright[60]; + WORD dfType; + WORD dfPoints; + WORD dfVertRes; + WORD dfHorizRes; + WORD dfAscent; + WORD dfInternalLeading; + WORD dfExternalLeading; + BYTE dfItalic; + BYTE dfUnderline; + BYTE dfStrikeOut; + WORD dfWeight; + BYTE dfCharSet; + WORD dfPixWidth; + WORD dfPixHeight; + BYTE dfPitchAndFamily; + WORD dfAvgWidth; + WORD dfMaxWidth; + BYTE dfFirstChar; + BYTE dfLastChar; + BYTE dfDefaultChar; + BYTE dfBreakChar; + WORD dfWidthBytes; + DWORD dfDevice; + DWORD dfFace; + DWORD dfBitsPointer; + DWORD dfBitsOffset; + BYTE dfReserved; + DWORD dfFlags; + WORD dfAspace; + WORD dfBspace; + WORD dfCspace; + DWORD dfColorPointer; + DWORD dfReserved1[4]; + GLYPHENTRY dfCharTable[CHAR_COUNT]; + CHAR szFaceName[LF_FACESIZE]; + }; +#pragma pack(pop) +} + +FontResource::FontResource(const gsl::span bitPattern, + const til::size sourceSize, + const til::size targetSize, + const size_t centeringHint) : + _bitPattern{ bitPattern.begin(), bitPattern.end() }, + _sourceSize{ sourceSize }, + _targetSize{ targetSize }, + _centeringHint{ centeringHint } +{ +} + +void FontResource::SetTargetSize(const til::size targetSize) +{ + if (_targetSize != targetSize) + { + _targetSize = targetSize; + _fontHandle = nullptr; + } +} + +FontResource::operator HFONT() +{ + if (!_fontHandle && !_bitPattern.empty()) + { + _regenerateFont(); + } + return _fontHandle.get(); +} + +void FontResource::_regenerateFont() +{ + const auto targetWidth = _targetSize.width(); + const auto targetHeight = _targetSize.height(); + const auto charSizeInBytes = (targetWidth + 7) / 8 * targetHeight; + + const DWORD fontBitmapSize = charSizeInBytes * CHAR_COUNT; + const DWORD fontResourceSize = sizeof(FONTINFO) + fontBitmapSize; + + auto fontResourceBuffer = std::vector(fontResourceSize); + void* fontResourceBufferPointer = fontResourceBuffer.data(); + auto& fontResource = *static_cast(fontResourceBufferPointer); + + fontResource.dfVersion = 0x300; + fontResource.dfSize = fontResourceSize; + fontResource.dfWeight = FW_NORMAL; + fontResource.dfCharSet = OEM_CHARSET; + fontResource.dfPixWidth = targetWidth; + fontResource.dfPixHeight = targetHeight; + fontResource.dfPitchAndFamily = FIXED_PITCH | FF_DONTCARE; + fontResource.dfAvgWidth = targetWidth; + fontResource.dfMaxWidth = targetWidth; + fontResource.dfFirstChar = L' '; + fontResource.dfLastChar = fontResource.dfFirstChar + CHAR_COUNT - 1; + fontResource.dfFace = offsetof(FONTINFO, szFaceName); + fontResource.dfBitsOffset = sizeof(FONTINFO); + fontResource.dfFlags = DFF_FIXED | DFF_1COLOR; + + // We use an atomic counter to create a locally-unique name for the font. + static std::atomic faceNameCounter; + sprintf_s(fontResource.szFaceName, "WTSOFTFONT%016llX", faceNameCounter++); + + // Each character has a fixed size and position in the font bitmap, but we + // still need to fill in the header table with that information. + for (auto i = 0u; i < std::size(fontResource.dfCharTable); i++) + { + const auto charOffset = fontResource.dfBitsOffset + charSizeInBytes * i; + fontResource.dfCharTable[i].geOffset = charOffset; + fontResource.dfCharTable[i].geWidth = targetWidth; + } + + // Raster fonts aren't generally scalable, so we need to resize the bit + // patterns for the character glyphs to the requested target size, and + // copy the results into the resource structure. + auto fontResourceSpan = gsl::span(fontResourceBuffer); + _resizeBitPattern(fontResourceSpan.subspan(fontResource.dfBitsOffset)); + + DWORD fontCount = 0; + _resourceHandle.reset(AddFontMemResourceEx(&fontResource, fontResourceSize, nullptr, &fontCount)); + LOG_HR_IF_NULL(E_FAIL, _resourceHandle.get()); + + // Once the resource has been registered, we should be able to create the + // font by using the same name and attributes as were set in the resource. + LOGFONTA logFont = {}; + logFont.lfHeight = fontResource.dfPixHeight; + logFont.lfWidth = fontResource.dfPixWidth; + logFont.lfCharSet = fontResource.dfCharSet; + logFont.lfOutPrecision = OUT_RASTER_PRECIS; + logFont.lfPitchAndFamily = fontResource.dfPitchAndFamily; + strcpy_s(logFont.lfFaceName, fontResource.szFaceName); + _fontHandle.reset(CreateFontIndirectA(&logFont)); + LOG_HR_IF_NULL(E_FAIL, _fontHandle.get()); +} + +void FontResource::_resizeBitPattern(gsl::span targetBuffer) +{ + auto sourceWidth = _sourceSize.width(); + auto targetWidth = _targetSize.width(); + const auto sourceHeight = _sourceSize.height(); + const auto targetHeight = _targetSize.height(); + + // If the text in the font is not perfectly centered, the _centeringHint + // gives us the offset needed to correct that misalignment. So to ensure + // that any inserted or deleted columns are evenly spaced around the center + // point of the glyphs, we need to adjust the source and target widths by + // that amount (proportionally) before calculating the scaling increments. + targetWidth -= std::lround((double)_centeringHint * targetWidth / sourceWidth); + sourceWidth -= gsl::narrow_cast(_centeringHint); + + // The way the scaling works is by iterating over the target range, and + // calculating the source offsets that correspond to each target position. + // We achieve that by incrementing the source offset every iteration by an + // integer value that is the quotient of the source and target dimensions. + // Because this is an integer division, we're going to be off by a certain + // fraction on each iteration, so we need to keep track of that accumulated + // error using the modulus of the division. Once the error total exceeds + // the target dimension (more or less), we add another pixel to compensate + // for the error, and reset the error total. + const auto createIncrementFunction = [](const auto sourceDimension, const auto targetDimension) { + const auto increment = sourceDimension / targetDimension; + const auto errorIncrement = sourceDimension % targetDimension * 2; + const auto errorThreshold = targetDimension * 2 - std::min(sourceDimension, targetDimension); + const auto errorReset = targetDimension * 2; + + return [=](auto& errorTotal) { + errorTotal += errorIncrement; + if (errorTotal > errorThreshold) + { + errorTotal -= errorReset; + return increment + 1; + } + return increment; + }; + }; + const auto columnIncrement = createIncrementFunction(sourceWidth, targetWidth); + const auto lineIncrement = createIncrementFunction(sourceHeight, targetHeight); + + // Once we've calculated the scaling increments, taking the centering hint + // into account, we reset the target width back to its original value. + targetWidth = _targetSize.width(); + + auto targetBufferPointer = targetBuffer.begin(); + for (auto ch = 0; ch < CHAR_COUNT; ch++) + { + // Bits are read from the source from left to right - MSB to LSB. The source + // column is a single bit representing the 1-based position. The reason for + // this will become clear in the mask calculation below. + auto sourceColumn = 1 << 16; + auto sourceColumnError = 0; + + // The target format expects the character bitmaps to be laid out in columns + // of 8 bits. So we generate 8 bits from each scanline until we've covered + // the full target height. Then we start again from the top with the next 8 + // bits of the line, until we've covered the full target width. + for (auto targetX = 0; targetX < targetWidth; targetX += 8) + { + auto sourceLine = std::next(_bitPattern.begin(), ch * sourceHeight); + auto sourceLineError = 0; + + // Since we're going to be reading from the same horizontal offset for each + // target line, we save the state here so we can reset it every iteration. + const auto initialSourceColumn = sourceColumn; + const auto initialSourceColumnError = sourceColumnError; + + for (auto targetY = 0; targetY < targetHeight; targetY++) + { + sourceColumn = initialSourceColumn; + sourceColumnError = initialSourceColumnError; + + // For a particular target line, we calculate the span of source lines from + // which it is derived, then OR those values together. We don't want the + // source value to be zero, though, so we must read at least one line. + const auto lineSpan = lineIncrement(sourceLineError); + auto sourceValue = 0; + for (auto i = 0; i < std::max(lineSpan, 1); i++) + { + sourceValue |= sourceLine[i]; + } + std::advance(sourceLine, lineSpan); + + // From the combined value of the source lines, we now need to extract eight + // bits to make up the next byte in the target at the current X offset. + byte targetValue = 0; + for (auto targetBit = 0; targetBit < 8; targetBit++) + { + targetValue <<= 1; + if (targetX + targetBit < targetWidth) + { + // As with the line iteration, we first need to calculate the span of source + // columns from which the target bit is derived. We shift our source column + // position right by that amount to determine the next column position, then + // subtract those two values to obtain a mask. For example, if we're reading + // from columns 6 to 3 (exclusively), the initial column position is 1<<6, + // the next column position is 1<<3, so the mask is 64-8=56, or 00111000. + // Again we don't want this mask to be zero, so if the span is zero, we need + // to shift an additional bit to make sure we cover at least one column. + const auto columnSpan = columnIncrement(sourceColumnError); + const auto nextSourceColumn = sourceColumn >> columnSpan; + const auto sourceMask = sourceColumn - (nextSourceColumn >> (columnSpan ? 0 : 1)); + sourceColumn = nextSourceColumn; + targetValue |= (sourceValue & sourceMask) ? 1 : 0; + } + } + *(targetBufferPointer++) = targetValue; + } + } + } +} diff --git a/src/renderer/base/RenderEngineBase.cpp b/src/renderer/base/RenderEngineBase.cpp index f31fff625..a32b4b525 100644 --- a/src/renderer/base/RenderEngineBase.cpp +++ b/src/renderer/base/RenderEngineBase.cpp @@ -36,6 +36,13 @@ HRESULT RenderEngineBase::UpdateTitle(const std::wstring_view newTitle) noexcept return hr; } +HRESULT RenderEngineBase::UpdateSoftFont(const gsl::span /*bitPattern*/, + const SIZE /*cellSize*/, + const size_t /*centeringHint*/) noexcept +{ + return S_FALSE; +} + HRESULT RenderEngineBase::PrepareRenderInfo(const RenderFrameInfo& /*info*/) noexcept { return S_FALSE; diff --git a/src/renderer/base/lib/base.vcxproj b/src/renderer/base/lib/base.vcxproj index f38398cfc..2a9ee32cb 100644 --- a/src/renderer/base/lib/base.vcxproj +++ b/src/renderer/base/lib/base.vcxproj @@ -15,6 +15,7 @@ + @@ -28,6 +29,7 @@ + diff --git a/src/renderer/base/lib/base.vcxproj.filters b/src/renderer/base/lib/base.vcxproj.filters index 1ec046c43..baff50352 100644 --- a/src/renderer/base/lib/base.vcxproj.filters +++ b/src/renderer/base/lib/base.vcxproj.filters @@ -27,6 +27,9 @@ Source Files + + Source Files + Source Files @@ -65,6 +68,9 @@ Header Files\inc + + Header Files\inc + Header Files\inc diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index b53bc6ead..863bcd24f 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -144,7 +144,7 @@ try }); // A. Prep Colors - RETURN_IF_FAILED(_UpdateDrawingBrushes(pEngine, _pData->GetDefaultBrushColors(), true)); + RETURN_IF_FAILED(_UpdateDrawingBrushes(pEngine, _pData->GetDefaultBrushColors(), false, true)); // B. Perform Scroll Operations RETURN_IF_FAILED(_PerformScrolling(pEngine)); @@ -526,6 +526,35 @@ void Renderer::TriggerFontChange(const int iDpi, const FontInfoDesired& FontInfo _NotifyPaintFrame(); } +// Routine Description: +// - Called when the active soft font has been updated. +// Arguments: +// - bitPattern - An array of scanlines representing all the glyphs in the font. +// - cellSize - The cell size for an individual glyph. +// - centeringHint - The horizontal extent that glyphs are offset from center. +// Return Value: +// - +void Renderer::UpdateSoftFont(const gsl::span bitPattern, const SIZE cellSize, const size_t centeringHint) +{ + // We reserve PUA code points U+EF20 to U+EF7F for soft fonts, but the range + // that we test for in _IsSoftFontChar will depend on the size of the active + // bitPattern. If it's empty (i.e. no soft font is set), then nothing will + // match, and those code points will be treated the same as everything else. + const auto softFontCharCount = cellSize.cy ? bitPattern.size() / cellSize.cy : 0; + _lastSoftFontChar = _firstSoftFontChar + softFontCharCount - 1; + + for (const auto pEngine : _rgpEngines) + { + LOG_IF_FAILED(pEngine->UpdateSoftFont(bitPattern, cellSize, centeringHint)); + } + TriggerRedrawAll(); +} + +bool Renderer::s_IsSoftFontChar(const std::wstring_view& v, const size_t firstSoftFontChar, const size_t lastSoftFontChar) +{ + return v.size() == 1 && v.front() >= firstSoftFontChar && v.front() <= lastSoftFontChar; +} + // Routine Description: // - Get the information on what font we would be using if we decided to create a font with the given parameters // - This is for use with speculative calculations. @@ -740,6 +769,8 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, auto color = it->TextAttr(); // Retrieve the first pattern id auto patternIds = _pData->GetPatternId(target); + // Determine whether we're using a soft font. + auto usingSoftFont = s_IsSoftFontChar(it->Chars(), _firstSoftFontChar, _lastSoftFontChar); // And hold the point where we should start drawing. auto screenPoint = target; @@ -756,8 +787,8 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, // Hold onto the current pattern id as well const auto currentPatternId = patternIds; - // Update the drawing brushes with our color. - THROW_IF_FAILED(_UpdateDrawingBrushes(pEngine, currentRunColor, false)); + // Update the drawing brushes with our color and font usage. + THROW_IF_FAILED(_UpdateDrawingBrushes(pEngine, currentRunColor, usingSoftFont, false)); // Advance the point by however many columns we've just outputted and reset the accumulator. screenPoint.X += gsl::narrow(cols); @@ -786,15 +817,18 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, { COORD thisPoint{ screenPoint.X + gsl::narrow(cols), screenPoint.Y }; const auto thisPointPatterns = _pData->GetPatternId(thisPoint); - if (color != it->TextAttr() || patternIds != thisPointPatterns) + const auto thisUsingSoftFont = s_IsSoftFontChar(it->Chars(), _firstSoftFontChar, _lastSoftFontChar); + const auto changedPatternOrFont = patternIds != thisPointPatterns || usingSoftFont != thisUsingSoftFont; + if (color != it->TextAttr() || changedPatternOrFont) { auto newAttr{ it->TextAttr() }; // foreground doesn't matter for runs of spaces (!) // if we trick it . . . we call Paint far fewer times for cmatrix - if (!_IsAllSpaces(it->Chars()) || !newAttr.HasIdenticalVisualRepresentationForBlankSpace(color, globalInvert) || patternIds != thisPointPatterns) + if (!_IsAllSpaces(it->Chars()) || !newAttr.HasIdenticalVisualRepresentationForBlankSpace(color, globalInvert) || changedPatternOrFont) { color = newAttr; patternIds = thisPointPatterns; + usingSoftFont = thisUsingSoftFont; break; // vend this run } } @@ -1183,17 +1217,21 @@ void Renderer::_PaintSelection(_In_ IRenderEngine* const pEngine) // Arguments: // - pEngine - Which engine is being updated // - textAttributes - The 16 color foreground/background combination to set +// - usingSoftFont - Whether we're rendering characters from a soft font // - isSettingDefaultBrushes - Alerts that the default brushes are being set which will // impact whether or not to include the hung window/erase window brushes in this operation // and can affect other draw state that wants to know the default color scheme. // (Usually only happens when the default is changed, not when each individual color is swapped in a multi-color run.) // Return Value: // - -[[nodiscard]] HRESULT Renderer::_UpdateDrawingBrushes(_In_ IRenderEngine* const pEngine, const TextAttribute textAttributes, const bool isSettingDefaultBrushes) +[[nodiscard]] HRESULT Renderer::_UpdateDrawingBrushes(_In_ IRenderEngine* const pEngine, + const TextAttribute textAttributes, + const bool usingSoftFont, + const bool isSettingDefaultBrushes) { // The last color needs to be each engine's responsibility. If it's local to this function, // then on the next engine we might not update the color. - return pEngine->UpdateDrawingBrushes(textAttributes, _pData, isSettingDefaultBrushes); + return pEngine->UpdateDrawingBrushes(textAttributes, _pData, usingSoftFont, isSettingDefaultBrushes); } // Routine Description: diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index 6f8f6d7a8..eb8eadf62 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -65,6 +65,10 @@ namespace Microsoft::Console::Render const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) override; + void UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) override; + [[nodiscard]] HRESULT GetProposedFont(const int iDpi, const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) override; @@ -120,7 +124,10 @@ namespace Microsoft::Console::Render void _PaintOverlays(_In_ IRenderEngine* const pEngine); void _PaintOverlay(IRenderEngine& engine, const RenderOverlay& overlay); - [[nodiscard]] HRESULT _UpdateDrawingBrushes(_In_ IRenderEngine* const pEngine, const TextAttribute attr, const bool isSettingDefaultBrushes); + [[nodiscard]] HRESULT _UpdateDrawingBrushes(_In_ IRenderEngine* const pEngine, + const TextAttribute attr, + const bool usingSoftFont, + const bool isSettingDefaultBrushes); [[nodiscard]] HRESULT _PerformScrolling(_In_ IRenderEngine* const pEngine); @@ -138,6 +145,10 @@ namespace Microsoft::Console::Render [[nodiscard]] std::optional _GetCursorInfo(); [[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine); + const size_t _firstSoftFontChar = 0xEF20; + size_t _lastSoftFontChar = 0; + static bool s_IsSoftFontChar(const std::wstring_view& v, const size_t firstSoftFontChar, const size_t lastSoftFontChar); + // Helper functions to diagnose issues with painting and layout. // These are only actually effective/on in Debug builds when the flag is set using an attached debugger. bool _fDebug = false; diff --git a/src/renderer/base/sources.inc b/src/renderer/base/sources.inc index 9aeaf8c65..e3f3d2171 100644 --- a/src/renderer/base/sources.inc +++ b/src/renderer/base/sources.inc @@ -29,6 +29,7 @@ SOURCES = \ ..\FontInfo.cpp \ ..\FontInfoBase.cpp \ ..\FontInfoDesired.cpp \ + ..\FontResource.cpp \ ..\RenderEngineBase.cpp \ ..\renderer.cpp \ ..\thread.cpp \ diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 8d8f238e1..e40a40ffd 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1907,11 +1907,13 @@ CATCH_RETURN() // Arguments: // - textAttributes - Text attributes to use for the brush color // - pData - The interface to console data structures required for rendering +// - usingSoftFont - Whether we're rendering characters from a soft font // - isSettingDefaultBrushes - Lets us know that these are the default brushes to paint the swapchain background or selection // Return Value: // - S_OK or relevant DirectX error. [[nodiscard]] HRESULT DxEngine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool /*usingSoftFont*/, const bool isSettingDefaultBrushes) noexcept { // GH#5098: If we're rendering with cleartype text, we need to always render diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 1351e61de..6fdf2fe7f 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -107,6 +107,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 230b09c4f..4caf62ad6 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -15,6 +15,7 @@ Author(s): #pragma once #include "../inc/RenderEngineBase.hpp" +#include "../inc/FontResource.hpp" namespace Microsoft::Console::Render { @@ -61,9 +62,13 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept override; + [[nodiscard]] HRESULT UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept override; [[nodiscard]] HRESULT UpdateDpi(const int iDpi) noexcept override; [[nodiscard]] HRESULT UpdateViewport(const SMALL_RECT srNewViewport) noexcept override; @@ -95,6 +100,7 @@ namespace Microsoft::Console::Render HFONT _hfont; HFONT _hfontItalic; TEXTMETRICW _tmFontMetrics; + FontResource _softFont; static const size_t s_cPolyTextCache = 80; POLYTEXTW _pPolyText[s_cPolyTextCache]; @@ -130,7 +136,14 @@ namespace Microsoft::Console::Render COLORREF _lastFg; COLORREF _lastBg; - bool _lastFontItalic; + + enum class FontType : size_t + { + Default, + Italic, + Soft + }; + FontType _lastFontType; XFORM _currentLineTransform; LineRendition _currentLineRendition; diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index 089e9777e..347135974 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -336,6 +336,9 @@ using namespace Microsoft::Console::Render; auto& polyWidth = _polyWidths.emplace_back(); polyWidth.reserve(cchLine); + // If we have a soft font, we only use the character's lower 7 bits. + const auto softFontCharMask = _lastFontType == FontType::Soft ? L'\x7F' : ~0; + // Sum up the total widths the entire line/run is expected to take while // copying the pixel widths into a structure to direct GDI how many pixels to use per character. size_t cchCharWidths = 0; @@ -347,6 +350,7 @@ using namespace Microsoft::Console::Render; const auto text = cluster.GetText(); polyString += text; + polyString.back() &= softFontCharMask; polyWidth.push_back(gsl::narrow(cluster.GetColumns()) * coordFontSize.X); cchCharWidths += polyWidth.back(); polyWidth.append(text.size() - 1, 0); diff --git a/src/renderer/gdi/state.cpp b/src/renderer/gdi/state.cpp index 3311e7585..04a5df7d5 100644 --- a/src/renderer/gdi/state.cpp +++ b/src/renderer/gdi/state.cpp @@ -29,7 +29,7 @@ GdiEngine::GdiEngine() : _fInvalidRectUsed(false), _lastFg(INVALID_COLOR), _lastBg(INVALID_COLOR), - _lastFontItalic(false), + _lastFontType(FontType::Default), _currentLineTransform(IDENTITY_XFORM), _currentLineRendition(LineRendition::SingleWidth), _fPaintStarted(false), @@ -148,8 +148,8 @@ GdiEngine::~GdiEngine() LOG_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, _hfont)); } - // Record the fact that the selected font is not italic. - _lastFontItalic = false; + // Record the fact that the selected font is the default. + _lastFontType = FontType::Default; if (nullptr != hdcRealWindow) { @@ -269,12 +269,14 @@ GdiEngine::~GdiEngine() // Arguments: // - textAttributes - Text attributes to use for the brush color // - pData - The interface to console data structures required for rendering +// - usingSoftFont - Whether we're rendering characters from a soft font // - isSettingDefaultBrushes - Lets us know that the default brushes are being set so we can update the DC background // and the hung app background painting color // Return Value: // - S_OK if set successfully or relevant GDI error via HRESULT. [[nodiscard]] HRESULT GdiEngine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept { RETURN_IF_FAILED(_FlushBufferLines()); @@ -304,12 +306,25 @@ GdiEngine::~GdiEngine() RETURN_IF_FAILED(s_SetWindowLongWHelper(_hwndTargetWindow, GWL_CONSOLE_BKCOLOR, colorBackground)); } - // If the italic attribute has changed, select an appropriate font variant. - const auto fontItalic = textAttributes.IsItalic(); - if (fontItalic != _lastFontItalic) + // If the font type has changed, select an appropriate font variant or soft font. + const auto usingItalicFont = textAttributes.IsItalic(); + const auto fontType = usingSoftFont ? FontType::Soft : usingItalicFont ? FontType::Italic : FontType::Default; + if (fontType != _lastFontType) { - SelectFont(_hdcMemoryContext, fontItalic ? _hfontItalic : _hfont); - _lastFontItalic = fontItalic; + switch (fontType) + { + case FontType::Soft: + SelectFont(_hdcMemoryContext, _softFont); + break; + case FontType::Italic: + SelectFont(_hdcMemoryContext, _hfontItalic); + break; + case FontType::Default: + default: + SelectFont(_hdcMemoryContext, _hfont); + break; + } + _lastFontType = fontType; } return S_OK; @@ -331,8 +346,8 @@ GdiEngine::~GdiEngine() // Select into DC RETURN_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, hFont.get())); - // Record the fact that the selected font is not italic. - _lastFontItalic = false; + // Record the fact that the selected font is the default. + _lastFontType = FontType::Default; // Save off the font metrics for various other calculations RETURN_HR_IF(E_FAIL, !(GetTextMetricsW(_hdcMemoryContext, &_tmFontMetrics))); @@ -419,11 +434,39 @@ GdiEngine::~GdiEngine() _isTrueTypeFont = Font.IsTrueTypeFont(); _fontCodepage = Font.GetCodePage(); + // Inform the soft font of the change in size. + _softFont.SetTargetSize(_GetFontSize()); + LOG_IF_FAILED(InvalidateAll()); return S_OK; } +// Routine Description: +// - This method will replace the active soft font with the given bit pattern. +// Arguments: +// - bitPattern - An array of scanlines representing all the glyphs in the font. +// - cellSize - The cell size for an individual glyph. +// - centeringHint - The horizontal extent that glyphs are offset from center. +// Return Value: +// - S_OK if successful. E_FAIL if there was an error. +[[nodiscard]] HRESULT GdiEngine::UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept +{ + // If the soft font is currently selected, replace it with the default font. + if (_lastFontType == FontType::Soft) + { + RETURN_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, _hfont)); + _lastFontType = FontType::Default; + } + + // Create a new font resource with the updated pattern, or delete if empty. + _softFont = { bitPattern, cellSize, _GetFontSize(), centeringHint }; + + return S_OK; +} + // Routine Description: // - This method will modify the DPI we're using for scaling calculations. // Arguments: diff --git a/src/renderer/inc/FontResource.hpp b/src/renderer/inc/FontResource.hpp new file mode 100644 index 000000000..aa159ed34 --- /dev/null +++ b/src/renderer/inc/FontResource.hpp @@ -0,0 +1,45 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- FontResource.hpp + +Abstract: +- This manages the construction of in-memory font resources for the VT soft fonts. +--*/ + +#pragma once + +namespace wil +{ + typedef unique_any unique_hfontresource; +} + +namespace Microsoft::Console::Render +{ + class FontResource + { + public: + FontResource(const gsl::span bitPattern, + const til::size sourceSize, + const til::size targetSize, + const size_t centeringHint); + FontResource() = default; + ~FontResource() = default; + FontResource& operator=(FontResource&&) = default; + void SetTargetSize(const til::size targetSize); + operator HFONT(); + + private: + void _regenerateFont(); + void _resizeBitPattern(gsl::span targetBuffer); + + std::vector _bitPattern; + til::size _sourceSize; + til::size _targetSize; + size_t _centeringHint{ 0 }; + wil::unique_hfontresource _resourceHandle; + wil::unique_hfont _fontHandle; + }; +} diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 3d6631e41..bcc98dfc4 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -96,9 +96,13 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept = 0; + [[nodiscard]] virtual HRESULT UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateDpi(const int iDpi) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateViewport(const SMALL_RECT srNewViewport) noexcept = 0; diff --git a/src/renderer/inc/IRenderer.hpp b/src/renderer/inc/IRenderer.hpp index 79058c5ae..a3894bdb7 100644 --- a/src/renderer/inc/IRenderer.hpp +++ b/src/renderer/inc/IRenderer.hpp @@ -50,6 +50,10 @@ namespace Microsoft::Console::Render const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) = 0; + virtual void UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) = 0; + [[nodiscard]] virtual HRESULT GetProposedFont(const int iDpi, const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) = 0; diff --git a/src/renderer/inc/RenderEngineBase.hpp b/src/renderer/inc/RenderEngineBase.hpp index 725347d82..6ad59fec3 100644 --- a/src/renderer/inc/RenderEngineBase.hpp +++ b/src/renderer/inc/RenderEngineBase.hpp @@ -38,6 +38,10 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateTitle(const std::wstring_view newTitle) noexcept override; + [[nodiscard]] HRESULT UpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) noexcept override; + [[nodiscard]] HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept override; [[nodiscard]] HRESULT ResetLineTransform() noexcept override; diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index ba7ca8e5f..2d5d17196 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -364,11 +364,13 @@ CATCH_RETURN(); // Arguments: // - textAttributes - // - pData - +// - usingSoftFont - // - isSettingDefaultBrushes - // Return Value: // - S_FALSE since we do nothing [[nodiscard]] HRESULT UiaEngine::UpdateDrawingBrushes(const TextAttribute& /*textAttributes*/, const gsl::not_null /*pData*/, + const bool /*usingSoftFont*/, const bool /*isSettingDefaultBrushes*/) noexcept { return S_FALSE; diff --git a/src/renderer/uia/UiaRenderer.hpp b/src/renderer/uia/UiaRenderer.hpp index 3ead4673b..6591579c5 100644 --- a/src/renderer/uia/UiaRenderer.hpp +++ b/src/renderer/uia/UiaRenderer.hpp @@ -62,6 +62,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/renderer/vt/Xterm256Engine.cpp b/src/renderer/vt/Xterm256Engine.cpp index 91e220c33..4edece715 100644 --- a/src/renderer/vt/Xterm256Engine.cpp +++ b/src/renderer/vt/Xterm256Engine.cpp @@ -20,12 +20,14 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, // Arguments: // - textAttributes - Text attributes to use for the colors and character rendition // - pData - The interface to console data structures required for rendering +// - usingSoftFont - Whether we're rendering characters from a soft font // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT Xterm256Engine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool /*usingSoftFont*/, const bool /*isSettingDefaultBrushes*/) noexcept { RETURN_IF_FAILED(VtEngine::_RgbUpdateDrawingBrushes(textAttributes)); diff --git a/src/renderer/vt/Xterm256Engine.hpp b/src/renderer/vt/Xterm256Engine.hpp index b1ed02332..4e5913e07 100644 --- a/src/renderer/vt/Xterm256Engine.hpp +++ b/src/renderer/vt/Xterm256Engine.hpp @@ -30,6 +30,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT ManuallyClearScrollback() noexcept override; diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index a948acd82..1c09225fa 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -137,12 +137,14 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // Arguments: // - textAttributes - Text attributes to use for the colors and character rendition // - pData - The interface to console data structures required for rendering +// - usingSoftFont - Whether we're rendering characters from a soft font // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT XtermEngine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null /*pData*/, + const bool /*usingSoftFont*/, const bool /*isSettingDefaultBrushes*/) noexcept { // The base xterm mode only knows about 16 colors diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 1dcf2d173..a46458ad6 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -40,6 +40,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT PaintBufferLine(gsl::span const clusters, const COORD coord, diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index f7b0a788b..bb8f0413f 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -75,6 +75,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& pfiFontInfoDesired, _Out_ FontInfo& pfiFontInfo) noexcept override; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index ed2c027ee..ba67819ab 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -307,6 +307,7 @@ bool WddmConEngine::IsInitialized() [[nodiscard]] HRESULT WddmConEngine::UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null /*pData*/, + const bool /*usingSoftFont*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = textAttributes.GetLegacyAttributes(); diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index 6898f1ece..57663d5cb 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -52,6 +52,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const TextAttribute& textAttributes, const gsl::not_null pData, + const bool usingSoftFont, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index 1938791d4..b22634637 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -434,6 +434,46 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes DependsOnMode }; + enum class DrcsEraseControl : size_t + { + AllChars = 0, + ReloadedChars = 1, + AllRenditions = 2 + }; + + enum class DrcsCellMatrix : size_t + { + Default = 0, + Invalid = 1, + Size5x10 = 2, + Size6x10 = 3, + Size7x10 = 4 + }; + + enum class DrcsFontSet : size_t + { + Default = 0, + Size80x24 = 1, + Size132x24 = 2, + Size80x36 = 11, + Size132x36 = 12, + Size80x48 = 21, + Size132x48 = 22 + }; + + enum class DrcsFontUsage : size_t + { + Default = 0, + Text = 1, + FullCell = 2 + }; + + enum class DrcsCharsetSize : size_t + { + Size94 = 0, + Size96 = 1 + }; + constexpr short s_sDECCOLMSetColumns = 132; constexpr short s_sDECCOLMResetColumns = 80; diff --git a/src/terminal/adapter/FontBuffer.cpp b/src/terminal/adapter/FontBuffer.cpp new file mode 100644 index 000000000..688325b27 --- /dev/null +++ b/src/terminal/adapter/FontBuffer.cpp @@ -0,0 +1,603 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include "FontBuffer.hpp" + +using namespace Microsoft::Console::VirtualTerminal; + +FontBuffer::FontBuffer() noexcept +{ + SetEraseControl(DispatchTypes::DrcsEraseControl::AllRenditions); +}; + +bool FontBuffer::SetEraseControl(const DispatchTypes::DrcsEraseControl eraseControl) noexcept +{ + switch (eraseControl) + { + case DispatchTypes::DrcsEraseControl::AllChars: + case DispatchTypes::DrcsEraseControl::AllRenditions: + // Setting the current cell matrix to an invalid value will guarantee + // that it's different from the pending cell matrix, and any change in + // the font attributes will force the buffer to be cleared. + _cellMatrix = DispatchTypes::DrcsCellMatrix::Invalid; + return true; + case DispatchTypes::DrcsEraseControl::ReloadedChars: + return true; + default: + return false; + } +} + +bool FontBuffer::SetAttributes(const DispatchTypes::DrcsCellMatrix cellMatrix, + const VTParameter cellHeight, + const DispatchTypes::DrcsFontSet fontSet, + const DispatchTypes::DrcsFontUsage fontUsage) noexcept +{ + auto valid = true; + + if (valid) + { + // We don't yet support screen sizes in which the font is horizontally + // or vertically compressed, so there is not much value in storing a + // separate font for each of the screen sizes. However, we still need + // to use these values to determine the cell size for which the font + // was originally targeted, so we can resize it appropriately. + switch (fontSet) + { + case DispatchTypes::DrcsFontSet::Default: + case DispatchTypes::DrcsFontSet::Size80x24: + _columnsPerPage = 80; + _linesPerPage = 24; + break; + case DispatchTypes::DrcsFontSet::Size80x36: + _columnsPerPage = 80; + _linesPerPage = 36; + break; + case DispatchTypes::DrcsFontSet::Size80x48: + _columnsPerPage = 80; + _linesPerPage = 48; + break; + case DispatchTypes::DrcsFontSet::Size132x24: + _columnsPerPage = 132; + _linesPerPage = 24; + break; + case DispatchTypes::DrcsFontSet::Size132x36: + _columnsPerPage = 132; + _linesPerPage = 36; + break; + case DispatchTypes::DrcsFontSet::Size132x48: + _columnsPerPage = 132; + _linesPerPage = 48; + break; + default: + valid = false; + break; + } + } + + if (valid) + { + switch (fontUsage) + { + case DispatchTypes::DrcsFontUsage::Default: + case DispatchTypes::DrcsFontUsage::Text: + _isTextFont = true; + break; + case DispatchTypes::DrcsFontUsage::FullCell: + _isTextFont = false; + break; + default: + valid = false; + break; + } + } + + if (valid) + { + switch (cellMatrix) + { + case DispatchTypes::DrcsCellMatrix::Invalid: + valid = false; + break; + case DispatchTypes::DrcsCellMatrix::Size5x10: + // Size 5x10 is only valid for text fonts. + valid = _isTextFont; + _sizeDeclaredAsMatrix = true; + _declaredWidth = 5; + _declaredHeight = 10; + break; + case DispatchTypes::DrcsCellMatrix::Size6x10: + // Size 6x10 is only valid for text fonts, + // unless it's a VT240 in 132-column mode. + valid = _isTextFont || _columnsPerPage == 132; + _sizeDeclaredAsMatrix = true; + _declaredWidth = 6; + _declaredHeight = 10; + break; + case DispatchTypes::DrcsCellMatrix::Size7x10: + // Size 7x10 is only valid for text fonts. + valid = _isTextFont; + _sizeDeclaredAsMatrix = true; + _declaredWidth = 7; + _declaredHeight = 10; + break; + case DispatchTypes::DrcsCellMatrix::Default: + default: + // If we aren't given one of the predefined matrix sizes, then the + // matrix parameter is a pixel width, and height is obtained from the + // height parameter. This also applies for the default of 0, since a + // 0 width is treated as unknown (we'll try and estimate the expected + // width), and the height parameter can still give us the height. + _sizeDeclaredAsMatrix = false; + _declaredWidth = static_cast(cellMatrix); + _declaredHeight = cellHeight.value_or(0); + valid = (_declaredWidth <= MAX_WIDTH && _declaredHeight <= MAX_HEIGHT); + break; + } + } + + // Save the pending attributes, but don't update the current values until we + // are sure we have a valid sequence that can replace the current buffer. + _pendingCellMatrix = cellMatrix; + _pendingCellHeight = cellHeight.value_or(0); + _pendingFontSet = fontSet; + _pendingFontUsage = fontUsage; + + // Reset the used dimensions. These values will be determined by the extent + // of the sixel data that we receive in the following string sequence. + _usedWidth = 0; + _usedHeight = 0; + + return valid; +} + +bool FontBuffer::SetStartChar(const VTParameter startChar, + const DispatchTypes::DrcsCharsetSize charsetSize) noexcept +{ + switch (charsetSize) + { + case DispatchTypes::DrcsCharsetSize::Size94: + _startChar = startChar.value_or(1); + break; + case DispatchTypes::DrcsCharsetSize::Size96: + _startChar = startChar.value_or(0); + break; + default: + return false; + } + + _currentChar = _startChar; + _pendingCharsetSize = charsetSize; + _charsetIdInitialized = false; + _charsetIdBuilder.Clear(); + + return true; +} + +void FontBuffer::AddSixelData(const wchar_t ch) +{ + if (!_charsetIdInitialized) + { + _buildCharsetId(ch); + } + else if (ch >= L'?' && ch <= L'~') + { + _addSixelValue(ch - L'?'); + } + else if (ch == L'/') + { + _endOfSixelLine(); + } + else if (ch == L';') + { + _endOfCharacter(); + } +} + +bool FontBuffer::FinalizeSixelData() +{ + // If the charset ID hasn't been initialized this isn't a valid update. + RETURN_BOOL_IF_FALSE(_charsetIdInitialized); + + // Flush the current line to make sure we take all the used positions + // into account when calculating the font dimensions. + _endOfSixelLine(); + + // If the buffer has been cleared, we'll need to recalculate the dimensions + // using the latest attributes, adjust the character bit patterns to fit + // their true size, and fill in unused buffer positions with an error glyph. + if (_bufferCleared) + { + std::tie(_fullWidth, _fullHeight, _textWidth) = _calculateDimensions(); + _packAndCenterBitPatterns(); + _fillUnusedCharacters(); + } + + return true; +} + +gsl::span FontBuffer::GetBitPattern() const noexcept +{ + return { _buffer.data(), MAX_CHARS * _fullHeight }; +} + +til::size FontBuffer::GetCellSize() const +{ + return { _fullWidth, _fullHeight }; +} + +size_t FontBuffer::GetTextCenteringHint() const noexcept +{ + return _textCenteringHint; +} + +VTID FontBuffer::GetDesignation() const noexcept +{ + return _charsetId; +} + +void FontBuffer::_buildCharsetId(const wchar_t ch) +{ + // Note that we ignore any characters that are not valid in this state. + if (ch >= 0x20 && ch <= 0x2F) + { + _charsetIdBuilder.AddIntermediate(ch); + } + else if (ch >= 0x30 && ch <= 0x7E) + { + _pendingCharsetId = _charsetIdBuilder.Finalize(ch); + _charsetIdInitialized = true; + _prepareCharacterBuffer(); + } +} + +void FontBuffer::_prepareCharacterBuffer() +{ + // If any of the attributes have changed since the last time characters + // were downloaded, the font dimensions will need to be recalculated, and + // the buffer will need to be cleared. Otherwise we'll just be adding to + // the existing font, assuming the current dimensions. + if (_cellMatrix != _pendingCellMatrix || + _cellHeight != _pendingCellHeight || + _fontSet != _pendingFontSet || + _fontUsage != _pendingFontUsage || + _charsetSize != _pendingCharsetSize || + _charsetId != _pendingCharsetId) + { + // Replace the current attributes with the pending values. + _cellMatrix = _pendingCellMatrix; + _cellHeight = _pendingCellHeight; + _fontSet = _pendingFontSet; + _fontUsage = _pendingFontUsage; + _charsetSize = _pendingCharsetSize; + _charsetId = _pendingCharsetId; + + // Reset the font dimensions to the maximum supported size, since we + // can't be certain of the intended size until we've received all of + // the sixel data. These values will be recalculated once we can work + // out the terminal type that the font was originally designed for. + _fullWidth = MAX_WIDTH; + _fullHeight = MAX_HEIGHT; + _textWidth = MAX_WIDTH; + _textOffset = 0; + + // Clear the buffer. + _buffer.fill(0); + _bufferCleared = true; + } + else + { + _bufferCleared = false; + } + + _prepareNextCharacter(); +} + +void FontBuffer::_prepareNextCharacter() +{ + _lastChar = _currentChar; + _currentCharBuffer = std::next(_buffer.begin(), _currentChar * _fullHeight); + _sixelColumn = 0; + _sixelRow = 0; + + // If the buffer hasn't been cleared, we'll need to clear each character + // position individually, before adding any new sixel data. + if (!_bufferCleared && _currentChar < MAX_CHARS) + { + std::fill_n(_currentCharBuffer, _fullHeight, uint16_t{ 0 }); + } +} + +void FontBuffer::_addSixelValue(const size_t value) noexcept +{ + if (_currentChar < MAX_CHARS && _sixelColumn < _textWidth) + { + // Each sixel updates six pixels of a single column, so we setup a bit + // mask for the column we want to update, and then set that bit in each + // row for which there is a corresponding "on" bit in the input value. + const auto outputColumnBit = (0x8000 >> (_sixelColumn + _textOffset)); + auto outputIterator = _currentCharBuffer; + auto inputValueMask = 1; + for (size_t i = 0; i < 6 && _sixelRow + i < _fullHeight; i++) + { + *outputIterator |= (value & inputValueMask) ? outputColumnBit : 0; + outputIterator++; + inputValueMask <<= 1; + } + } + _sixelColumn++; +} + +void FontBuffer::_endOfSixelLine() +{ + // Move down six rows to the get to the next sixel position. + std::advance(_currentCharBuffer, 6); + _sixelRow += 6; + + // Keep track of the maximum width and height covered by the sixel data. + _usedWidth = std::max(_usedWidth, _sixelColumn); + _usedHeight = std::max(_usedHeight, _sixelRow); + + // Reset the column number to the start of the next line. + _sixelColumn = 0; +} + +void FontBuffer::_endOfCharacter() +{ + _endOfSixelLine(); + _currentChar++; + _prepareNextCharacter(); +} + +std::tuple FontBuffer::_calculateDimensions() const +{ + // If the size is declared as a matrix, this is most likely a VT2xx font, + // typically with a cell size of 10x10. However, in 132-column mode, the + // VT240 has a cell size of 6x10, but that's only for widths of 6 or less. + if (_sizeDeclaredAsMatrix) + { + if (_columnsPerPage == 132 && _declaredWidth <= 6) + { + // 6x10 cell with no clipping. + return { 6, 10, 0 }; + } + else + { + // 10x10 cell with text clipped to 8 pixels. + return { 10, 10, 8 }; + } + } + + // If we've been given explicit dimensions, and this is not a text font, + // then we assume those dimensions are the exact cell size. + if (_declaredWidth && _declaredHeight && !_isTextFont) + { + // Since this is not a text font, no clipping is required. + return { _declaredWidth, _declaredHeight, 0 }; + } + + // For most of the cases that follow, a text font will be clipped within + // the bounds of the declared width (if given). There are only a few cases + // where we'll need to use a hard-coded text width, and that's when the + // font appears to be targeting a VT2xx. + const auto textWidth = _isTextFont ? _declaredWidth : 0; + + // If the lines per page isn't 24, this must be targeting a VT420 or VT5xx. + // The cell width is 6 for 132 columns, and 10 for 80 columns. + // The cell height is 8 for 48 lines and 10 for 36 lines. + if (_linesPerPage != 24) + { + const auto cellWidth = _columnsPerPage == 132 ? 6 : 10; + const auto cellHeight = _linesPerPage == 48 ? 8 : 10; + return { cellWidth, cellHeight, textWidth }; + } + + // Now we're going to test whether the dimensions are in range for a number + // of known terminals. We use the declared dimensions if given, otherwise + // estimate the size from the used sixel values. If comparing a sixel-based + // height, though, we need to round up the target cell height to account for + // the fact that our used height will always be a multiple of six. + const auto inRange = [=](const size_t cellWidth, const size_t cellHeight) { + const auto sixelHeight = (cellHeight + 5) / 6 * 6; + const auto heightInRange = _declaredHeight ? _declaredHeight <= cellHeight : _usedHeight <= sixelHeight; + const auto widthInRange = _declaredWidth ? _declaredWidth <= cellWidth : _usedWidth <= cellWidth; + return heightInRange && widthInRange; + }; + + // In the case of a VT2xx font, you could only use a matrix size (which + // we've dealt with above), or a default size, so the tests below are only + // applicable for a VT2xx when no explicit dimensions have been declared. + const auto noDeclaredSize = _declaredWidth == 0 && _declaredHeight == 0; + + if (_columnsPerPage == 80) + { + if (inRange(8, 10) && noDeclaredSize) + { + // VT2xx - 10x10 cell with text clipped to 8 pixels. + return { 10, 10, 8 }; + } + else if (inRange(15, 12)) + { + // VT320 - 15x12 cell with default text width. + return { 15, 12, textWidth }; + } + else if (inRange(10, 16)) + { + // VT420 & VT5xx - 10x16 cell with default text width. + return { 10, 16, textWidth }; + } + else if (inRange(10, 20)) + { + // VT340 - 10x20 cell with default text width. + return { 10, 20, textWidth }; + } + else if (inRange(12, 30)) + { + // VT382 - 12x30 cell with default text width. + return { 12, 30, textWidth }; + } + else + { + // If all else fails, assume the maximum size. + return { MAX_WIDTH, MAX_HEIGHT, textWidth }; + } + } + else + { + if (inRange(6, 10) && noDeclaredSize) + { + // VT240 - 6x10 cell with no clipping. + return { 6, 10, 0 }; + } + else if (inRange(9, 12)) + { + // VT320 - 9x12 cell with default text width. + return { 9, 12, textWidth }; + } + else if (inRange(6, 16)) + { + // VT420 & VT5xx - 6x16 cell with default text width. + return { 6, 16, textWidth }; + } + else if (inRange(6, 20)) + { + // VT340 - 6x20 cell with default text width. + return { 6, 20, textWidth }; + } + else if (inRange(7, 30)) + { + // VT382 - 7x30 cell with default text width. + return { 7, 30, textWidth }; + } + else + { + // If all else fails, assume the maximum size. + return { MAX_WIDTH, MAX_HEIGHT, textWidth }; + } + } +} + +void FontBuffer::_packAndCenterBitPatterns() +{ + // If this is a text font, we'll clip the bits up to the text width and + // center them within the full cell width. For a full cell font we'll just + // use all of the bits, and no offset will be required. + _textWidth = _textWidth ? _textWidth : _fullWidth; + _textWidth = std::min(_textWidth, _fullWidth); + _textOffset = (_fullWidth - _textWidth) / 2; + const auto textClippingMask = ~(0xFFFF >> _textWidth); + + // If the text is given an explicit width, we check to what extent the + // content is offset from center. Knowing that information will enable the + // renderer to scale the font more symmetrically. + _textCenteringHint = _declaredWidth ? _fullWidth - (_declaredWidth + _textOffset * 2) : 0; + + // Initially the characters are written to the buffer assuming the maximum + // cell height, but now that we know the true height, we need to pack the + // buffer data so that each character occupies the exact number of scanlines + // that are required. + for (auto srcLine = 0u, dstLine = 0u; srcLine < _buffer.size(); srcLine++) + { + if ((srcLine % MAX_HEIGHT) < _fullHeight) + { + auto characterScanline = til::at(_buffer, srcLine); + characterScanline &= textClippingMask; + characterScanline >>= _textOffset; + til::at(_buffer, dstLine++) = characterScanline; + } + } +} + +void FontBuffer::_fillUnusedCharacters() +{ + // Every character in the buffer that hasn't been uploaded will be replaced + // with an error glyph (a reverse question mark). This includes every + // character prior to the start char, or after the last char. + const auto errorPattern = _generateErrorGlyph(); + for (auto ch = 0u; ch < MAX_CHARS; ch++) + { + if (ch < _startChar || ch > _lastChar) + { + auto charBuffer = std::next(_buffer.begin(), ch * _fullHeight); + std::copy_n(errorPattern.begin(), _fullHeight, charBuffer); + } + } +} + +std::array FontBuffer::_generateErrorGlyph() +{ + // We start with a bit pattern for a reverse question mark covering the + // maximum font resolution that we might need. + constexpr std::array inputBitPattern = { + 0b000000000000000, + 0b000000000000000, + 0b000000000000000, + 0b000000000000000, + 0b000000000000000, + 0b000000000000000, + 0b001111111110000, + 0b011111111111000, + 0b111000000011100, + 0b111000000011100, + 0b111000000000000, + 0b111000000000000, + 0b111100000000000, + 0b011111000000000, + 0b000011110000000, + 0b000001110000000, + 0b000001110000000, + 0b000001110000000, + 0b000001110000000, + 0b000000000000000, + 0b000001110000000, + 0b000001110000000, + 0b000001110000000, + }; + + // Then for each possible width and height, we have hard-coded bit masks + // indicating a range of columns and rows to select from the base bitmap + // to produce a scaled down version of reasonable quality. + constexpr std::array widthMasks = { + // clang-format off + 0, 1, 3, 32771, 8457, 9481, 9545, 9673, 42441, 26061, + 58829, 28141, 60909, 63453, 63485, 65533, 65535 + // clang-format on + }; + constexpr std::array heightMasks = { + // clang-format off + 0, 1, 3, 7, 15, 1613952, 10002560, 10002816, 10068352, 10068353, + 26845569, 26847617, 26847619, 26864003, 28961155, 28961219, + 28961731, 62516163, 62516167, 129625031, 129625039, 129756111, + 263973839, 263974863, 268169167, 536604623, 536608719, 536608735, + 536870879, 1073741791, 2147483615, 2147483647, 4294967295 + // clang-format on + }; + + const auto widthMask = widthMasks.at(_fullWidth); + const auto heightMask = heightMasks.at(_fullHeight); + + auto outputBitPattern = std::array{}; + auto outputIterator = outputBitPattern.begin(); + for (auto y = 0; y < MAX_HEIGHT; y++) + { + const auto yBit = (1 << y); + if (heightMask & yBit) + { + const uint16_t inputScanline = til::at(inputBitPattern, y); + uint16_t outputScanline = 0; + for (auto x = MAX_WIDTH; x-- > 0;) + { + const auto xBit = 1 << x; + if (widthMask & xBit) + { + outputScanline <<= 1; + outputScanline |= (inputScanline & xBit) ? 1 : 0; + } + } + outputScanline <<= (MAX_WIDTH - _fullWidth); + *(outputIterator++) = outputScanline; + } + } + return outputBitPattern; +} diff --git a/src/terminal/adapter/FontBuffer.hpp b/src/terminal/adapter/FontBuffer.hpp new file mode 100644 index 000000000..bdd6fff72 --- /dev/null +++ b/src/terminal/adapter/FontBuffer.hpp @@ -0,0 +1,95 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- FontBuffer.hpp + +Abstract: +- This manages the construction and storage of font definitions for the VT DECDLD control sequence. +--*/ + +#pragma once + +#include "DispatchTypes.hpp" + +namespace Microsoft::Console::VirtualTerminal +{ + class FontBuffer + { + public: + FontBuffer() noexcept; + ~FontBuffer() = default; + bool SetEraseControl(const DispatchTypes::DrcsEraseControl eraseControl) noexcept; + bool SetAttributes(const DispatchTypes::DrcsCellMatrix cellMatrix, + const VTParameter cellHeight, + const DispatchTypes::DrcsFontSet fontSet, + const DispatchTypes::DrcsFontUsage fontUsage) noexcept; + bool SetStartChar(const VTParameter startChar, + const DispatchTypes::DrcsCharsetSize charsetSize) noexcept; + void AddSixelData(const wchar_t ch); + bool FinalizeSixelData(); + + gsl::span GetBitPattern() const noexcept; + til::size GetCellSize() const; + size_t GetTextCenteringHint() const noexcept; + VTID GetDesignation() const noexcept; + + private: + static constexpr size_t MAX_WIDTH = 16; + static constexpr size_t MAX_HEIGHT = 32; + static constexpr size_t MAX_CHARS = 96; + + void _buildCharsetId(const wchar_t ch); + void _prepareCharacterBuffer(); + void _prepareNextCharacter(); + void _addSixelValue(const size_t value) noexcept; + void _endOfSixelLine(); + void _endOfCharacter(); + + std::tuple _calculateDimensions() const; + void _packAndCenterBitPatterns(); + void _fillUnusedCharacters(); + std::array _generateErrorGlyph(); + + DispatchTypes::DrcsCellMatrix _cellMatrix; + DispatchTypes::DrcsCellMatrix _pendingCellMatrix; + size_t _cellHeight; + size_t _pendingCellHeight; + bool _sizeDeclaredAsMatrix; + size_t _declaredWidth; + size_t _declaredHeight; + size_t _usedWidth; + size_t _usedHeight; + size_t _fullWidth; + size_t _fullHeight; + size_t _textWidth; + size_t _textOffset; + size_t _textCenteringHint; + + DispatchTypes::DrcsFontSet _fontSet; + DispatchTypes::DrcsFontSet _pendingFontSet; + DispatchTypes::DrcsFontUsage _fontUsage; + DispatchTypes::DrcsFontUsage _pendingFontUsage; + size_t _linesPerPage; + size_t _columnsPerPage; + bool _isTextFont; + + DispatchTypes::DrcsCharsetSize _charsetSize; + DispatchTypes::DrcsCharsetSize _pendingCharsetSize; + VTID _charsetId{ 0 }; + VTID _pendingCharsetId{ 0 }; + bool _charsetIdInitialized; + VTIDBuilder _charsetIdBuilder; + size_t _startChar; + size_t _lastChar; + size_t _currentChar; + + using buffer_type = std::array; + buffer_type _buffer; + buffer_type::iterator _currentCharBuffer; + bool _bufferCleared; + size_t _sixelColumn; + size_t _sixelRow; + }; +} diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp index fe53e49d8..b0efd81d8 100644 --- a/src/terminal/adapter/ITermDispatch.hpp +++ b/src/terminal/adapter/ITermDispatch.hpp @@ -23,6 +23,8 @@ namespace Microsoft::Console::VirtualTerminal class Microsoft::Console::VirtualTerminal::ITermDispatch { public: + using StringHandler = std::function; + #pragma warning(push) #pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril virtual ~ITermDispatch() = 0; @@ -130,6 +132,15 @@ public: virtual bool EndHyperlink() = 0; virtual bool DoConEmuAction(const std::wstring_view string) = 0; + + virtual StringHandler DownloadDRCS(const size_t fontNumber, + const VTParameter startChar, + const DispatchTypes::DrcsEraseControl eraseControl, + const DispatchTypes::DrcsCellMatrix cellMatrix, + const DispatchTypes::DrcsFontSet fontSet, + const DispatchTypes::DrcsFontUsage fontUsage, + const VTParameter cellHeight, + const DispatchTypes::DrcsCharsetSize charsetSize) = 0; // DECDLD }; inline Microsoft::Console::VirtualTerminal::ITermDispatch::~ITermDispatch() {} #pragma warning(pop) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 4bd30cbc8..50db520c0 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -1884,6 +1884,7 @@ bool AdaptDispatch::SoftReset() // - Performs a communications line disconnect. // - Clears UDKs. // - Clears a down-line-loaded character set. +// * The soft font is reset in the renderer and the font buffer is deleted. // - Clears the screen. // * This is like Erase in Display (3), also clearing scrollback, as well as ED(2) // - Returns the cursor to the upper-left corner of the screen. @@ -1929,6 +1930,10 @@ bool AdaptDispatch::HardReset() // Delete all current tab stops and reapply _ResetTabStops(); + // Clear the soft font in the renderer and delete the font buffer. + success = _pConApi->PrivateUpdateSoftFont({}, {}, false) && success; + _fontBuffer = nullptr; + // GH#2715 - If all this succeeded, but we're in a conpty, return `false` to // make the state machine propagate this RIS sequence to the connected // terminal application. We've reset our state, but the connected terminal @@ -2440,6 +2445,88 @@ bool AdaptDispatch::DoConEmuAction(const std::wstring_view /*string*/) noexcept return false; } +// Method Description: +// - DECDLD - Downloads one or more characters of a dynamically redefinable +// character set (DRCS) with a specified pixel pattern. The pixel array is +// transmitted in sixel format via the returned StringHandler function. +// Arguments: +// - fontNumber - The buffer number into which the font will be loaded. +// - startChar - The first character in the set that will be replaced. +// - eraseControl - Which characters to erase before loading the new data. +// - cellMatrix - The character cell width (sometimes also height in legacy formats). +// - fontSet - The screen size for which the font is designed. +// - fontUsage - Whether it is a text font or a full-cell font. +// - cellHeight - The character cell height (if not defined by cellMatrix). +// - charsetSize - Whether the character set is 94 or 96 characters. +// Return Value: +// - a function to receive the pixel data or nullptr if parameters are invalid +ITermDispatch::StringHandler AdaptDispatch::DownloadDRCS(const size_t fontNumber, + const VTParameter startChar, + const DispatchTypes::DrcsEraseControl eraseControl, + const DispatchTypes::DrcsCellMatrix cellMatrix, + const DispatchTypes::DrcsFontSet fontSet, + const DispatchTypes::DrcsFontUsage fontUsage, + const VTParameter cellHeight, + const DispatchTypes::DrcsCharsetSize charsetSize) +{ + // If we're a conpty, we're just going to ignore the operation for now. + // There's no point in trying to pass it through without also being able + // to pass through the character set designations. + if (_pConApi->IsConsolePty()) + { + return nullptr; + } + + // The font buffer is created on demand. + if (!_fontBuffer) + { + _fontBuffer = std::make_unique(); + } + + // Only one font buffer is supported, so only 0 (default) and 1 are valid. + auto success = fontNumber <= 1; + success = success && _fontBuffer->SetEraseControl(eraseControl); + success = success && _fontBuffer->SetAttributes(cellMatrix, cellHeight, fontSet, fontUsage); + success = success && _fontBuffer->SetStartChar(startChar, charsetSize); + + // If any of the parameters are invalid, we return a null handler to let + // the state machine know we want to ignore the subsequent data string. + if (!success) + { + return nullptr; + } + + return [=](const auto ch) { + // We pass the data string straight through to the font buffer class + // until we receive an ESC, indicating the end of the string. At that + // point we can finalize the buffer, and if valid, update the renderer + // with the constructed bit pattern. + if (ch != AsciiChars::ESC) + { + _fontBuffer->AddSixelData(ch); + } + else if (_fontBuffer->FinalizeSixelData()) + { + // We also need to inform the character set mapper of the ID that + // will map to this font (we only support one font buffer so there + // will only ever be one active dynamic character set). + if (charsetSize == DispatchTypes::DrcsCharsetSize::Size96) + { + _termOutput.SetDrcs96Designation(_fontBuffer->GetDesignation()); + } + else + { + _termOutput.SetDrcs94Designation(_fontBuffer->GetDesignation()); + } + const auto bitPattern = _fontBuffer->GetBitPattern(); + const auto cellSize = _fontBuffer->GetCellSize(); + const auto centeringHint = _fontBuffer->GetTextCenteringHint(); + _pConApi->PrivateUpdateSoftFont(bitPattern, cellSize, centeringHint); + } + return true; + }; +} + // Routine Description: // - Determines whether we should pass any sequence that manipulates // TerminalInput's input generator through the PTY. It encapsulates diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index 5ea582343..7eeade5cb 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -18,6 +18,7 @@ Author(s): #include "DispatchCommon.hpp" #include "conGetSet.hpp" #include "adaptDefaults.hpp" +#include "FontBuffer.hpp" #include "terminalOutput.hpp" #include "..\..\types\inc\sgrStack.hpp" @@ -130,6 +131,15 @@ namespace Microsoft::Console::VirtualTerminal bool DoConEmuAction(const std::wstring_view string) noexcept override; + StringHandler DownloadDRCS(const size_t fontNumber, + const VTParameter startChar, + const DispatchTypes::DrcsEraseControl eraseControl, + const DispatchTypes::DrcsCellMatrix cellMatrix, + const DispatchTypes::DrcsFontSet fontSet, + const DispatchTypes::DrcsFontUsage fontUsage, + const VTParameter cellHeight, + const DispatchTypes::DrcsCharsetSize charsetSize) override; // DECDLD + private: enum class ScrollDirection { @@ -187,6 +197,7 @@ namespace Microsoft::Console::VirtualTerminal std::unique_ptr _pConApi; std::unique_ptr _pDefaults; TerminalOutput _termOutput; + std::unique_ptr _fontBuffer; std::optional _initialCodePage; // We have two instances of the saved cursor state, because we need diff --git a/src/terminal/adapter/charsets.hpp b/src/terminal/adapter/charsets.hpp index 4ead5f14b..0a3a649ff 100644 --- a/src/terminal/adapter/charsets.hpp +++ b/src/terminal/adapter/charsets.hpp @@ -43,6 +43,11 @@ namespace Microsoft::Console::VirtualTerminal return rhs == lhs; } + // Note that the 94-character sets are deliberately defined with a size of + // 95 to avoid having to test the lower bound. We just alway leave the first + // entry - which is not meant to be mapped - as a SPACE or NBSP, which is at + // least visually equivalent to leaving it untranslated. + typedef CharSet AsciiBasedCharSet; typedef CharSet Latin1BasedCharSet94; typedef CharSet Latin1BasedCharSet96; @@ -1051,5 +1056,11 @@ namespace Microsoft::Console::VirtualTerminal { L'\x7e', L'\u00fc' }, // Latin Small Letter U With Diaeresis }; + // We're reserving 96 characters (U+EF20 to U+EF7F) from the Unicode + // Private Use Area for our dynamically redefinable characters sets. + static constexpr auto DRCS_BASE_CHAR = L'\uEF20'; + static constexpr auto Drcs94 = CharSet{ { DRCS_BASE_CHAR, '\x20' } }; + static constexpr auto Drcs96 = CharSet{}; + #pragma warning(pop) } diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index d604d8e06..e69ab753f 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -107,5 +107,9 @@ namespace Microsoft::Console::VirtualTerminal virtual bool PrivateAddHyperlink(const std::wstring_view uri, const std::wstring_view params) const = 0; virtual bool PrivateEndHyperlink() const = 0; + + virtual bool PrivateUpdateSoftFont(const gsl::span bitPattern, + const SIZE cellSize, + const size_t centeringHint) = 0; }; } diff --git a/src/terminal/adapter/lib/adapter.vcxproj b/src/terminal/adapter/lib/adapter.vcxproj index eeb6f4574..1626e7a7d 100644 --- a/src/terminal/adapter/lib/adapter.vcxproj +++ b/src/terminal/adapter/lib/adapter.vcxproj @@ -12,6 +12,7 @@ + @@ -27,6 +28,7 @@ + diff --git a/src/terminal/adapter/lib/adapter.vcxproj.filters b/src/terminal/adapter/lib/adapter.vcxproj.filters index 4675a7e3a..fb644781d 100644 --- a/src/terminal/adapter/lib/adapter.vcxproj.filters +++ b/src/terminal/adapter/lib/adapter.vcxproj.filters @@ -39,6 +39,9 @@ Source Files + + Source Files + @@ -83,6 +86,9 @@ Header Files + + Header Files + diff --git a/src/terminal/adapter/sources.inc b/src/terminal/adapter/sources.inc index 0fadc810f..8b3ae4f43 100644 --- a/src/terminal/adapter/sources.inc +++ b/src/terminal/adapter/sources.inc @@ -32,6 +32,7 @@ PRECOMPILED_INCLUDE = ..\precomp.h SOURCES= \ ..\adaptDispatch.cpp \ ..\DispatchCommon.cpp \ + ..\FontBuffer.cpp \ ..\InteractDispatch.cpp \ ..\adaptDispatchGraphics.cpp \ ..\terminalOutput.cpp \ diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp index d82a28c22..1fcb83a4e 100644 --- a/src/terminal/adapter/termDispatch.hpp +++ b/src/terminal/adapter/termDispatch.hpp @@ -123,4 +123,13 @@ public: bool EndHyperlink() noexcept override { return false; } bool DoConEmuAction(const std::wstring_view /*string*/) noexcept override { return false; } + + StringHandler DownloadDRCS(const size_t /*fontNumber*/, + const VTParameter /*startChar*/, + const DispatchTypes::DrcsEraseControl /*eraseControl*/, + const DispatchTypes::DrcsCellMatrix /*cellMatrix*/, + const DispatchTypes::DrcsFontSet /*fontSet*/, + const DispatchTypes::DrcsFontUsage /*fontUsage*/, + const VTParameter /*cellHeight*/, + const DispatchTypes::DrcsCharsetSize /*charsetSize*/) noexcept override { return nullptr; } }; diff --git a/src/terminal/adapter/terminalOutput.cpp b/src/terminal/adapter/terminalOutput.cpp index 14de84e13..944ff88b7 100644 --- a/src/terminal/adapter/terminalOutput.cpp +++ b/src/terminal/adapter/terminalOutput.cpp @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -#include -#include +#include "precomp.h" #include "charsets.hpp" #include "terminalOutput.hpp" #include "strsafe.h" @@ -19,91 +18,30 @@ TerminalOutput::TerminalOutput() noexcept bool TerminalOutput::Designate94Charset(size_t gsetNumber, const VTID charset) { - switch (charset) - { - case VTID("B"): // US ASCII - case VTID("1"): // Alternate Character ROM - return _SetTranslationTable(gsetNumber, Ascii); - case VTID("0"): // DEC Special Graphics - case VTID("2"): // Alternate Character ROM Special Graphics - return _SetTranslationTable(gsetNumber, DecSpecialGraphics); - case VTID("<"): // DEC Supplemental - return _SetTranslationTable(gsetNumber, DecSupplemental); - case VTID("A"): // British NRCS - return _SetTranslationTable(gsetNumber, BritishNrcs); - case VTID("4"): // Dutch NRCS - return _SetTranslationTable(gsetNumber, DutchNrcs); - case VTID("5"): // Finnish NRCS - case VTID("C"): // (fallback) - return _SetTranslationTable(gsetNumber, FinnishNrcs); - case VTID("R"): // French NRCS - return _SetTranslationTable(gsetNumber, FrenchNrcs); - case VTID("f"): // French NRCS (ISO update) - return _SetTranslationTable(gsetNumber, FrenchNrcsIso); - case VTID("9"): // French Canadian NRCS - case VTID("Q"): // (fallback) - return _SetTranslationTable(gsetNumber, FrenchCanadianNrcs); - case VTID("K"): // German NRCS - return _SetTranslationTable(gsetNumber, GermanNrcs); - case VTID("Y"): // Italian NRCS - return _SetTranslationTable(gsetNumber, ItalianNrcs); - case VTID("6"): // Norwegian/Danish NRCS - case VTID("E"): // (fallback) - return _SetTranslationTable(gsetNumber, NorwegianDanishNrcs); - case VTID("`"): // Norwegian/Danish NRCS (ISO standard) - return _SetTranslationTable(gsetNumber, NorwegianDanishNrcsIso); - case VTID("Z"): // Spanish NRCS - return _SetTranslationTable(gsetNumber, SpanishNrcs); - case VTID("7"): // Swedish NRCS - case VTID("H"): // (fallback) - return _SetTranslationTable(gsetNumber, SwedishNrcs); - case VTID("="): // Swiss NRCS - return _SetTranslationTable(gsetNumber, SwissNrcs); - case VTID("&4"): // DEC Cyrillic - return _SetTranslationTable(gsetNumber, DecCyrillic); - case VTID("&5"): // Russian NRCS - return _SetTranslationTable(gsetNumber, RussianNrcs); - case VTID("\"?"): // DEC Greek - return _SetTranslationTable(gsetNumber, DecGreek); - case VTID("\">"): // Greek NRCS - return _SetTranslationTable(gsetNumber, GreekNrcs); - case VTID("\"4"): // DEC Hebrew - return _SetTranslationTable(gsetNumber, DecHebrew); - case VTID("%="): // Hebrew NRCS - return _SetTranslationTable(gsetNumber, HebrewNrcs); - case VTID("%0"): // DEC Turkish - return _SetTranslationTable(gsetNumber, DecTurkish); - case VTID("%2"): // Turkish NRCS - return _SetTranslationTable(gsetNumber, TurkishNrcs); - case VTID("%5"): // DEC Supplemental - return _SetTranslationTable(gsetNumber, DecSupplemental); - case VTID("%6"): // Portuguese NRCS - return _SetTranslationTable(gsetNumber, PortugueseNrcs); - default: - return false; - } + const auto translationTable = _LookupTranslationTable94(charset); + RETURN_BOOL_IF_FALSE(!translationTable.empty()); + return _SetTranslationTable(gsetNumber, translationTable); } bool TerminalOutput::Designate96Charset(size_t gsetNumber, const VTID charset) { - switch (charset) - { - case VTID("A"): // ISO Latin-1 Supplemental - case VTID("<"): // (UPSS when assigned to Latin-1) - return _SetTranslationTable(gsetNumber, Latin1); - case VTID("B"): // ISO Latin-2 Supplemental - return _SetTranslationTable(gsetNumber, Latin2); - case VTID("L"): // ISO Latin-Cyrillic Supplemental - return _SetTranslationTable(gsetNumber, LatinCyrillic); - case VTID("F"): // ISO Latin-Greek Supplemental - return _SetTranslationTable(gsetNumber, LatinGreek); - case VTID("H"): // ISO Latin-Hebrew Supplemental - return _SetTranslationTable(gsetNumber, LatinHebrew); - case VTID("M"): // ISO Latin-5 Supplemental - return _SetTranslationTable(gsetNumber, Latin5); - default: - return false; - } + const auto translationTable = _LookupTranslationTable96(charset); + RETURN_BOOL_IF_FALSE(!translationTable.empty()); + return _SetTranslationTable(gsetNumber, translationTable); +} + +void TerminalOutput::SetDrcs94Designation(const VTID charset) +{ + _ReplaceDrcsTable(_LookupTranslationTable94(charset), Drcs94); + _drcsId = charset; + _drcsTranslationTable = Drcs94; +} + +void TerminalOutput::SetDrcs96Designation(const VTID charset) +{ + _ReplaceDrcsTable(_LookupTranslationTable96(charset), Drcs96); + _drcsId = charset; + _drcsTranslationTable = Drcs96; } #pragma warning(suppress : 26440) // Suppress spurious "function can be declared noexcept" warning @@ -186,9 +124,139 @@ wchar_t TerminalOutput::TranslateKey(const wchar_t wch) const noexcept return wchFound; } +const std::wstring_view TerminalOutput::_LookupTranslationTable94(const VTID charset) const +{ + // Note that the DRCS set can be designated with either a 94 or 96 sequence, + // regardless of the actual size of the set. This isn't strictly correct, + // but there is existing software that depends on this behavior. + if (charset == _drcsId) + { + return _drcsTranslationTable; + } + switch (charset) + { + case VTID("B"): // US ASCII + case VTID("1"): // Alternate Character ROM + return Ascii; + case VTID("0"): // DEC Special Graphics + case VTID("2"): // Alternate Character ROM Special Graphics + return DecSpecialGraphics; + case VTID("<"): // DEC Supplemental + return DecSupplemental; + case VTID("A"): // British NRCS + return BritishNrcs; + case VTID("4"): // Dutch NRCS + return DutchNrcs; + case VTID("5"): // Finnish NRCS + case VTID("C"): // (fallback) + return FinnishNrcs; + case VTID("R"): // French NRCS + return FrenchNrcs; + case VTID("f"): // French NRCS (ISO update) + return FrenchNrcsIso; + case VTID("9"): // French Canadian NRCS + case VTID("Q"): // (fallback) + return FrenchCanadianNrcs; + case VTID("K"): // German NRCS + return GermanNrcs; + case VTID("Y"): // Italian NRCS + return ItalianNrcs; + case VTID("6"): // Norwegian/Danish NRCS + case VTID("E"): // (fallback) + return NorwegianDanishNrcs; + case VTID("`"): // Norwegian/Danish NRCS (ISO standard) + return NorwegianDanishNrcsIso; + case VTID("Z"): // Spanish NRCS + return SpanishNrcs; + case VTID("7"): // Swedish NRCS + case VTID("H"): // (fallback) + return SwedishNrcs; + case VTID("="): // Swiss NRCS + return SwissNrcs; + case VTID("&4"): // DEC Cyrillic + return DecCyrillic; + case VTID("&5"): // Russian NRCS + return RussianNrcs; + case VTID("\"?"): // DEC Greek + return DecGreek; + case VTID("\">"): // Greek NRCS + return GreekNrcs; + case VTID("\"4"): // DEC Hebrew + return DecHebrew; + case VTID("%="): // Hebrew NRCS + return HebrewNrcs; + case VTID("%0"): // DEC Turkish + return DecTurkish; + case VTID("%2"): // Turkish NRCS + return TurkishNrcs; + case VTID("%5"): // DEC Supplemental + return DecSupplemental; + case VTID("%6"): // Portuguese NRCS + return PortugueseNrcs; + default: + return {}; + } +} + +const std::wstring_view TerminalOutput::_LookupTranslationTable96(const VTID charset) const +{ + // Note that the DRCS set can be designated with either a 94 or 96 sequence, + // regardless of the actual size of the set. This isn't strictly correct, + // but there is existing software that depends on this behavior. + if (charset == _drcsId) + { + return _drcsTranslationTable; + } + switch (charset) + { + case VTID("A"): // ISO Latin-1 Supplemental + case VTID("<"): // (UPSS when assigned to Latin-1) + return Latin1; + case VTID("B"): // ISO Latin-2 Supplemental + return Latin2; + case VTID("L"): // ISO Latin-Cyrillic Supplemental + return LatinCyrillic; + case VTID("F"): // ISO Latin-Greek Supplemental + return LatinGreek; + case VTID("H"): // ISO Latin-Hebrew Supplemental + return LatinHebrew; + case VTID("M"): // ISO Latin-5 Supplemental + return Latin5; + default: + return {}; + } +} + bool TerminalOutput::_SetTranslationTable(const size_t gsetNumber, const std::wstring_view translationTable) { _gsetTranslationTables.at(gsetNumber) = translationTable; // We need to reapply the locking shifts in case the underlying G-sets have changed. return LockingShift(_glSetNumber) && LockingShiftRight(_grSetNumber); } + +void TerminalOutput::_ReplaceDrcsTable(const std::wstring_view oldTable, const std::wstring_view newTable) +{ + if (newTable.data() != oldTable.data()) + { + for (size_t gsetNumber = 0; gsetNumber < 4; gsetNumber++) + { + // Get the current translation table for this G-set. + auto gsetTable = _gsetTranslationTables.at(gsetNumber); + // If it's already a DRCS, replace it with a default charset. + if (Drcs94 == gsetTable || Drcs96 == gsetTable) + { + gsetTable = gsetNumber < 2 ? (std::wstring_view)Ascii : (std::wstring_view)Latin1; + } + // If it matches the old table, replace it with the new table. + if (gsetTable.data() == oldTable.data()) + { + gsetTable = newTable; + } + // Update the G-set entry with the new translation table. + _gsetTranslationTables.at(gsetNumber) = gsetTable; + } + // Reapply the locking shifts in case the underlying G-sets have changed. + LockingShift(_glSetNumber); + LockingShiftRight(_grSetNumber); + } +} diff --git a/src/terminal/adapter/terminalOutput.hpp b/src/terminal/adapter/terminalOutput.hpp index e82e067a7..aabc12657 100644 --- a/src/terminal/adapter/terminalOutput.hpp +++ b/src/terminal/adapter/terminalOutput.hpp @@ -28,6 +28,8 @@ namespace Microsoft::Console::VirtualTerminal wchar_t TranslateKey(const wchar_t wch) const noexcept; bool Designate94Charset(const size_t gsetNumber, const VTID charset); bool Designate96Charset(const size_t gsetNumber, const VTID charset); + void SetDrcs94Designation(const VTID charset); + void SetDrcs96Designation(const VTID charset); bool LockingShift(const size_t gsetNumber); bool LockingShiftRight(const size_t gsetNumber); bool SingleShift(const size_t gsetNumber); @@ -35,7 +37,10 @@ namespace Microsoft::Console::VirtualTerminal void EnableGrTranslation(boolean enabled); private: + const std::wstring_view _LookupTranslationTable94(const VTID charset) const; + const std::wstring_view _LookupTranslationTable96(const VTID charset) const; bool _SetTranslationTable(const size_t gsetNumber, const std::wstring_view translationTable); + void _ReplaceDrcsTable(const std::wstring_view oldTable, const std::wstring_view newTable); std::array _gsetTranslationTables; size_t _glSetNumber = 0; @@ -44,5 +49,7 @@ namespace Microsoft::Console::VirtualTerminal std::wstring_view _grTranslationTable; mutable std::wstring_view _ssTranslationTable; boolean _grTranslationEnabled = false; + VTID _drcsId = 0; + std::wstring_view _drcsTranslationTable; }; } diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index ce120ea3b..13f9476ae 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -567,6 +567,19 @@ public: return TRUE; } + bool PrivateUpdateSoftFont(const gsl::span /*bitPattern*/, + const SIZE cellSize, + const size_t /*centeringHint*/) noexcept override + { + Log::Comment(L"PrivateUpdateSoftFont MOCK called..."); + + Log::Comment(NoThrowString().Format(L"Cell size: %dx%d", cellSize.cx, cellSize.cy)); + VERIFY_ARE_EQUAL(_expectedCellSize.cx, cellSize.cx); + VERIFY_ARE_EQUAL(_expectedCellSize.cy, cellSize.cy); + + return TRUE; + } + void PrepData() { PrepData(CursorDirection::UP); // if called like this, the cursor direction doesn't matter. @@ -810,6 +823,8 @@ public: bool _privateSetDefaultBackgroundResult = false; COLORREF _expectedDefaultBackgroundColorValue = INVALID_COLOR; + SIZE _expectedCellSize = {}; + private: HANDLE _hCon; }; @@ -2353,6 +2368,229 @@ public: VERIFY_IS_FALSE(_pDispatch.get()->SetColorTableEntry(15, testColor)); } + TEST_METHOD(SoftFontSizeDetection) + { + using CellMatrix = DispatchTypes::DrcsCellMatrix; + using FontSet = DispatchTypes::DrcsFontSet; + using FontUsage = DispatchTypes::DrcsFontUsage; + + const auto decdld = [=](const auto cmw, const auto cmh, const auto ss, const auto u, const std::wstring_view data = {}) { + const auto ec = DispatchTypes::DrcsEraseControl::AllChars; + const auto css = DispatchTypes::DrcsCharsetSize::Size94; + const auto cellMatrix = static_cast(cmw); + const auto stringHandler = _pDispatch.get()->DownloadDRCS(0, 0, ec, cellMatrix, ss, u, cmh, css); + if (stringHandler) + { + stringHandler(L'B'); // Charset identifier + for (auto ch : data) + { + stringHandler(ch); + } + stringHandler(L'\033'); // String terminator + } + return stringHandler != nullptr; + }; + + // Matrix sizes at 80x24 should always use a 10x10 cell size (VT2xx). + Log::Comment(L"Matrix 5x10 for 80x24 font set with text usage"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size5x10, 0, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Matrix 6x10 for 80x24 font set with text usage"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size6x10, 0, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Matrix 7x10 for 80x24 font set with text usage"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size7x10, 0, FontSet::Size80x24, FontUsage::Text)); + + // At 132x24 the cell size is typically 6x10 (VT240), but could be 10x10 (VT220) + Log::Comment(L"Matrix 5x10 for 132x24 font set with text usage"); + _testGetSet->_expectedCellSize = { 6, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size5x10, 0, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Matrix 6x10 for 132x24 font set with text usage"); + _testGetSet->_expectedCellSize = { 6, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size6x10, 0, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Matrix 7x10 for 132x24 font set with text usage (VT220 only)"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size7x10, 0, FontSet::Size132x24, FontUsage::Text)); + + // Full cell usage is invalid for all matrix sizes except 6x10 at 132x24. + Log::Comment(L"Matrix 5x10 for 80x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Size5x10, 0, FontSet::Size80x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 6x10 for 80x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Size6x10, 0, FontSet::Size80x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 7x10 for 80x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Size7x10, 0, FontSet::Size80x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 5x10 for 132x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Size5x10, 0, FontSet::Size132x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 6x10 for 132x24 font set with full cell usage"); + _testGetSet->_expectedCellSize = { 6, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size6x10, 0, FontSet::Size132x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 7x10 for 132x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Size7x10, 0, FontSet::Size132x24, FontUsage::FullCell)); + + // Matrix size 1 is always invalid. + Log::Comment(L"Matrix 1 for 80x24 font set with text usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Invalid, 0, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Matrix 1 for 132x24 font set with text usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Invalid, 0, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Matrix 1 for 80x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Invalid, 0, FontSet::Size80x24, FontUsage::FullCell)); + Log::Comment(L"Matrix 1 for 132x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(CellMatrix::Invalid, 0, FontSet::Size132x24, FontUsage::FullCell)); + + // The height parameter has no effect when a matrix size is used. + Log::Comment(L"Matrix 7x10 with unused height parameter"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Size7x10, 20, FontSet::Size80x24, FontUsage::Text)); + + // Full cell fonts with explicit dimensions are accepted as their given cell size. + Log::Comment(L"Explicit 13x17 for 80x24 font set with full cell usage"); + _testGetSet->_expectedCellSize = { 13, 17 }; + VERIFY_IS_TRUE(decdld(13, 17, FontSet::Size80x24, FontUsage::FullCell)); + Log::Comment(L"Explicit 9x25 for 132x24 font set with full cell usage"); + _testGetSet->_expectedCellSize = { 9, 25 }; + VERIFY_IS_TRUE(decdld(9, 25, FontSet::Size132x24, FontUsage::FullCell)); + + // Cell sizes outside the maximum supported range (16x32) are invalid. + Log::Comment(L"Explicit 18x38 for 80x24 font set with full cell usage (invalid)"); + VERIFY_IS_FALSE(decdld(18, 38, FontSet::Size80x24, FontUsage::FullCell)); + + // Text fonts with explicit dimensions are interpreted as their closest matching device. + Log::Comment(L"Explicit 12x12 for 80x24 font set with text usage (VT320)"); + _testGetSet->_expectedCellSize = { 15, 12 }; + VERIFY_IS_TRUE(decdld(12, 12, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Explicit 9x20 for 80x24 font set with text usage (VT340)"); + _testGetSet->_expectedCellSize = { 10, 20 }; + VERIFY_IS_TRUE(decdld(9, 20, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Explicit 10x30 for 80x24 font set with text usage (VT382)"); + _testGetSet->_expectedCellSize = { 12, 30 }; + VERIFY_IS_TRUE(decdld(10, 30, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Explicit 8x16 for 80x24 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 16 }; + VERIFY_IS_TRUE(decdld(8, 16, FontSet::Size80x24, FontUsage::Text)); + Log::Comment(L"Explicit 7x12 for 132x24 font set with text usage (VT320)"); + _testGetSet->_expectedCellSize = { 9, 12 }; + VERIFY_IS_TRUE(decdld(7, 12, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Explicit 5x20 for 132x24 font set with text usage (VT340)"); + _testGetSet->_expectedCellSize = { 6, 20 }; + VERIFY_IS_TRUE(decdld(5, 20, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Explicit 6x30 for 132x24 font set with text usage (VT382)"); + _testGetSet->_expectedCellSize = { 7, 30 }; + VERIFY_IS_TRUE(decdld(6, 30, FontSet::Size132x24, FontUsage::Text)); + Log::Comment(L"Explicit 5x16 for 132x24 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 16 }; + VERIFY_IS_TRUE(decdld(5, 16, FontSet::Size132x24, FontUsage::Text)); + + // Font sets with more than 24 lines must be VT420/VT5xx. + Log::Comment(L"80x36 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x36, FontUsage::Text)); + Log::Comment(L"80x48 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 8 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x48, FontUsage::Text)); + Log::Comment(L"132x36 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x36, FontUsage::Text)); + Log::Comment(L"132x48 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 8 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x48, FontUsage::Text)); + Log::Comment(L"80x36 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x36, FontUsage::FullCell)); + Log::Comment(L"80x48 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 8 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x48, FontUsage::FullCell)); + Log::Comment(L"132x36 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 10 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x36, FontUsage::FullCell)); + Log::Comment(L"132x48 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 8 }; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x48, FontUsage::FullCell)); + + // Without an explicit size, the cell size is estimated from the number of sixels + // used in the character bitmaps. But note that sixel heights are always a multiple + // of 6, so will often be larger than the cell size for which they were intended. + Log::Comment(L"8x12 bitmap for 80x24 font set with text usage (VT2xx)"); + _testGetSet->_expectedCellSize = { 10, 10 }; + const auto bitmapOf8x12 = L"????????/????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::Text, bitmapOf8x12)); + Log::Comment(L"12x12 bitmap for 80x24 font set with text usage (VT320)"); + _testGetSet->_expectedCellSize = { 15, 12 }; + const auto bitmapOf12x12 = L"????????????/????????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::Text, bitmapOf12x12)); + Log::Comment(L"9x24 bitmap for 80x24 font set with text usage (VT340)"); + _testGetSet->_expectedCellSize = { 10, 20 }; + const auto bitmapOf9x24 = L"?????????/?????????/?????????/?????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::Text, bitmapOf9x24)); + Log::Comment(L"10x30 bitmap for 80x24 font set with text usage (VT382)"); + _testGetSet->_expectedCellSize = { 12, 30 }; + const auto bitmapOf10x30 = L"??????????/??????????/??????????/??????????/??????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::Text, bitmapOf10x30)); + Log::Comment(L"8x18 bitmap for 80x24 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 16 }; + const auto bitmapOf8x18 = L"????????/????????/????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::Text, bitmapOf8x18)); + + Log::Comment(L"5x12 bitmap for 132x24 font set with text usage (VT240)"); + _testGetSet->_expectedCellSize = { 6, 10 }; + const auto bitmapOf5x12 = L"?????/?????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::Text, bitmapOf5x12)); + Log::Comment(L"7x12 bitmap for 132x24 font set with text usage (VT320)"); + _testGetSet->_expectedCellSize = { 9, 12 }; + const auto bitmapOf7x12 = L"???????/???????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::Text, bitmapOf7x12)); + Log::Comment(L"5x24 bitmap for 132x24 font set with text usage (VT340)"); + _testGetSet->_expectedCellSize = { 6, 20 }; + const auto bitmapOf5x24 = L"?????/?????/?????/?????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::Text, bitmapOf5x24)); + Log::Comment(L"6x30 bitmap for 132x24 font set with text usage (VT382)"); + _testGetSet->_expectedCellSize = { 7, 30 }; + const auto bitmapOf6x30 = L"??????/??????/??????/??????/??????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::Text, bitmapOf6x30)); + Log::Comment(L"5x18 bitmap for 132x24 font set with text usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 16 }; + const auto bitmapOf5x18 = L"?????/?????/?????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::Text, bitmapOf5x18)); + + Log::Comment(L"15x12 bitmap for 80x24 font set with full cell usage (VT320)"); + _testGetSet->_expectedCellSize = { 15, 12 }; + const auto bitmapOf15x12 = L"???????????????/???????????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::FullCell, bitmapOf15x12)); + Log::Comment(L"10x24 bitmap for 80x24 font set with full cell usage (VT340)"); + _testGetSet->_expectedCellSize = { 10, 20 }; + const auto bitmapOf10x24 = L"??????????/??????????/??????????/??????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::FullCell, bitmapOf10x24)); + Log::Comment(L"12x30 bitmap for 80x24 font set with full cell usage (VT382)"); + _testGetSet->_expectedCellSize = { 12, 30 }; + const auto bitmapOf12x30 = L"????????????/????????????/????????????/????????????/????????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::FullCell, bitmapOf12x30)); + Log::Comment(L"10x18 bitmap for 80x24 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 10, 16 }; + const auto bitmapOf10x18 = L"??????????/??????????/??????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size80x24, FontUsage::FullCell, bitmapOf10x18)); + + Log::Comment(L"6x12 bitmap for 132x24 font set with full cell usage (VT240)"); + _testGetSet->_expectedCellSize = { 6, 10 }; + const auto bitmapOf6x12 = L"??????/??????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::FullCell, bitmapOf6x12)); + Log::Comment(L"9x12 bitmap for 132x24 font set with full cell usage (VT320)"); + _testGetSet->_expectedCellSize = { 9, 12 }; + const auto bitmapOf9x12 = L"?????????/?????????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::FullCell, bitmapOf9x12)); + Log::Comment(L"6x24 bitmap for 132x24 font set with full cell usage (VT340)"); + _testGetSet->_expectedCellSize = { 6, 20 }; + const auto bitmapOf6x24 = L"??????/??????/??????/??????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::FullCell, bitmapOf6x24)); + Log::Comment(L"7x30 bitmap for 132x24 font set with full cell usage (VT382)"); + _testGetSet->_expectedCellSize = { 7, 30 }; + const auto bitmapOf7x30 = L"???????/???????/???????/???????/???????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::FullCell, bitmapOf7x30)); + Log::Comment(L"6x18 bitmap for 132x24 font set with full cell usage (VT420/VT5xx)"); + _testGetSet->_expectedCellSize = { 6, 16 }; + const auto bitmapOf6x18 = L"??????/??????/??????"; + VERIFY_IS_TRUE(decdld(CellMatrix::Default, 0, FontSet::Size132x24, FontUsage::FullCell, bitmapOf6x18)); + } + private: TestGetSet* _testGetSet; // non-ownership pointer std::unique_ptr _pDispatch; diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index 0ecd5ea4d..953fce415 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -645,10 +645,27 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete // - parameters - set of numeric parameters collected while parsing the sequence. // Return Value: // - the data string handler function or nullptr if the sequence is not supported -IStateMachineEngine::StringHandler OutputStateMachineEngine::ActionDcsDispatch(const VTID /*id*/, const VTParameters /*parameters*/) noexcept +IStateMachineEngine::StringHandler OutputStateMachineEngine::ActionDcsDispatch(const VTID id, const VTParameters parameters) { StringHandler handler = nullptr; + switch (id) + { + case DcsActionCodes::DECDLD_DownloadDRCS: + handler = _dispatch->DownloadDRCS(parameters.at(0), + parameters.at(1), + parameters.at(2), + parameters.at(3), + parameters.at(4), + parameters.at(5), + parameters.at(6), + parameters.at(7)); + break; + default: + handler = nullptr; + break; + } + _ClearLastChar(); return handler; diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index 1f79795a9..8ef831160 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -39,7 +39,7 @@ namespace Microsoft::Console::VirtualTerminal bool ActionCsiDispatch(const VTID id, const VTParameters parameters) override; - StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) noexcept override; + StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) override; bool ActionClear() noexcept override; @@ -144,6 +144,11 @@ namespace Microsoft::Console::VirtualTerminal DECSCPP_SetColumnsPerPage = VTID("$|"), }; + enum DcsActionCodes : uint64_t + { + DECDLD_DownloadDRCS = VTID("{"), + }; + enum Vt52ActionCodes : uint64_t { CursorUp = VTID("A"), From 9f2d40614bde8f80dbcc11f369dc33e77f902652 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 9 Aug 2021 10:21:59 -0500 Subject: [PATCH 10/16] Allow `ThrottledFunc` to work on different types of dispatcher (#10187) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### ⚠️ targets #10051 ## Summary of the Pull Request This updates our `ThrottledFunc`s to take a dispatcher parameter. This means that we can use the `Windows::UI::Core::CoreDispatcher` in the `TermControl`, where there's always a `CoreDispatcher`, and use a `Windows::System::DispatcherQueue` in `ControlCore`/`ControlInteractivity`. When running in-proc, these are always the _same thing_. However, out-of-proc, the core needs a dispatcher queue that's not tied to a UI thread (because the content proces _doesn't have a UI thread!_). This lets us get rid of the output event, because we don't need to bubble that event out to the `TermControl` to let it throttle that update anymore. ## References * Tear-out: #1256 * Megathread: #5000 * Project: https://github.com/microsoft/terminal/projects/5 ## PR Checklist * [x] This is a part of #1256 * [x] I work here * [n/a] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments Fortunately, `winrt::resume_foreground` works the same on both a `CoreDispatcher` and a `DispatcherQueue`, so this wasn't too hard! ## Validation Steps Performed This was validated in `dev/migrie/oop/the-whole-thing` (or `dev/migrie/oop/connection-factory`, I forget which), and I made sure that it worked both in-proc and x-proc. Not only that, _it wasn't any slower_!This reverts commit 04b751faa70680bf0296063deacec4657c6ff9d6. --- src/cascadia/TerminalApp/AppLogic.cpp | 2 +- src/cascadia/TerminalControl/ControlCore.cpp | 106 +++++++++++++++--- src/cascadia/TerminalControl/ControlCore.h | 23 +++- src/cascadia/TerminalControl/TermControl.cpp | 83 ++++---------- src/cascadia/TerminalControl/TermControl.h | 9 +- .../UnitTests_Control/ControlCoreTests.cpp | 25 +++-- .../ControlInteractivityTests.cpp | 11 +- src/cascadia/UnitTests_Control/pch.h | 2 + src/cascadia/WinRTUtils/inc/ThrottledFunc.h | 8 +- 9 files changed, 169 insertions(+), 100 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index a9ac10d8f..c14a951de 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -203,7 +203,7 @@ namespace winrt::TerminalApp::implementation _isElevated = _isUserAdmin(); _root = winrt::make_self(); - _reloadSettings = std::make_shared>(_root->Dispatcher(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() { + _reloadSettings = std::make_shared>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() { if (auto self{ weakSelf.get() }) { self->_ReloadSettings(); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index bf106322a..2cc3fef16 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -23,6 +23,16 @@ using namespace winrt::Windows::Graphics::Display; using namespace winrt::Windows::System; using namespace winrt::Windows::ApplicationModel::DataTransfer; +// The minimum delay between updates to the scroll bar's values. +// The updates are throttled to limit power usage. +constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8); + +// The minimum delay between updating the TSF input control. +constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100); + +// The minimum delay between updating the locations of regex patterns +constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500); + namespace winrt::Microsoft::Terminal::Control::implementation { // Helper static function to ensure that all ambiguous-width glyphs are reported as narrow. @@ -94,6 +104,61 @@ namespace winrt::Microsoft::Terminal::Control::implementation auto pfnTerminalTaskbarProgressChanged = std::bind(&ControlCore::_terminalTaskbarProgressChanged, this); _terminal->TaskbarProgressChangedCallback(pfnTerminalTaskbarProgressChanged); + // Get our dispatcher. If we're hosted in-proc with XAML, this will get + // us the same dispatcher as TermControl::Dispatcher(). If we're out of + // proc, this'll return null. We'll need to instead make a new + // DispatcherQueue (on a new thread), so we can use that for throttled + // functions. + _dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); + if (!_dispatcher) + { + auto controller{ winrt::Windows::System::DispatcherQueueController::CreateOnDedicatedThread() }; + _dispatcher = controller.DispatcherQueue(); + } + + // A few different events should be throttled, so they don't fire absolutely all the time: + // * _tsfTryRedrawCanvas: When the cursor position moves, we need to + // inform TSF, so it can move the canvas for the composition. We + // throttle this so that we're not hopping across the process boundary + // every time that the cursor moves. + // * _updatePatternLocations: When there's new output, or we scroll the + // viewport, we should re-check if there are any visible hyperlinks. + // But we don't really need to do this every single time text is + // output, we can limit this update to once every 500ms. + // * _updateScrollBar: Same idea as the TSF update - we don't _really_ + // need to hop across the process boundary every time text is output. + // We can throttle this to once every 8ms, which will get us out of + // the way of the main output & rendering threads. + _tsfTryRedrawCanvas = std::make_shared>( + _dispatcher, + TsfRedrawInterval, + [weakThis = get_weak()]() { + if (auto core{ weakThis.get() }; !core->_IsClosing()) + { + core->_CursorPositionChangedHandlers(*core, nullptr); + } + }); + + _updatePatternLocations = std::make_shared>( + _dispatcher, + UpdatePatternLocationsInterval, + [weakThis = get_weak()]() { + if (auto core{ weakThis.get() }; !core->_IsClosing()) + { + core->UpdatePatternLocations(); + } + }); + + _updateScrollBar = std::make_shared>( + _dispatcher, + ScrollBarUpdateInterval, + [weakThis = get_weak()](const auto& update) { + if (auto core{ weakThis.get() }; !core->_IsClosing()) + { + core->_ScrollPositionChangedHandlers(*core, update); + } + }); + UpdateSettings(settings); } @@ -1103,15 +1168,28 @@ namespace winrt::Microsoft::Terminal::Control::implementation // TODO GH#9617: refine locking around pattern tree _terminal->ClearPatternTree(); - _ScrollPositionChangedHandlers(*this, - winrt::make(viewTop, - viewHeight, - bufferSize)); + // Start the throttled update of our scrollbar. + auto update{ winrt::make(viewTop, + viewHeight, + bufferSize) }; + if (!_inUnitTests) + { + _updateScrollBar->Run(update); + } + else + { + _ScrollPositionChangedHandlers(*this, update); + } + + // Additionally, start the throttled update of where our links are. + _updatePatternLocations->Run(); } void ControlCore::_terminalCursorPositionChanged() { - _CursorPositionChangedHandlers(*this, nullptr); + // When the buffer's cursor moves, start the throttled func to + // eventually dispatch a CursorPositionChanged event. + _tsfTryRedrawCanvas->Run(); } void ControlCore::_terminalTaskbarProgressChanged() @@ -1221,8 +1299,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::Close() { - if (!_closing.exchange(true)) + if (!_IsClosing()) { + _closing = true; + // Stop accepting new output and state changes before we disconnect everything. _connection.TerminalOutput(_connectionOutputEventToken); _connectionStateChangedRevoker.revoke(); @@ -1400,18 +1480,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->Write(hstr); - // NOTE: We're raising an event here to inform the TermControl that - // output has been received, so it can queue up a throttled - // UpdatePatternLocations call. In the future, we should have the - // _updatePatternLocations ThrottledFunc internal to this class, and - // run on this object's dispatcher queue. - // - // We're not doing that quite yet, because the Core will eventually - // be out-of-proc from the UI thread, and won't be able to just use - // the UI thread as the dispatcher queue thread. - // - // See TODO: https://github.com/microsoft/terminal/projects/5#card-50760282 - _ReceivedOutputHandlers(*this, nullptr); + // Start the throttled update of where our hyperlinks are. + _updatePatternLocations->Run(); } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 855aa66d3..3bb321f0b 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -168,7 +168,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation private: bool _initializedTerminal{ false }; - std::atomic _closing{ false }; + bool _closing{ false }; TerminalConnection::ITerminalConnection _connection{ nullptr }; event_token _connectionOutputEventToken; @@ -206,6 +206,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation double _panelHeight{ 0 }; double _compositionScale{ 0 }; + winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr }; + std::shared_ptr> _tsfTryRedrawCanvas; + std::shared_ptr> _updatePatternLocations; + std::shared_ptr> _updateScrollBar; + winrt::fire_and_forget _asyncCloseConnection(); void _setFontSize(int fontSize); @@ -239,8 +244,24 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _connectionOutputHandler(const hstring& hstr); void _updateHoveredCell(const std::optional terminalPosition); + inline bool _IsClosing() const noexcept + { +#ifndef NDEBUG + if (_dispatcher) + { + // _closing isn't atomic and may only be accessed from the main thread. + // + // Though, the unit tests don't actually run in TAEF's main + // thread, so we don't care when we're running in tests. + assert(_inUnitTests || _dispatcher.HasThreadAccess()); + } +#endif + return _closing; + } + friend class ControlUnitTests::ControlCoreTests; friend class ControlUnitTests::ControlInteractivityTests; + bool _inUnitTests{ false }; }; } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index e1ea1d7a0..0d20606d1 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -33,7 +33,8 @@ using namespace winrt::Windows::ApplicationModel::DataTransfer; constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8); // The minimum delay between updating the TSF input control. -constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100); +// This is already throttled primarily in the ControlCore, with a timeout of 100ms. We're adding another smaller one here, as the (potentially x-proc) call will come in off the UI thread +constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(8); // The minimum delay between updating the locations of regex patterns constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500); @@ -64,10 +65,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation _interactivity = winrt::make(settings, connection); _core = _interactivity.Core(); - // Use a manual revoker on the output event, so we can immediately stop - // worrying about it on destruction. - _coreOutputEventToken = _core.ReceivedOutput({ this, &TermControl::_coreReceivedOutput }); - // These events might all be triggered by the connection, but that // should be drained and closed before we complete destruction. So these // are safe. @@ -104,37 +101,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation } }); - // Many of these ThrottledFunc's should be inside ControlCore. However, - // currently they depend on the Dispatcher() of the UI thread, which the - // Core eventually won't have access to. When we get to - // https://github.com/microsoft/terminal/projects/5#card-50760282 - // then we'll move the applicable ones. - // - // These four throttled functions are triggered by terminal output and interact with the UI. + // Get our dispatcher. This will get us the same dispatcher as + // TermControl::Dispatcher(). + auto dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); + + // These three throttled functions are triggered by terminal output and interact with the UI. // Since Close() is the point after which we are removed from the UI, but before the // destructor has run, we MUST check control->_IsClosing() before actually doing anything. - _tsfTryRedrawCanvas = std::make_shared>( - Dispatcher(), - TsfRedrawInterval, - [weakThis = get_weak()]() { - if (auto control{ weakThis.get() }; !control->_IsClosing()) - { - control->TSFInputControl().TryRedrawCanvas(); - } - }); - - _updatePatternLocations = std::make_shared>( - Dispatcher(), - UpdatePatternLocationsInterval, - [weakThis = get_weak()]() { - if (auto control{ weakThis.get() }; !control->_IsClosing()) - { - control->_core.UpdatePatternLocations(); - } - }); - _playWarningBell = std::make_shared( - Dispatcher(), + dispatcher, TerminalWarningBellInterval, [weakThis = get_weak()]() { if (auto control{ weakThis.get() }; !control->_IsClosing()) @@ -144,7 +119,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation }); _updateScrollBar = std::make_shared>( - Dispatcher(), + dispatcher, ScrollBarUpdateInterval, [weakThis = get_weak()](const auto& update) { if (auto control{ weakThis.get() }; !control->_IsClosing()) @@ -540,7 +515,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // create a custom automation peer with this code pattern: // (https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers) - if (const auto& interactivityAutoPeer = _interactivity.OnCreateAutomationPeer()) + if (const auto& interactivityAutoPeer{ _interactivity.OnCreateAutomationPeer() }) { _automationPeer = winrt::make(this, interactivityAutoPeer); return _automationPeer; @@ -1276,23 +1251,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation CATCH_LOG(); } - void TermControl::_coreReceivedOutput(const IInspectable& /*sender*/, - const IInspectable& /*args*/) - { - // Queue up a throttled UpdatePatternLocations call. In the future, we - // should have the _updatePatternLocations ThrottledFunc internal to - // ControlCore, and run on that object's dispatcher queue. - // - // We're not doing that quite yet, because the Core will eventually - // be out-of-proc from the UI thread, and won't be able to just use - // the UI thread as the dispatcher queue thread. - // - // THIS IS CALLED ON EVERY STRING OF TEXT OUTPUT TO THE TERMINAL. Think - // twice before adding anything here. - - _updatePatternLocations->Run(); - } - // Method Description: // - Reset the font size of the terminal to its default size. // Arguments: @@ -1330,8 +1288,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation _updateScrollBar->ModifyPending([](auto& update) { update.newValue.reset(); }); - - _updatePatternLocations->Run(); } // Method Description: @@ -1665,7 +1621,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation update.newValue = args.ViewTop(); _updateScrollBar->Run(update); - _updatePatternLocations->Run(); } // Method Description: @@ -1673,10 +1628,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation // to be where the current cursor position is. // Arguments: // - N/A - void TermControl::_CursorPositionChanged(const IInspectable& /*sender*/, - const IInspectable& /*args*/) + winrt::fire_and_forget TermControl::_CursorPositionChanged(const IInspectable& /*sender*/, + const IInspectable& /*args*/) { - _tsfTryRedrawCanvas->Run(); + // Prior to GH#10187, this fired a trailing throttled func to update the + // TSF canvas only every 100ms. Now, the throttling occurs on the + // ControlCore side. If we're told to update the cursor position, we can + // just go ahead and do it. + // This can come in off the COM thread - hop back to the UI thread. + auto weakThis{ get_weak() }; + co_await resume_foreground(Dispatcher()); + if (auto control{ weakThis.get() }; !control->_IsClosing()) + { + control->TSFInputControl().TryRedrawCanvas(); + } } hstring TermControl::Title() @@ -1730,8 +1695,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _closing = true; - _core.ReceivedOutput(_coreOutputEventToken); _RestorePointerCursorHandlers(*this, nullptr); + // Disconnect the TSF input control so it doesn't receive EditContext events. TSFInputControl().Close(); _autoScrollTimer.Stop(); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 6c9aa6757..e16ecbaae 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -153,8 +153,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _focused{ false }; bool _initializedTerminal{ false }; - std::shared_ptr> _tsfTryRedrawCanvas; - std::shared_ptr> _updatePatternLocations; std::shared_ptr _playWarningBell; struct ScrollBarUpdate @@ -164,7 +162,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation double newMinimum; double newViewportSize; }; + std::shared_ptr> _updateScrollBar; + bool _isInternalScrollBarUpdate; // Auto scroll occurs when user, while selecting, drags cursor outside @@ -181,8 +181,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::optional _cursorTimer; std::optional _blinkTimer; - event_token _coreOutputEventToken; - winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; inline bool _IsClosing() const noexcept @@ -233,7 +231,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _TerminalTabColorChanged(const std::optional color); void _ScrollPositionChanged(const IInspectable& sender, const Control::ScrollPositionChangedArgs& args); - void _CursorPositionChanged(const IInspectable& sender, const IInspectable& args); + winrt::fire_and_forget _CursorPositionChanged(const IInspectable& sender, const IInspectable& args); bool _CapturePointer(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e); bool _ReleasePointerCapture(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e); @@ -265,7 +263,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation const int fontHeight, const bool isInitialChange); winrt::fire_and_forget _coreTransparencyChanged(IInspectable sender, Control::TransparencyChangedEventArgs args); - void _coreReceivedOutput(const IInspectable& sender, const IInspectable& args); void _coreRaisedNotice(const IInspectable& s, const Control::NoticeEventArgs& args); void _coreWarningBell(const IInspectable& sender, const IInspectable& args); }; diff --git a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp index 96617b513..e57dd8c23 100644 --- a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp @@ -56,6 +56,16 @@ namespace ControlUnitTests return { settings, conn }; } + + winrt::com_ptr createCore(Control::IControlSettings settings, + TerminalConnection::ITerminalConnection conn) + { + Log::Comment(L"Create ControlCore object"); + + auto core = winrt::make_self(settings, conn); + core->_inUnitTests = true; + return core; + } }; void ControlCoreTests::ComPtrSettings() @@ -71,8 +81,7 @@ namespace ControlUnitTests { auto [settings, conn] = _createSettingsAndConnection(); - Log::Comment(L"Create ControlCore object"); - auto core = winrt::make_self(*settings, *conn); + auto core = createCore(*settings, *conn); VERIFY_IS_NOT_NULL(core); } @@ -80,8 +89,7 @@ namespace ControlUnitTests { auto [settings, conn] = _createSettingsAndConnection(); - Log::Comment(L"Create ControlCore object"); - auto core = winrt::make_self(*settings, *conn); + auto core = createCore(*settings, *conn); VERIFY_IS_NOT_NULL(core); VERIFY_IS_FALSE(core->_initializedTerminal); @@ -99,8 +107,7 @@ namespace ControlUnitTests settings->UseAcrylic(true); settings->TintOpacity(0.5f); - Log::Comment(L"Create ControlCore object"); - auto core = winrt::make_self(*settings, *conn); + auto core = createCore(*settings, *conn); VERIFY_IS_NOT_NULL(core); // A callback to make sure that we're raising TransparencyChanged events @@ -167,8 +174,7 @@ namespace ControlUnitTests { auto [settings, conn] = _createSettingsAndConnection(); - Log::Comment(L"Create ControlCore object"); - auto core = winrt::make_self(*settings, *conn); + auto core = createCore(*settings, *conn); VERIFY_IS_NOT_NULL(core); Log::Comment(L"Close the Core, like a TermControl would"); @@ -190,8 +196,7 @@ namespace ControlUnitTests // that you don't default to Cascadia* settings->FontFace(L"Impact"); - Log::Comment(L"Create ControlCore object"); - auto core = winrt::make_self(*settings, *conn); + auto core = createCore(*settings, *conn); VERIFY_IS_NOT_NULL(core); VERIFY_ARE_EQUAL(L"Impact", std::wstring_view{ core->_actualFont.GetFaceName() }); diff --git a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp index 14801722e..a94ed8778 100644 --- a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp @@ -73,6 +73,7 @@ namespace ControlUnitTests auto interactivity = winrt::make_self(settings, conn); VERIFY_IS_NOT_NULL(interactivity); auto core = interactivity->_core; + core->_inUnitTests = true; VERIFY_IS_NOT_NULL(core); return { core, interactivity }; @@ -163,6 +164,10 @@ namespace ControlUnitTests void ControlInteractivityTests::TestScrollWithMouse() { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + END_TEST_METHOD_PROPERTIES() + WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; auto [settings, conn] = _createSettingsAndConnection(); @@ -243,7 +248,7 @@ namespace ControlUnitTests buttonState); Log::Comment(NoThrowString().Format(L"internal scrollbar pos:%f", interactivity->_internalScrollbarPosition)); } - Log::Comment(L"Scrolling up more should do nothing"); + Log::Comment(L"Scrolling down more should do nothing"); expectedTop = 21; interactivity->MouseWheel(modifiers, -WHEEL_DELTA, @@ -257,6 +262,10 @@ namespace ControlUnitTests void ControlInteractivityTests::CreateSubsequentSelectionWithDragging() { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + END_TEST_METHOD_PROPERTIES() + // This is a test for GH#9725 WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; diff --git a/src/cascadia/UnitTests_Control/pch.h b/src/cascadia/UnitTests_Control/pch.h index 7568822f2..a013f7462 100644 --- a/src/cascadia/UnitTests_Control/pch.h +++ b/src/cascadia/UnitTests_Control/pch.h @@ -47,6 +47,8 @@ Licensed under the MIT license. // Manually include til after we include Windows.Foundation to give it winrt superpowers #include "til.h" +#include "ThrottledFunc.h" + // Common includes for most tests: #include "../../inc/argb.h" #include "../../inc/conattrs.hpp" diff --git a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h index d5ff0a0e4..40482faf1 100644 --- a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h +++ b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h @@ -24,7 +24,7 @@ public: // // After `func` was invoked the state is reset and this cycle is repeated again. ThrottledFunc( - winrt::Windows::UI::Core::CoreDispatcher dispatcher, + winrt::Windows::System::DispatcherQueue dispatcher, filetime_duration delay, function func) : _dispatcher{ std::move(dispatcher) }, @@ -81,7 +81,7 @@ private: { if constexpr (leading) { - _dispatcher.RunAsync(winrt::Windows::UI::Core::CoreDispatcherPriority::Normal, [weakSelf = this->weak_from_this()]() { + _dispatcher.TryEnqueue(winrt::Windows::System::DispatcherQueuePriority::Normal, [weakSelf = this->weak_from_this()]() { if (auto self{ weakSelf.lock() }) { try @@ -108,7 +108,7 @@ private: } else { - _dispatcher.RunAsync(winrt::Windows::UI::Core::CoreDispatcherPriority::Normal, [weakSelf = this->weak_from_this()]() { + _dispatcher.TryEnqueue(winrt::Windows::System::DispatcherQueuePriority::Normal, [weakSelf = this->weak_from_this()]() { if (auto self{ weakSelf.lock() }) { try @@ -129,7 +129,7 @@ private: } FILETIME _delay; - winrt::Windows::UI::Core::CoreDispatcher _dispatcher; + winrt::Windows::System::DispatcherQueue _dispatcher; function _func; wil::unique_threadpool_timer _timer; From fdffa24a711e66472c6f3a5f151c6859bc290c42 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 9 Aug 2021 10:29:04 -0700 Subject: [PATCH 11/16] Update SUI tooltips from 'checked' to 'enabled' (#10885) Updates the Settings UI tooltips to use "enabled" and "disabled" instead of "checked" and "unchecked" respectively. Closes #10814 --- .../Resources/en-US/Resources.resw | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw index 2dec26d0e..26eefd38b 100644 --- a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw @@ -232,7 +232,7 @@ Header for a control to toggle if the app should always show the tabs (similar to a website browser). - When unchecked, the tab bar will appear when a new tab is created. + When disabled, the tab bar will appear when a new tab is created. A description for what the "always show tabs" setting does. Presented near "Globals_AlwaysShowTabs.Header". @@ -272,7 +272,7 @@ Header for a control to toggle the "force full repaint" setting. When enabled, the app renders new content between screen frames. - When unchecked, the terminal will render only the updates to the screen between frames. + When disabled, the terminal will render only the updates to the screen between frames. A description for what the "force full repaint" setting does. Presented near "Globals_ForceFullRepaint.Header". @@ -340,7 +340,7 @@ Header for a control to toggle whether the title bar should be shown or not. Changing this setting requires the user to relaunch the app. - When unchecked, the title bar will appear above the tabs. + When disabled, the title bar will appear above the tabs. A description for what the "show titlebar" setting does. Presented near "Globals_ShowTitlebar.Header". @@ -348,7 +348,7 @@ Header for a control to toggle whether the terminal's title is shown as the application title, or not. - When unchecked, the title bar will be 'Windows Terminal'. + When disabled, the title bar will be 'Windows Terminal'. A description for what the "show title in titlebar" setting does. Presented near "Globals_ShowTitleInTitlebar.Header".{Locked="Windows"} @@ -356,7 +356,7 @@ Header for a control to toggle whether the terminal snaps the window to the character grid when resizing, or not. - When unchecked, the window will resize smoothly. + When disabled, the window will resize smoothly. A description for what the "snap to grid on resize" setting does. Presented near "Globals_SnapToGridOnResize.Header". @@ -364,7 +364,7 @@ Header for a control to toggle whether the terminal should use software to render content instead of the hardware. - When checked, the terminal will use the software renderer (a.k.a. WARP) instead of the hardware one. + When enabled, the terminal will use the software renderer (a.k.a. WARP) instead of the hardware one. A description for what the "software rendering" setting does. Presented near "Globals_SoftwareRendering.Header". @@ -372,7 +372,7 @@ Header for a control to toggle whether the app should launch when the user's machine starts up, or not. - When checked, this enables the launch of Windows Terminal at machine startup. + When enabled, this enables the launch of Windows Terminal at machine startup. A description for what the "start on user login" setting does. Presented near "Globals_StartOnUserLogin.Header". @@ -480,7 +480,7 @@ Header for a control to toggle whether the app treats ctrl+alt as the AltGr (also known as the Alt Graph) modifier key found on keyboards. {Locked="AltGr"} - By default Windows treats Ctrl+Alt as an alias for AltGr. When unchecked, this behavior will be disabled. + By default Windows treats Ctrl+Alt as an alias for AltGr. When disabled, this behavior will be disabled. A description for what the "AltGr aliasing" setting does. Presented near "Profile_AltGrAliasing.Header". @@ -700,7 +700,7 @@ Header for a control to toggle whether the profile is shown in a dropdown menu, or not. - If checked, the profile will not appear in the list of profiles. This can be used to hide default profiles and dynamically generated profiles, while leaving them in your settings file. + If enabled, the profile will not appear in the list of profiles. This can be used to hide default profiles and dynamically generated profiles, while leaving them in your settings file. A description for what the "hidden" setting does. Presented near "Profile_Hidden". @@ -736,7 +736,7 @@ Header for a control to toggle classic CRT display effects, which gives the terminal a retro look. - When checked, enables retro terminal effects such as glowing text and scan lines. + When enabled, enables retro terminal effects such as glowing text and scan lines. A description for what the "retro terminal effects" setting does. Presented near "Profile_RetroTerminalEffect". @@ -772,7 +772,7 @@ A supplementary setting to the "starting directory" setting. "Parent" refers to the parent process of the current process. - If checked, this profile will spawn in the directory from which Windows Terminal was launched. + If enabled, this profile will spawn in the directory from which Windows Terminal was launched. A description for what the supplementary "use parent process directory" setting does. Presented near "Profile_StartingDirectoryUseParentCheckbox". @@ -808,7 +808,7 @@ A supplementary setting to the "background image" setting. When enabled, the OS desktop wallpaper is used as the background image. Presented near "Profile_BackgroundImage". - When checked, use the desktop wallpaper image as the background image for the terminal. + When enabled, use the desktop wallpaper image as the background image for the terminal. A description for what the supplementary "use desktop image" setting does. Presented near "Profile_UseDesktopImage". @@ -1111,7 +1111,7 @@ A supplementary setting to the "font face" setting. Toggling this control updates the font face control to show all of the fonts installed. - If checked, show all installed fonts in the list above. Otherwise, only show the list of monospace fonts. + If enabled, show all installed fonts in the list above. Otherwise, only show the list of monospace fonts. A description for what the supplementary "show all fonts" setting does. Presented near "Profile_FontFaceShowAllFonts". From cd4aabda84d4cc33ed2f4401a37a154bf18953b7 Mon Sep 17 00:00:00 2001 From: Don-Vito Date: Mon, 9 Aug 2021 21:22:08 +0300 Subject: [PATCH 12/16] Prevent redraw upon resize if new size is equal to old (#10895) ## Summary of the Pull Request Do not invoke terminal resize logic if view port dimensions didn't change ## PR Checklist * [x] Closes #10857 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments Short-circuit `ControlCore::_doResizeUnderLock` if the dimensions of the required view port are equal to the dimensions of the current view port --- src/cascadia/TerminalControl/ControlCore.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 2cc3fef16..ac7bf1b07 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -804,6 +804,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } + // Convert our new dimensions to characters + const auto viewInPixels = Viewport::FromDimensions({ 0, 0 }, + { static_cast(size.cx), static_cast(size.cy) }); + const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels); + const auto currentVP = _terminal->GetViewport(); + + // Don't actually resize if viewport dimensions didn't change + if (vp.Height() == currentVP.Height() && vp.Width() == currentVP.Width()) + { + return; + } + _terminal->ClearSelection(); // Tell the dx engine that our window is now the new size. @@ -812,11 +824,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Invalidate everything _renderer->TriggerRedrawAll(); - // Convert our new dimensions to characters - const auto viewInPixels = Viewport::FromDimensions({ 0, 0 }, - { static_cast(size.cx), static_cast(size.cy) }); - const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels); - // If this function succeeds with S_FALSE, then the terminal didn't // actually change size. No need to notify the connection of this no-op. const HRESULT hr = _terminal->UserResize({ vp.Width(), vp.Height() }); From 7acec306a65d18320401bf05c63896849732d16e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 9 Aug 2021 13:27:20 -0500 Subject: [PATCH 13/16] Account for the window frame when calculating initial position (#10902) ## Summary of the Pull Request Turns out, we'd only ever use the non-client size to calculate the size of the window, but not the actual position. As we learned in #10676, the nonclient area extends a few pixels past the visible borders of the window. ## PR Checklist * [x] Closes #10583 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed * [x] Works with the `IslandWindow` * [x] Works with the `NonClientIslandWindow` --- src/cascadia/WindowsTerminal/AppHost.cpp | 9 ++++-- src/cascadia/WindowsTerminal/IslandWindow.cpp | 29 ++++++++++++++--- src/cascadia/WindowsTerminal/IslandWindow.h | 1 + .../WindowsTerminal/NonClientIslandWindow.cpp | 31 +++++++++++++++---- .../WindowsTerminal/NonClientIslandWindow.h | 1 + 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 092fe8eae..bb1b0889e 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -410,6 +410,7 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode // Get the size of a window we'd need to host that client rect. This will // add the titlebar space. const til::size nonClientSize = _window->GetTotalNonClientExclusiveSize(dpix); + const til::rectangle nonClientFrame = _window->GetNonClientFrame(dpix); adjustedWidth = islandWidth + nonClientSize.width(); adjustedHeight = islandHeight + nonClientSize.height(); @@ -425,14 +426,18 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode const til::size desktopDimensions{ gsl::narrow(nearestMonitorInfo.rcWork.right - nearestMonitorInfo.rcWork.left), gsl::narrow(nearestMonitorInfo.rcWork.bottom - nearestMonitorInfo.rcWork.top) }; - til::point origin{ (proposedRect.left), + // GH#10583 - Adjust the position of the rectangle to account for the size + // of the invisible borders on the left/right. We DON'T want to adjust this + // for the top here - the IslandWindow includes the titlebar in + // nonClientFrame.top, so adjusting for that would actually place the + // titlebar _off_ the monitor. + til::point origin{ (proposedRect.left + nonClientFrame.left()), (proposedRect.top) }; if (_logic.IsQuakeWindow()) { // If we just use rcWork by itself, we'll fail to account for the invisible // space reserved for the resize handles. So retrieve that size here. - const til::size ncSize{ _window->GetTotalNonClientExclusiveSize(dpix) }; const til::size availableSpace = desktopDimensions + nonClientSize; origin = til::point{ diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 50fd2d156..5d8f823de 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -616,12 +616,20 @@ void IslandWindow::SetContent(winrt::Windows::UI::Xaml::UIElement content) } // Method Description: -// - Gets the difference between window and client area size. +// - Get the dimensions of our non-client area, as a rect where each component +// represents that side. +// - The .left will be a negative number, to represent that the actual side of +// the non-client area is outside the border of our window. It's roughly 8px ( +// * DPI scaling) to the left of the visible border. +// - The .right component will be positive, indicating that the nonclient border +// is in the positive-x direction from the edge of our client area. +// - This will also include our titlebar! It's in the nonclient area for us. // Arguments: -// - dpi: dpi of a monitor on which the window is placed -// Return Value -// - The size difference -SIZE IslandWindow::GetTotalNonClientExclusiveSize(const UINT dpi) const noexcept +// - dpi: the scaling that we should use to calculate the border sizes. +// Return Value: +// - a RECT whose components represent the margins of the nonclient area, +// relative to the client area. +RECT IslandWindow::GetNonClientFrame(const UINT dpi) const noexcept { const auto windowStyle = static_cast(GetWindowLong(_window.get(), GWL_STYLE)); RECT islandFrame{}; @@ -630,7 +638,18 @@ SIZE IslandWindow::GetTotalNonClientExclusiveSize(const UINT dpi) const noexcept // the error and go on. We'll use whatever the control proposed as the // size of our window, which will be at least close. LOG_IF_WIN32_BOOL_FALSE(AdjustWindowRectExForDpi(&islandFrame, windowStyle, false, 0, dpi)); + return islandFrame; +} +// Method Description: +// - Gets the difference between window and client area size. +// Arguments: +// - dpi: dpi of a monitor on which the window is placed +// Return Value +// - The size difference +SIZE IslandWindow::GetTotalNonClientExclusiveSize(const UINT dpi) const noexcept +{ + const auto islandFrame{ GetNonClientFrame(dpi) }; return { islandFrame.right - islandFrame.left, islandFrame.bottom - islandFrame.top diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index 8e4a0ec4d..b0699864d 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -24,6 +24,7 @@ public: virtual void OnAppInitialized(); virtual void SetContent(winrt::Windows::UI::Xaml::UIElement content); virtual void OnApplicationThemeChanged(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme); + virtual RECT GetNonClientFrame(const UINT dpi) const noexcept; virtual SIZE GetTotalNonClientExclusiveSize(const UINT dpi) const noexcept; virtual void Initialize(); diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index a1f5e6801..9dc8307ed 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -629,14 +629,21 @@ int NonClientIslandWindow::_GetResizeHandleHeight() const noexcept return DefWindowProc(GetHandle(), WM_SETCURSOR, wParam, lParam); } - // Method Description: -// - Gets the difference between window and client area size. +// - Get the dimensions of our non-client area, as a rect where each component +// represents that side. +// - The .left will be a negative number, to represent that the actual side of +// the non-client area is outside the border of our window. It's roughly 8px ( +// * DPI scaling) to the left of the visible border. +// - The .right component will be positive, indicating that the nonclient border +// is in the positive-x direction from the edge of our client area. +// - This DOES NOT include our titlebar! It's in the client area for us. // Arguments: -// - dpi: dpi of a monitor on which the window is placed -// Return Value -// - The size difference -SIZE NonClientIslandWindow::GetTotalNonClientExclusiveSize(UINT dpi) const noexcept +// - dpi: the scaling that we should use to calculate the border sizes. +// Return Value: +// - a RECT whose components represent the margins of the nonclient area, +// relative to the client area. +RECT NonClientIslandWindow::GetNonClientFrame(UINT dpi) const noexcept { const auto windowStyle = static_cast(GetWindowLong(_window.get(), GWL_STYLE)); RECT islandFrame{}; @@ -647,6 +654,18 @@ SIZE NonClientIslandWindow::GetTotalNonClientExclusiveSize(UINT dpi) const noexc LOG_IF_WIN32_BOOL_FALSE(AdjustWindowRectExForDpi(&islandFrame, windowStyle, false, 0, dpi)); islandFrame.top = -topBorderVisibleHeight; + return islandFrame; +} + +// Method Description: +// - Gets the difference between window and client area size. +// Arguments: +// - dpi: dpi of a monitor on which the window is placed +// Return Value +// - The size difference +SIZE NonClientIslandWindow::GetTotalNonClientExclusiveSize(UINT dpi) const noexcept +{ + const auto islandFrame{ GetNonClientFrame(dpi) }; // If we have a titlebar, this is being called after we've initialized, and // we can just ask that titlebar how big it wants to be. diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h index 10a39b4fc..02e59cc7a 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h @@ -37,6 +37,7 @@ public: [[nodiscard]] virtual LRESULT MessageHandler(UINT const message, WPARAM const wparam, LPARAM const lparam) noexcept override; + virtual RECT GetNonClientFrame(UINT dpi) const noexcept override; virtual SIZE GetTotalNonClientExclusiveSize(UINT dpi) const noexcept override; void Initialize() override; From c55888f88d6b59ae88afb50a806df0dec8fb3a3b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 9 Aug 2021 13:28:06 -0500 Subject: [PATCH 14/16] Make the TerminalApi exception handler less garrulous (#10901) ## Summary of the Pull Request Apparently the exception handler in TerminalApi is far too talkative. We're apparently throwing in `TerminalApi::CursorLineFeed` way too often, and that's caused an internal bug to be filed on us. This represents making the event less talkative, but doesn't actually fix the bug. It's just easier to get the OS bug cleared out quick this way. ## References * MSFT:33310649 ## PR Checklist * [x] Fixes the **A** portion of #10882, which closes MSFT:33310649 * [x] I work here * [n/a] Tests added/passed * [n/a] Requires documentation to be updated --- src/cascadia/TerminalCore/TerminalApi.cpp | 34 +++++++++++------------ src/inc/til.h | 11 +++++++- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 1fc36389d..83c898e47 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -16,7 +16,7 @@ try _WriteBuffer(stringView); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() bool Terminal::ExecuteChar(wchar_t wch) noexcept try @@ -24,7 +24,7 @@ try _WriteBuffer({ &wch, 1 }); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() TextAttribute Terminal::GetTextAttributes() const noexcept { @@ -54,7 +54,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() COORD Terminal::GetCursorPosition() noexcept { @@ -75,7 +75,7 @@ try _buffer->GetCursor().SetColor(color); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Moves the cursor down one line, and possibly also to the leftmost column. @@ -101,7 +101,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - deletes count characters starting from the cursor's current position @@ -150,7 +150,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Inserts count spaces starting from the cursor's current position, moving over the existing text @@ -205,7 +205,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() bool Terminal::EraseCharacters(const size_t numChars) noexcept try @@ -218,7 +218,7 @@ try _buffer->Write(eraseIter, absoluteCursorPos); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method description: // - erases a line of text, either from @@ -264,7 +264,7 @@ try _buffer->Write(eraseIter, startPos, false); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method description: // - erases text in the buffer in two ways depending on erase type @@ -348,7 +348,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() bool Terminal::WarningBell() noexcept try @@ -356,7 +356,7 @@ try _pfnWarningBell(); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() bool Terminal::SetWindowTitle(std::wstring_view title) noexcept try @@ -368,7 +368,7 @@ try } return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Updates the value in the colortable at index tableIndex to the new color @@ -387,7 +387,7 @@ try _buffer->GetRenderTarget().TriggerRedrawAll(); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Sets the cursor style to the given style. @@ -457,7 +457,7 @@ try _buffer->GetRenderTarget().TriggerRedrawAll(); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Updates the default background color from a COLORREF, format 0x00BBGGRR. @@ -475,7 +475,7 @@ try _buffer->GetRenderTarget().TriggerRedrawAll(); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() til::color Terminal::GetDefaultBackground() const noexcept { @@ -509,7 +509,7 @@ try _buffer->GetRenderTarget().TriggerRedrawAll(); return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() bool Terminal::EnableVT200MouseMode(const bool enabled) noexcept { @@ -591,7 +591,7 @@ try return true; } -CATCH_LOG_RETURN_FALSE() +CATCH_RETURN_FALSE() // Method Description: // - Updates the buffer's current text attributes to start a hyperlink diff --git a/src/inc/til.h b/src/inc/til.h index 509f02db9..3202a5959 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -89,7 +89,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } \ } while (0, 0) -// Due to a bug (DevDiv 441931), Warning 4297 (function marked noexcept throws exception) is detected even when the throwing code is unreachable, such as the end of scope after a return, in function-level catch. +// Due to a bug (DevDiv 441931), Warning 4297 (function marked noexcept throws +// exception) is detected even when the throwing code is unreachable, such as +// the end of scope after a return, in function-level catch. #define CATCH_LOG_RETURN_FALSE() \ catch (...) \ { \ @@ -98,6 +100,13 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return false; \ } +// This is like the above, but doesn't log any messages. This is for GH#10882. +#define CATCH_RETURN_FALSE() \ + catch (...) \ + { \ + return false; \ + } + // MultiByteToWideChar has a bug in it where it can return 0 and then not set last error. // WIL has a fit if the last error is 0 when a bool false is returned. // This macro doesn't have a fit. It just reports E_UNEXPECTED instead. From a14b6f89f686bda084fd45104806182b780d6586 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 10 Aug 2021 06:16:17 -0500 Subject: [PATCH 15/16] Combine progress states in the tab, taskbar (#10755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request ![background-progress-000](https://user-images.githubusercontent.com/18356694/126653006-3ad2fdae-67ae-4cdb-aa46-25d09217e365.gif) This PR causes the Terminal to combine taskbar states at the tab and window level, according to the [MSDN docs for `SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group). This allows the Terminal's taskbar icon to continue showing progress information, even if you're in a pane/tab that _doesn't_ have progress state. This is helpful for cases where the user may be running a build in one tab, and working on something else in another. ## References * [`SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group) * Progress mega: #6700 ## PR Checklist * [x] Closes #10090 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments This also fixes a related bug where transitioning from the "error" or "warning" state directly to the "indeterminate" state would cause the taskbar icon to get stuck in a bad state. ## Validation Steps Performed
progress.cmd ```cmd @echo off setlocal enabledelayedexpansion set _type=3 if (%1) == () ( set _type=3 ) else ( set _type=%1 ) if (%_type%) == (0) ( --- src/cascadia/TerminalApp/AppLogic.cpp | 21 +----- src/cascadia/TerminalApp/AppLogic.h | 3 +- src/cascadia/TerminalApp/AppLogic.idl | 3 +- src/cascadia/TerminalApp/Pane.cpp | 23 ++++++ src/cascadia/TerminalApp/Pane.h | 3 + src/cascadia/TerminalApp/TaskbarState.cpp | 45 +++++++++++ src/cascadia/TerminalApp/TaskbarState.h | 34 +++++++++ src/cascadia/TerminalApp/TaskbarState.idl | 15 ++++ .../TerminalApp/TerminalAppLib.vcxproj | 7 ++ src/cascadia/TerminalApp/TerminalPage.cpp | 42 ++++++----- src/cascadia/TerminalApp/TerminalPage.h | 3 +- src/cascadia/TerminalApp/TerminalPage.idl | 4 +- src/cascadia/TerminalApp/TerminalTab.cpp | 75 ++++++++++++------- src/cascadia/TerminalApp/TerminalTab.h | 1 + src/cascadia/WindowsTerminal/AppHost.cpp | 9 ++- src/cascadia/WindowsTerminal/IslandWindow.cpp | 7 +- 16 files changed, 218 insertions(+), 77 deletions(-) create mode 100644 src/cascadia/TerminalApp/TaskbarState.cpp create mode 100644 src/cascadia/TerminalApp/TaskbarState.h create mode 100644 src/cascadia/TerminalApp/TaskbarState.idl diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index c14a951de..420630774 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -1129,28 +1129,11 @@ namespace winrt::TerminalApp::implementation } } - // Method Description: - // - Gets the taskbar state value from the last active control - // Return Value: - // - The taskbar state of the last active control - uint64_t AppLogic::GetLastActiveControlTaskbarState() + winrt::TerminalApp::TaskbarState AppLogic::TaskbarState() { if (_root) { - return _root->GetLastActiveControlTaskbarState(); - } - return {}; - } - - // Method Description: - // - Gets the taskbar progress value from the last active control - // Return Value: - // - The taskbar progress of the last active control - uint64_t AppLogic::GetLastActiveControlTaskbarProgress() - { - if (_root) - { - return _root->GetLastActiveControlTaskbarProgress(); + return _root->TaskbarState(); } return {}; } diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 6d2e36d9e..4131d28f1 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -90,8 +90,7 @@ namespace winrt::TerminalApp::implementation void WindowCloseButtonClicked(); - uint64_t GetLastActiveControlTaskbarState(); - uint64_t GetLastActiveControlTaskbarProgress(); + winrt::TerminalApp::TaskbarState TaskbarState(); winrt::Windows::Foundation::IAsyncOperation ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog); diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index bb334d26f..0b8891038 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -68,8 +68,7 @@ namespace TerminalApp void TitlebarClicked(); void WindowCloseButtonClicked(); - UInt64 GetLastActiveControlTaskbarState(); - UInt64 GetLastActiveControlTaskbarProgress(); + TaskbarState TaskbarState{ get; }; FindTargetWindowResult FindTargetWindow(String[] args); diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 59dacf6b9..45db24870 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -2548,6 +2548,29 @@ bool Pane::ContainsReadOnly() const return _IsLeaf() ? _control.ReadOnly() : (_firstChild->ContainsReadOnly() || _secondChild->ContainsReadOnly()); } +// Method Description: +// - If we're a parent, place the taskbar state for all our leaves into the +// provided vector. +// - If we're a leaf, place our own state into the vector. +// Arguments: +// - states: a vector that will receive all the states of all leaves in the tree +// Return Value: +// - +void Pane::CollectTaskbarStates(std::vector& states) +{ + if (_IsLeaf()) + { + auto tbState{ winrt::make(_control.TaskbarState(), + _control.TaskbarProgress()) }; + states.push_back(tbState); + } + else + { + _firstChild->CollectTaskbarStates(states); + _secondChild->CollectTaskbarStates(states); + } +} + DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate>); DEFINE_EVENT(Pane, LostFocus, _LostFocusHandlers, winrt::delegate>); DEFINE_EVENT(Pane, PaneRaiseBell, _PaneRaiseBellHandlers, winrt::Windows::Foundation::EventHandler); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index eed685d32..34d696340 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -21,6 +21,7 @@ #pragma once #include "../../cascadia/inc/cppwinrt_utils.h" +#include "TaskbarState.h" // fwdecl unittest classes namespace TerminalAppLocalTests @@ -92,6 +93,8 @@ public: bool ContainsReadOnly() const; + void CollectTaskbarStates(std::vector& states); + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate>); DECLARE_EVENT(LostFocus, _LostFocusHandlers, winrt::delegate>); diff --git a/src/cascadia/TerminalApp/TaskbarState.cpp b/src/cascadia/TerminalApp/TaskbarState.cpp new file mode 100644 index 000000000..183460556 --- /dev/null +++ b/src/cascadia/TerminalApp/TaskbarState.cpp @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "pch.h" +#include "TaskbarState.h" +#include "TaskbarState.g.cpp" + +namespace winrt::TerminalApp::implementation +{ + // Default to unset, 0%. + TaskbarState::TaskbarState() : + TaskbarState(0, 0){}; + + TaskbarState::TaskbarState(const uint64_t dispatchTypesState, const uint64_t progressParam) : + _State{ dispatchTypesState }, + _Progress{ progressParam } {} + + uint64_t TaskbarState::Priority() const + { + // This seemingly nonsensical ordering is from + // https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group + switch (_State) + { + case 0: // Clear = 0, + return 5; + case 1: // Set = 1, + return 3; + case 2: // Error = 2, + return 1; + case 3: // Indeterminate = 3, + return 4; + case 4: // Paused = 4 + return 2; + } + // Here, return 6, to definitely be greater than all the other valid values. + // This should never really happen. + return 6; + } + + int TaskbarState::ComparePriority(const winrt::TerminalApp::TaskbarState& lhs, const winrt::TerminalApp::TaskbarState& rhs) + { + return lhs.Priority() < rhs.Priority(); + } + +} diff --git a/src/cascadia/TerminalApp/TaskbarState.h b/src/cascadia/TerminalApp/TaskbarState.h new file mode 100644 index 000000000..e36b4440c --- /dev/null +++ b/src/cascadia/TerminalApp/TaskbarState.h @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once +#include "inc/cppwinrt_utils.h" +#include "TaskbarState.g.h" + +// fwdecl unittest classes +namespace TerminalAppLocalTests +{ + class TabTests; +}; + +namespace winrt::TerminalApp::implementation +{ + struct TaskbarState : TaskbarStateT + { + public: + TaskbarState(); + TaskbarState(const uint64_t dispatchTypesState, const uint64_t progress); + + static int ComparePriority(const winrt::TerminalApp::TaskbarState& lhs, const winrt::TerminalApp::TaskbarState& rhs); + + uint64_t Priority() const; + + WINRT_PROPERTY(uint64_t, State, 0); + WINRT_PROPERTY(uint64_t, Progress, 0); + }; +} + +namespace winrt::TerminalApp::factory_implementation +{ + BASIC_FACTORY(TaskbarState); +} diff --git a/src/cascadia/TerminalApp/TaskbarState.idl b/src/cascadia/TerminalApp/TaskbarState.idl new file mode 100644 index 000000000..159242a17 --- /dev/null +++ b/src/cascadia/TerminalApp/TaskbarState.idl @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace TerminalApp +{ + [default_interface] runtimeclass TaskbarState + { + TaskbarState(); + TaskbarState(UInt64 dispatchTypesState, UInt64 progress); + + UInt64 State{ get; }; + UInt64 Progress{ get; }; + UInt64 Priority { get; }; + } +} diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj index 25cd7cfa3..4d9b5d3dc 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj @@ -90,6 +90,9 @@ TabBase.idl + + TaskbarState.idl + TerminalTab.idl @@ -166,6 +169,9 @@ TabBase.idl + + TaskbarState.idl + TerminalTab.idl @@ -258,6 +264,7 @@ + TerminalPage.xaml diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 3bbf27eab..4d49779ad 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -2077,29 +2077,35 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Gets the taskbar state value from the last active control + // - Get the combined taskbar state for the page. This is the combination of + // all the states of all the tabs, which are themselves a combination of + // all their panes. Taskbar states are given a priority based on the rules + // in: + // https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate + // under "How the Taskbar Button Chooses the Progress Indicator for a Group" + // Arguments: + // - // Return Value: - // - The taskbar state of the last active control - uint64_t TerminalPage::GetLastActiveControlTaskbarState() + // - A TaskbarState object representing the combined taskbar state and + // progress percentage of all our tabs. + winrt::TerminalApp::TaskbarState TerminalPage::TaskbarState() const { - if (auto control{ _GetActiveControl() }) - { - return control.TaskbarState(); - } - return {}; - } + auto state{ winrt::make() }; - // Method Description: - // - Gets the taskbar progress value from the last active control - // Return Value: - // - The taskbar progress of the last active control - uint64_t TerminalPage::GetLastActiveControlTaskbarProgress() - { - if (auto control{ _GetActiveControl() }) + for (const auto& tab : _tabs) { - return control.TaskbarProgress(); + if (auto tabImpl{ _GetTerminalTabImpl(tab) }) + { + auto tabState{ tabImpl->GetCombinedTaskbarState() }; + // lowest priority wins + if (tabState.Priority() < state.Priority()) + { + state = tabState; + } + } } - return {}; + + return state; } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index bb1a6f13e..6ec7bd7a0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -85,8 +85,7 @@ namespace winrt::TerminalApp::implementation winrt::TerminalApp::IDialogPresenter DialogPresenter() const; void DialogPresenter(winrt::TerminalApp::IDialogPresenter dialogPresenter); - uint64_t GetLastActiveControlTaskbarState(); - uint64_t GetLastActiveControlTaskbarProgress(); + winrt::TerminalApp::TaskbarState TaskbarState() const; void ShowKeyboardServiceWarning(); winrt::hstring KeyboardServiceDisabledText(); diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl index 0d0557822..7d4c580e9 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import "TaskbarState.idl"; namespace TerminalApp { @@ -42,8 +43,7 @@ namespace TerminalApp void ShowKeyboardServiceWarning(); String KeyboardServiceDisabledText { get; }; - UInt64 GetLastActiveControlTaskbarState(); - UInt64 GetLastActiveControlTaskbarProgress(); + TaskbarState TaskbarState{ get; }; event Windows.Foundation.TypedEventHandler TitleChanged; event Windows.Foundation.TypedEventHandler LastTabClosed; diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index ad4595a1b..48cf05d33 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -177,10 +177,9 @@ namespace winrt::TerminalApp::implementation { lastFocusedControl.Focus(_focusState); - // Update our own progress state, and fire an event signaling + // Update our own progress state. This will fire an event signaling // that our taskbar progress changed. _UpdateProgressState(); - _TaskbarProgressChangedHandlers(lastFocusedControl, nullptr); } // When we gain focus, remove the bell indicator if it is active if (_tabStatus.BellIndicator()) @@ -675,6 +674,26 @@ namespace winrt::TerminalApp::implementation }); } + // Method Description: + // - Get the combined taskbar state for the tab. This is the combination of + // all the states of all our panes. Taskbar states are given a priority + // based on the rules in: + // https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate + // under "How the Taskbar Button Chooses the Progress Indicator for a + // Group" + // Arguments: + // - + // Return Value: + // - A TaskbarState object representing the combined taskbar state and + // progress percentage of all our panes. + winrt::TerminalApp::TaskbarState TerminalTab::GetCombinedTaskbarState() const + { + std::vector states; + _rootPane->CollectTaskbarStates(states); + return states.empty() ? winrt::make() : + *std::min_element(states.begin(), states.end(), TerminalApp::implementation::TaskbarState::ComparePriority); + } + // Method Description: // - This should be called on the UI thread. If you don't, then it might // silently do nothing. @@ -690,37 +709,39 @@ namespace winrt::TerminalApp::implementation // - void TerminalTab::_UpdateProgressState() { - if (const auto& activeControl{ GetActiveTerminalControl() }) - { - const auto taskbarState = activeControl.TaskbarState(); - // The progress of the control changed, but not necessarily the progress of the tab. - // Set the tab's progress ring to the active pane's progress - if (taskbarState > 0) - { - if (taskbarState == 3) - { - // 3 is the indeterminate state, set the progress ring as such - _tabStatus.IsProgressRingIndeterminate(true); - } - else - { - // any non-indeterminate state has a value, set the progress ring as such - _tabStatus.IsProgressRingIndeterminate(false); + const auto state{ GetCombinedTaskbarState() }; - const auto progressValue = gsl::narrow(activeControl.TaskbarProgress()); - _tabStatus.ProgressValue(progressValue); - } - // Hide the tab icon (the progress ring is placed over it) - HideIcon(true); - _tabStatus.IsProgressRingActive(true); + const auto taskbarState = state.State(); + // The progress of the control changed, but not necessarily the progress of the tab. + // Set the tab's progress ring to the active pane's progress + if (taskbarState > 0) + { + if (taskbarState == 3) + { + // 3 is the indeterminate state, set the progress ring as such + _tabStatus.IsProgressRingIndeterminate(true); } else { - // Show the tab icon - HideIcon(false); - _tabStatus.IsProgressRingActive(false); + // any non-indeterminate state has a value, set the progress ring as such + _tabStatus.IsProgressRingIndeterminate(false); + + const auto progressValue = gsl::narrow(state.Progress()); + _tabStatus.ProgressValue(progressValue); } + // Hide the tab icon (the progress ring is placed over it) + HideIcon(true); + _tabStatus.IsProgressRingActive(true); } + else + { + // Show the tab icon + HideIcon(false); + _tabStatus.IsProgressRingActive(false); + } + + // fire an event signaling that our taskbar progress changed. + _TaskbarProgressChangedHandlers(nullptr, nullptr); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index f3cee7555..6a9c7808c 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -83,6 +83,7 @@ namespace winrt::TerminalApp::implementation void TogglePaneReadOnly(); std::shared_ptr GetActivePane() const; + winrt::TerminalApp::TaskbarState GetCombinedTaskbarState() const; winrt::TerminalApp::TerminalTabStatus TabStatus() { diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index bb1b0889e..5f821d190 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -126,13 +126,14 @@ bool AppHost::OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, cons // Arguments: // - sender: not used // - args: not used -void AppHost::SetTaskbarProgress(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::Foundation::IInspectable& /*args*/) +void AppHost::SetTaskbarProgress(const winrt::Windows::Foundation::IInspectable& /*sender*/, + const winrt::Windows::Foundation::IInspectable& /*args*/) { if (_logic) { - const auto state = gsl::narrow_cast(_logic.GetLastActiveControlTaskbarState()); - const auto progress = gsl::narrow_cast(_logic.GetLastActiveControlTaskbarProgress()); - _window->SetTaskbarProgress(state, progress); + const auto state = _logic.TaskbarState(); + _window->SetTaskbarProgress(gsl::narrow_cast(state.State()), + gsl::narrow_cast(state.Progress())); } } diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 5d8f823de..5cfffa4fc 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -776,7 +776,12 @@ void IslandWindow::SetTaskbarProgress(const size_t state, const size_t progress) _taskbar->SetProgressValue(_window.get(), progress, 100); break; case 3: - // sets the progress indicator to an indeterminate state + // sets the progress indicator to an indeterminate state. + // FIRST, set the progress to "no progress". That'll clear out any + // progress value from the previous state. Otherwise, a transition + // from (error,x%) or (warning,x%) to indeterminate will leave the + // progress value unchanged, and not show the spinner. + _taskbar->SetProgressState(_window.get(), TBPF_NOPROGRESS); _taskbar->SetProgressState(_window.get(), TBPF_INDETERMINATE); break; case 4: From ebf41dd6b2e3d968262b4c53bc506c107c82bf0c Mon Sep 17 00:00:00 2001 From: Floris Westerman Date: Tue, 10 Aug 2021 21:53:07 +0200 Subject: [PATCH 16/16] Adding/fixing Alt+Space handling (#10799) ## Summary of the Pull Request This PR implements/solves #7125. Concretely: two requests regarding alt+space were posted there: 1. Disabling the alt+space menu when the keychord explicitly unbound - and forwarding the keystroke to the terminal 2. Disabling the alt+space menu when the keychord is bound to an action ## References Not that I know ## PR Checklist * [x] Closes #7125 * [x] CLA signed. * [x] Tests added/passed * [x] Documentation updated. N/A * [x] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. The issue was marked Help-Wanted. I am happy to change the implementation to better fit your (planned) architecture. ## Detailed Description of the Pull Request / Additional comments While researching the solution, I noticed that the XAML system was always opening the system menu after Alt+Space, even when explicitly setting the event to be handled according to the documentation. The only solution I could find was to hook into the "XAML bypass" already in place for F7 KeyDown, and Alt KeyUp keystrokes. This bypass sends the keystroke to the AppHost immediately. This bypass method will "fall back" to the normal XAML routing when the keystroke is not handled. The implemented behaviour is as follows: - Default: same as normal; system menu is working since the bypass does not handle the keystroke - Alt+Space explicitly unbound: bypass passes the keystroke to the terminal and marks it as handled - Alt+Space bound to command: bypass invokes the command and marks it as handled Concretely, added a method to the KeyBindings and ActionMap interfaces to check whether a keychord is explicitly unbound. The implementation for `_GetActionByKeyChordInternal` already distinguishes between explicitly unbound and lack of binding, however this distinction is not carried over to the public methods. I decided not to change this existing method, to avoid breaking other stuff and to make the API more explicit. Furthermore, there were some checks against Alt+Space further down in the code, preventing this keystroke from being entered in the terminal. Since the check for this keystroke is now done at a "higher" level, I thought I could safely remove these checks as otherwise the keystroke could never be sent to the terminal itself. Please correct me if I'm wrong. Note that when alt+space is bound to an action that opens the command pallette (such as tab search), then a second press of the key combination does still open the system menu. This is because at that point, the "bypass" is cancelled (called "not a good implementation" in #4031). I don't think this can easily be solved for now, but this is a very minor bug/inconvenience. ## Validation Steps Performed Added tests for the new method. Performed manual checking: * [x] Default configuration still opens system menu like normal * [x] Binding alt+space to an action performs the action and does not show the system menu * [x] Explicitly unbinding alt+space no longer shows the system menu and sends the keystroke to the terminal. I was unable to run the debug tap (it crashed my instance - same thing happening on preview and release builds) to check for sure, but behaviour was identical to native linux terminals. --- .../KeyBindingsTests.cpp | 27 ++++++++++++++- src/cascadia/TerminalApp/AppKeyBindings.cpp | 5 +++ src/cascadia/TerminalApp/AppKeyBindings.h | 1 + src/cascadia/TerminalControl/IKeyBindings.idl | 1 + src/cascadia/TerminalControl/TermControl.cpp | 33 ++++++++++++------- src/cascadia/TerminalCore/Terminal.cpp | 15 --------- .../TerminalSettingsModel/ActionMap.cpp | 26 +++++++++++++++ .../TerminalSettingsModel/ActionMap.h | 1 + .../TerminalSettingsModel/ActionMap.idl | 2 ++ .../UnitTests_TerminalCore/InputTest.cpp | 11 ------- src/cascadia/WindowsTerminal/main.cpp | 16 +++++++++ 11 files changed, 100 insertions(+), 38 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp index a5479f6a7..e5c354d25 100644 --- a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp @@ -40,7 +40,7 @@ namespace SettingsModelLocalTests TEST_METHOD(ManyKeysSameAction); TEST_METHOD(LayerKeybindings); TEST_METHOD(UnbindKeybindings); - + TEST_METHOD(TestExplicitUnbind); TEST_METHOD(TestArbitraryArgs); TEST_METHOD(TestSplitPaneArgs); @@ -232,6 +232,31 @@ namespace SettingsModelLocalTests VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast('C'), 0 })); } + void KeyBindingsTests::TestExplicitUnbind() + { + const std::string bindings0String{ R"([ { "command": "copy", "keys": ["ctrl+c"] } ])" }; + const std::string bindings1String{ R"([ { "command": "unbound", "keys": ["ctrl+c"] } ])" }; + const std::string bindings2String{ R"([ { "command": "copy", "keys": ["ctrl+c"] } ])" }; + + const auto bindings0Json = VerifyParseSucceeded(bindings0String); + const auto bindings1Json = VerifyParseSucceeded(bindings1String); + const auto bindings2Json = VerifyParseSucceeded(bindings2String); + + const KeyChord keyChord{ VirtualKeyModifiers::Control, static_cast('C'), 0 }; + + auto actionMap = winrt::make_self(); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings0Json); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings1Json); + VERIFY_IS_TRUE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings2Json); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + } + void KeyBindingsTests::TestArbitraryArgs() { const std::string bindings0String{ R"([ diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 71a3a03bb..3f921bc55 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -21,6 +21,11 @@ namespace winrt::TerminalApp::implementation return false; } + bool AppKeyBindings::IsKeyChordExplicitlyUnbound(const KeyChord& kc) + { + return _actionMap.IsKeyChordExplicitlyUnbound(kc); + } + void AppKeyBindings::SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch) { _dispatch = dispatch; diff --git a/src/cascadia/TerminalApp/AppKeyBindings.h b/src/cascadia/TerminalApp/AppKeyBindings.h index 5004edeca..f4f6c20ba 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.h +++ b/src/cascadia/TerminalApp/AppKeyBindings.h @@ -20,6 +20,7 @@ namespace winrt::TerminalApp::implementation AppKeyBindings() = default; bool TryKeyChord(winrt::Microsoft::Terminal::Control::KeyChord const& kc); + bool IsKeyChordExplicitlyUnbound(winrt::Microsoft::Terminal::Control::KeyChord const& kc); void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch); void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); diff --git a/src/cascadia/TerminalControl/IKeyBindings.idl b/src/cascadia/TerminalControl/IKeyBindings.idl index 2b3901559..83d665faa 100644 --- a/src/cascadia/TerminalControl/IKeyBindings.idl +++ b/src/cascadia/TerminalControl/IKeyBindings.idl @@ -9,5 +9,6 @@ namespace Microsoft.Terminal.Control interface IKeyBindings { Boolean TryKeyChord(KeyChord kc); + Boolean IsKeyChordExplicitlyUnbound(KeyChord kc); } } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 0d20606d1..238d75c90 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -761,28 +761,39 @@ namespace winrt::Microsoft::Terminal::Control::implementation (void)_TrySendKeyEvent(VK_MENU, scanCode, modifiers, false); handled = true; } - else if (vkey == VK_F7 && down) + else if ((vkey == VK_F7 || vkey == VK_SPACE) && down) { // Manually generate an F7 event into the key bindings or terminal. // This is required as part of GH#638. + // Or do so for alt+space; only send to terminal when explicitly unbound + // That is part of #GH7125 auto bindings{ _settings.KeyBindings() }; + bool isUnbound = false; + const KeyChord kc = { + modifiers.IsCtrlPressed(), + modifiers.IsAltPressed(), + modifiers.IsShiftPressed(), + modifiers.IsWinPressed(), + gsl::narrow_cast(vkey), + 0 + }; if (bindings) { - handled = bindings.TryKeyChord({ - modifiers.IsCtrlPressed(), - modifiers.IsAltPressed(), - modifiers.IsShiftPressed(), - modifiers.IsWinPressed(), - VK_F7, - 0, - }); + handled = bindings.TryKeyChord(kc); + + if (!handled) + { + isUnbound = bindings.IsKeyChordExplicitlyUnbound(kc); + } } - if (!handled) + const bool sendToTerminal = vkey == VK_F7 || (vkey == VK_SPACE && isUnbound); + + if (!handled && sendToTerminal) { // _TrySendKeyEvent pretends it didn't handle F7 for some unknown reason. - (void)_TrySendKeyEvent(VK_F7, scanCode, modifiers, true); + (void)_TrySendKeyEvent(gsl::narrow_cast(vkey), scanCode, modifiers, true); // GH#6438: Note that we're _not_ sending the key up here - that'll // get passed through XAML to our KeyUp handler normally. handled = true; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 68e8424ed..53ca70878 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -594,14 +594,6 @@ bool Terminal::SendKeyEvent(const WORD vkey, const auto isAltOnlyPressed = states.IsAltPressed() && !states.IsCtrlPressed(); - // DON'T manually handle Alt+Space - the system will use this to bring up - // the system menu for restore, min/maximize, size, move, close. - // (This doesn't apply to Ctrl+Alt+Space.) - if (isAltOnlyPressed && vkey == VK_SPACE) - { - return false; - } - // By default Windows treats Ctrl+Alt as an alias for AltGr. // When the altGrAliasing setting is set to false, this behaviour should be disabled. // @@ -672,13 +664,6 @@ bool Terminal::SendMouseEvent(const COORD viewportPos, const unsigned int uiButt // - false otherwise. bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const ControlKeyStates states) { - // DON'T manually handle Alt+Space - the system will use this to bring up - // the system menu for restore, min/maximize, size, move, close. - if (ch == L' ' && states.IsAltPressed() && !states.IsCtrlPressed()) - { - return false; - } - auto vkey = _TakeVirtualKeyFromLastKeyEvent(scanCode); if (vkey == 0 && scanCode != 0) { diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index b0740f8d3..75a1213ad 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -677,6 +677,32 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } } + // Method Description: + // - Determines whether the given key chord is explicitly unbound + // Arguments: + // - keys: the key chord to check + // Return value: + // - true if the keychord is explicitly unbound + // - false if either the keychord is bound, or not bound at all + bool ActionMap::IsKeyChordExplicitlyUnbound(Control::KeyChord const& keys) const + { + const auto modifiers = keys.Modifiers(); + + // The "keys" given to us can contain both a Vkey, as well as a ScanCode. + // For instance our UI code fills out a KeyChord with all available information. + // But our _KeyMap only contains KeyChords that contain _either_ a Vkey or ScanCode. + // Due to this we'll have to call _GetActionByKeyChordInternal twice. + if (auto vkey = keys.Vkey()) + { + // We use the fact that the ..Internal call returns nullptr for explicitly unbound + // key chords, and nullopt for keychord that are not bound - it allows us to distinguish + // between unbound and lack of binding. + return nullptr == _GetActionByKeyChordInternal({ modifiers, vkey, 0 }); + } + + return nullptr == _GetActionByKeyChordInternal({ modifiers, 0, keys.ScanCode() }); + } + // Method Description: // - Retrieves the assigned command that can be invoked with the given key chord // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h index c81bd14c3..a79c2c6e8 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.h +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -61,6 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // queries Model::Command GetActionByKeyChord(Control::KeyChord const& keys) const; + bool IsKeyChordExplicitlyUnbound(Control::KeyChord const& keys) const; Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action) const; Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action, IActionArgs const& actionArgs) const; diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.idl b/src/cascadia/TerminalSettingsModel/ActionMap.idl index 0a9c4d07c..806baa17a 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.idl +++ b/src/cascadia/TerminalSettingsModel/ActionMap.idl @@ -8,6 +8,8 @@ namespace Microsoft.Terminal.Settings.Model // This interface ensures that no changes are made to ActionMap interface IActionMapView { + Boolean IsKeyChordExplicitlyUnbound(Microsoft.Terminal.Control.KeyChord keys); + Command GetActionByKeyChord(Microsoft.Terminal.Control.KeyChord keys); Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(ShortcutAction action); diff --git a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp index 131e410d0..34dd9118b 100644 --- a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp @@ -29,7 +29,6 @@ namespace TerminalCoreUnitTests }; TEST_METHOD(AltShiftKey); - TEST_METHOD(AltSpace); TEST_METHOD(InvalidKeyEvent); void _VerifyExpectedInput(std::wstring& actualInput) @@ -57,16 +56,6 @@ namespace TerminalCoreUnitTests VERIFY_IS_TRUE(term.SendCharEvent(L'A', 0, ControlKeyStates::LeftAltPressed | ControlKeyStates::ShiftPressed)); } - void InputTest::AltSpace() - { - // Make sure we don't handle Alt+Space. The system will use this to - // bring up the system menu for restore, min/maximize, size, move, - // close - VERIFY_IS_FALSE(term.SendKeyEvent(L' ', 0, ControlKeyStates::LeftAltPressed, true)); - VERIFY_IS_FALSE(term.SendKeyEvent(L' ', 0, ControlKeyStates::LeftAltPressed, false)); - VERIFY_IS_FALSE(term.SendCharEvent(L' ', 0, ControlKeyStates::LeftAltPressed)); - } - void InputTest::InvalidKeyEvent() { // Certain applications like AutoHotKey and its keyboard remapping feature, diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index 0bf491de7..7fb2523a5 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -82,6 +82,10 @@ static bool _messageIsAltKeyup(const MSG& message) { return (message.message == WM_KEYUP || message.message == WM_SYSKEYUP) && message.wParam == VK_MENU; } +static bool _messageIsAltSpaceKeypress(const MSG& message) +{ + return message.message == WM_SYSKEYDOWN && message.wParam == VK_SPACE; +} int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) { @@ -171,6 +175,18 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) } } + // GH#7125 = System XAML will show a system dialog on Alt Space. We might want to + // explicitly prevent that - for example when an action is bound to it. So similar to + // above, we steal the event and hand it off to the host. When the host does not process + // it, we will still dispatch like normal. + if (_messageIsAltSpaceKeypress(message)) + { + if (host.OnDirectKeyEvent(VK_SPACE, LOBYTE(HIWORD(message.lParam)), true)) + { + continue; + } + } + TranslateMessage(&message); DispatchMessage(&message); }