Skip to content

Commit

Permalink
Fix font axis/feature SUI issues (#17173)
Browse files Browse the repository at this point in the history 
Something something code changes, fixes issue. Events need throwing,
some events don't need throwing, some events need throttling to
not overwhelm the flurble function within the gumbies component.
This is all boilerplate and it works now.

Closes #17171
Closes #17172

## Validation Steps Performed
* Resetting the font axis/feature expander keeps
  the add new button flyout sorted ✅
* Changing a font axis/feature value updates the preview ✅
* After changing the font, features/axes can still be edited ✅
  • Loading branch information
@lhecker
lhecker committed May 2, 2024
1 parent 353fe7b commit ae91734
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 78 deletions.
5 changes: 5 additions & 0 deletions .github/actions/spelling/expect/expect.txt
View file
Expand Up @@ -245,6 +245,7 @@ CONKBD
conlibk
conmsgl
CONNECTINFO
connyection
CONOUT
conprops
conpropsp
Expand Down Expand Up @@ -1420,6 +1421,8 @@ PUCHAR
pvar
pwch
PWDDMCONSOLECONTEXT
Pwease
pweview
pws
pwstr
pwsz
Expand Down Expand Up @@ -1898,6 +1901,7 @@ UVWXY
UVWXYZ
uwa
uwp
uwu
uxtheme
Vanara
vararg
Expand Down Expand Up @@ -1971,6 +1975,7 @@ wdm
webpage
websites
wekyb
wewoad
wex
wextest
wextestclass
Expand Down
157 changes: 105 additions & 52 deletions src/cascadia/TerminalSettingsEditor/Appearances.cpp
View file
Expand Up @@ -544,14 +544,33 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
return item;
}

// Call this when all the _fontFaceDependents members have changed.
void AppearanceViewModel::_notifyChangesForFontSettings()
{
_NotifyChanges(
L"FontFaceDependents",
L"FontAxes",
L"FontFeatures",
L"HasFontAxes",
L"HasFontFeatures");
_NotifyChanges(L"FontFaceDependents");
_NotifyChanges(L"FontAxes");
_NotifyChanges(L"FontFeatures");
_NotifyChanges(L"HasFontAxes");
_NotifyChanges(L"HasFontFeatures");
}

// Call this when used items moved into unused and vice versa.
// Because this doesn't recreate the IObservableVector instances,
// we don't need to notify the UI about changes to the "FontAxes" property.
void AppearanceViewModel::_notifyChangesForFontSettingsReactive(FontSettingIndex fontSettingsIndex)
{
_NotifyChanges(L"FontFaceDependents");
switch (fontSettingsIndex)
{
case FontAxesIndex:
_NotifyChanges(L"HasFontAxes");
break;
case FontFeaturesIndex:
_NotifyChanges(L"HasFontFeatures");
break;
default:
break;
}
}

double AppearanceViewModel::LineHeight() const
Expand Down Expand Up @@ -616,6 +635,30 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
FontWeight(winrt::Microsoft::Terminal::UI::Converters::DoubleToFontWeight(fontWeight));
}

const AppearanceViewModel::FontFaceDependentsData& AppearanceViewModel::FontFaceDependents()
{
if (!_fontFaceDependents)
{
_refreshFontFaceDependents();
}
return *_fontFaceDependents;
}

winrt::hstring AppearanceViewModel::MissingFontFaces()
{
return FontFaceDependents().missingFontFaces;
}

winrt::hstring AppearanceViewModel::ProportionalFontFaces()
{
return FontFaceDependents().proportionalFontFaces;
}

bool AppearanceViewModel::HasPowerlineCharacters()
{
return FontFaceDependents().hasPowerlineCharacters;
}

IObservableVector<Editor::FontKeyValuePair> AppearanceViewModel::FontAxes()
{
return FontFaceDependents().fontSettingsUsed[FontAxesIndex];
Expand All @@ -628,7 +671,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void AppearanceViewModel::ClearFontAxes()
{
_deleteAllFontSettings(FontAxesIndex);
_deleteAllFontKeyValuePairs(FontAxesIndex);
}

Model::FontConfig AppearanceViewModel::FontAxesOverrideSource() const
Expand All @@ -648,7 +691,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void AppearanceViewModel::ClearFontFeatures()
{
_deleteAllFontSettings(FontFeaturesIndex);
_deleteAllFontKeyValuePairs(FontFeaturesIndex);
}

Model::FontConfig AppearanceViewModel::FontFeaturesOverrideSource() const
Expand All @@ -664,7 +707,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}

const auto kvImpl = winrt::get_self<FontKeyValuePair>(kv);
const auto fontSettingsIndex = kvImpl->IsFontFeature() ? 1 : 0;
const auto fontSettingsIndex = kvImpl->IsFontFeature() ? FontFeaturesIndex : FontAxesIndex;
auto& d = *_fontFaceDependents;
auto& used = d.fontSettingsUsed[fontSettingsIndex];
auto& unused = d.fontSettingsUnused[fontSettingsIndex];
Expand All @@ -686,7 +729,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

unused.erase(it);

_notifyChangesForFontSettings();
_notifyChangesForFontSettingsReactive(fontSettingsIndex);
}

void AppearanceViewModel::DeleteFontKeyValuePair(const Editor::FontKeyValuePair& kv)
Expand All @@ -699,10 +742,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
const auto kvImpl = winrt::get_self<FontKeyValuePair>(kv);
const auto tag = kvImpl->Key();
const auto tagString = tagToString(tag);
const auto fontSettingsIndex = kvImpl->IsFontFeature() ? 1 : 0;
const auto fontSettingsIndex = kvImpl->IsFontFeature() ? FontFeaturesIndex : FontAxesIndex;
auto& d = *_fontFaceDependents;
auto& used = d.fontSettingsUsed[fontSettingsIndex];
auto& unused = d.fontSettingsUnused[fontSettingsIndex];

const auto fontInfo = _appearance.SourceProfile().FontInfo();
auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes();
Expand All @@ -719,47 +761,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

fontSettingsUser.Remove(std::wstring_view{ tagString });

// Insert the item into the unused list, keeping it sorted by the display text.
{
const auto item = _createFontSettingMenuItem(*it);
const auto it = std::lower_bound(unused.begin(), unused.end(), item, [](const MenuFlyoutItemBase& lhs, const MenuFlyoutItemBase& rhs) {
const auto& a = lhs.as<MenuFlyoutItem>().Text();
const auto& b = rhs.as<MenuFlyoutItem>().Text();
return til::compare_linguistic_insensitive(a, b) < 0;
});
unused.insert(it, item);
}

_addMenuFlyoutItemToUnused(fontSettingsIndex, _createFontSettingMenuItem(*it));
used.RemoveAt(gsl::narrow<uint32_t>(it - used.begin()));

_notifyChangesForFontSettings();
}

void AppearanceViewModel::UpdateFontSetting(const FontKeyValuePair* kvImpl)
{
const auto tag = kvImpl->Key();
const auto value = kvImpl->Value();
const auto tagString = tagToString(tag);
const auto fontInfo = _appearance.SourceProfile().FontInfo();
auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes();

if (!fontSettingsUser)
{
fontSettingsUser = winrt::single_threaded_map<hstring, float>();
if (kvImpl->IsFontFeature())
{
fontInfo.FontFeatures(fontSettingsUser);
}
else
{
fontInfo.FontAxes(fontSettingsUser);
}
}

std::ignore = fontSettingsUser.Insert(std::wstring_view{ tagString }, value);
_notifyChangesForFontSettingsReactive(fontSettingsIndex);
}

void AppearanceViewModel::_deleteAllFontSettings(FontSettingIndex fontSettingsIndex)
void AppearanceViewModel::_deleteAllFontKeyValuePairs(FontSettingIndex fontSettingsIndex)
{
const auto fontInfo = _appearance.SourceProfile().FontInfo();
if (fontSettingsIndex == FontFeaturesIndex)
Expand All @@ -778,16 +786,61 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

auto& d = *_fontFaceDependents;
auto& used = d.fontSettingsUsed[fontSettingsIndex];
auto& unused = d.fontSettingsUnused[fontSettingsIndex];

for (const auto& kv : used)
{
unused.emplace_back(_createFontSettingMenuItem(kv));
_addMenuFlyoutItemToUnused(fontSettingsIndex, _createFontSettingMenuItem(kv));
}

used.Clear();

_notifyChangesForFontSettings();
_notifyChangesForFontSettingsReactive(fontSettingsIndex);
}

// Inserts the given menu item into the unused list, while keeping it sorted by the display text.
void AppearanceViewModel::_addMenuFlyoutItemToUnused(FontSettingIndex index, MenuFlyoutItemBase item)
{
if (!_fontFaceDependents)
{
return;
}

auto& d = *_fontFaceDependents;
auto& unused = d.fontSettingsUnused[index];

const auto it = std::lower_bound(unused.begin(), unused.end(), item, [](const MenuFlyoutItemBase& lhs, const MenuFlyoutItemBase& rhs) {
const auto& a = lhs.as<MenuFlyoutItem>().Text();
const auto& b = rhs.as<MenuFlyoutItem>().Text();
return til::compare_linguistic_insensitive(a, b) < 0;
});
unused.insert(it, std::move(item));
}

void AppearanceViewModel::UpdateFontSetting(const FontKeyValuePair* kvImpl)
{
const auto tag = kvImpl->Key();
const auto value = kvImpl->Value();
const auto tagString = tagToString(tag);
const auto fontInfo = _appearance.SourceProfile().FontInfo();
auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes();

if (!fontSettingsUser)
{
fontSettingsUser = winrt::single_threaded_map<hstring, float>();
if (kvImpl->IsFontFeature())
{
fontInfo.FontFeatures(fontSettingsUser);
}
else
{
fontInfo.FontAxes(fontSettingsUser);
}
}

std::ignore = fontSettingsUser.Insert(std::wstring_view{ tagString }, value);
// Pwease call Profiles_Appearance::_onProfilePropertyChanged to make the pweview connyection wewoad. Thanks!! uwu
// ...I hate this.
_NotifyChanges(L"uwu");
}

void AppearanceViewModel::SetBackgroundImageOpacityFromPercentageValue(double percentageValue)
Expand Down
20 changes: 7 additions & 13 deletions src/cascadia/TerminalSettingsEditor/Appearances.h
View file
Expand Up @@ -94,18 +94,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void SetFontWeightFromDouble(double fontWeight);

const FontFaceDependentsData& FontFaceDependents()
{
if (!_fontFaceDependents)
{
_refreshFontFaceDependents();
}
return *_fontFaceDependents;
}

winrt::hstring MissingFontFaces() { return FontFaceDependents().missingFontFaces; }
winrt::hstring ProportionalFontFaces() { return FontFaceDependents().proportionalFontFaces; }
bool HasPowerlineCharacters() { return FontFaceDependents().hasPowerlineCharacters; }
const FontFaceDependentsData& FontFaceDependents();
winrt::hstring MissingFontFaces();
winrt::hstring ProportionalFontFaces();
bool HasPowerlineCharacters();

Windows::Foundation::Collections::IObservableVector<Editor::FontKeyValuePair> FontAxes();
bool HasFontAxes() const;
Expand Down Expand Up @@ -165,7 +157,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void _generateFontFeatures(IDWriteFontFace* fontFace, std::vector<Editor::FontKeyValuePair>& list);
Windows::UI::Xaml::Controls::MenuFlyoutItemBase _createFontSettingMenuItem(const Editor::FontKeyValuePair& kv);
void _notifyChangesForFontSettings();
void _deleteAllFontSettings(FontSettingIndex index);
void _notifyChangesForFontSettingsReactive(FontSettingIndex fontSettingsIndex);
void _deleteAllFontKeyValuePairs(FontSettingIndex index);
void _addMenuFlyoutItemToUnused(FontSettingIndex index, Windows::UI::Xaml::Controls::MenuFlyoutItemBase item);

Model::AppearanceConfig _appearance;
winrt::hstring _lastBgImagePath;
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Appearances.xaml
View file
Expand Up @@ -309,7 +309,7 @@
<StackPanel Spacing="16">
<ListView IsItemClickEnabled="False"
ItemTemplate="{StaticResource FontKeyValuePairTemplate}"
ItemsSource="{x:Bind Appearance.FontAxes}"
ItemsSource="{x:Bind Appearance.FontAxes, Mode=OneWay}"
SelectionMode="None" />
<muxc:DropDownButton x:Name="AddFontAxisButton"
x:Uid="Profile_AddFontAxisButton">
Expand All @@ -336,7 +336,7 @@
<StackPanel Spacing="16">
<ListView IsItemClickEnabled="False"
ItemTemplate="{StaticResource FontKeyValuePairTemplate}"
ItemsSource="{x:Bind Appearance.FontFeatures}"
ItemsSource="{x:Bind Appearance.FontFeatures, Mode=OneWay}"
SelectionMode="None" />
<muxc:DropDownButton x:Name="AddFontFeatureButton"
x:Uid="Profile_AddFontFeatureButton">
Expand Down
24 changes: 16 additions & 8 deletions src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp
View file
Expand Up @@ -3,13 +3,11 @@

#include "pch.h"
#include "Profiles_Appearance.h"
#include "Profiles_Appearance.g.cpp"

#include "ProfileViewModel.h"
#include "PreviewConnection.h"
#include "EnumEntry.h"

#include <LibraryResources.h>
#include "..\WinRTUtils\inc\Utils.h"
#include "Profiles_Appearance.g.cpp"

using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Navigation;
Expand Down Expand Up @@ -64,10 +62,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_Profile.DeleteUnfocusedAppearance();
}

void Profiles_Appearance::_onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const
void Profiles_Appearance::_onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&)
{
const auto settings = _Profile.TermSettings();
_previewConnection->DisplayPowerlineGlyphs(_Profile.DefaultAppearance().HasPowerlineCharacters());
_previewControl.UpdateControlSettings(settings, settings);
if (!_updatePreviewControl)
{
_updatePreviewControl = std::make_shared<ThrottledFuncTrailing<>>(
winrt::Windows::System::DispatcherQueue::GetForCurrentThread(),
std::chrono::milliseconds{ 100 },
[this]() {
const auto settings = _Profile.TermSettings();
_previewConnection->DisplayPowerlineGlyphs(_Profile.DefaultAppearance().HasPowerlineCharacters());
_previewControl.UpdateControlSettings(settings, settings);
});
}

_updatePreviewControl->Run();
}
}
10 changes: 7 additions & 3 deletions src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h
View file
Expand Up @@ -3,9 +3,12 @@

#pragma once

#include "Profiles_Appearance.g.h"
#include "Utils.h"
#include <ThrottledFunc.h>

#include "PreviewConnection.h"
#include "Utils.h"

#include "Profiles_Appearance.g.h"

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
Expand All @@ -26,10 +29,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Editor::ProfileViewModel, Profile, nullptr);

private:
void _onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const;
void _onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&);

winrt::com_ptr<PreviewConnection> _previewConnection{ nullptr };
Microsoft::Terminal::Control::TermControl _previewControl{ nullptr };
std::shared_ptr<ThrottledFuncTrailing<>> _updatePreviewControl;
Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker;
Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _AppearanceViewModelChangedRevoker;
Editor::IHostedInWindow _windowRoot;
Expand Down

0 comments on commit ae91734

Please sign in to comment.