Skip to content

Commit

Permalink
sanely pass around a cache instead of... whatever that was.
Browse files Browse the repository at this point in the history
  • Loading branch information
zadjii-msft committed Mar 4, 2024
1 parent 17075d6 commit 0a11643
Show file tree
Hide file tree
Showing 14 changed files with 196 additions and 76 deletions.
31 changes: 12 additions & 19 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1285,28 +1285,21 @@ void Pane::_FocusFirstChild()
}
}

void Pane::UpdateSettings(const CascadiaSettings& settings)
void Pane::UpdateSettings(const CascadiaSettings& settings, const winrt::TerminalApp::TerminalSettingsCache& cache)
{
if (_content)
{
_content.UpdateSettings(settings);
}
}

// Method Description:
// - Updates the settings of this pane, presuming that it is a leaf.
// Arguments:
// - settings: The new TerminalSettings to apply to any matching controls
// - profile: The profile from which these settings originated.
// Return Value:
// - <none>
void Pane::UpdateTerminalSettings(const TerminalSettingsCreateResult& settings, const Profile& profile)
{
assert(_IsLeaf());

if (const auto& terminalPane{ _getTerminalContent() })
{
return terminalPane.UpdateTerminalSettings(settings, profile);
// We need to do a bit more work here for terminal
// panes. They need to know about the profile that was used for
// them, and about the focused/unfocused settings.
if (const auto& terminalPaneContent{ _content.try_as<TerminalPaneContent>() })
{
terminalPaneContent.UpdateTerminalSettings(cache);
}
else
{
_content.UpdateSettings(settings);
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ class Pane : public std::enable_shared_from_this<Pane>
BuildStartupState BuildStartupActions(uint32_t currentId, uint32_t nextId, const bool asContent = false, const bool asMovePane = false);
winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetTerminalArgsForPane(const bool asContent = false) const;

void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
void UpdateTerminalSettings(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings,
const winrt::Microsoft::Terminal::Settings::Model::Profile& profile);
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const winrt::TerminalApp::TerminalSettingsCache& cache);
bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane,
const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
Expand Down
7 changes: 7 additions & 0 deletions src/cascadia/TerminalApp/TerminalAppLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@
<DependentUpon>TerminalPaneContent.idl</DependentUpon>
</ClInclude>
<ClInclude Include="Toast.h" />
<ClInclude Include="TerminalSettingsCache.h">
<DependentUpon>TerminalSettingsCache.idl</DependentUpon>
</ClInclude>
<ClInclude Include="SuggestionsControl.h">
<DependentUpon>SuggestionsControl.xaml</DependentUpon>
</ClInclude>
Expand Down Expand Up @@ -288,6 +291,9 @@
</ClCompile>
<ClCompile Include="$(GeneratedFilesDir)module.g.cpp" />
<ClCompile Include="Toast.cpp" />
<ClCompile Include="TerminalSettingsCache.cpp">
<DependentUpon>TerminalSettingsCache.idl</DependentUpon>
</ClCompile>
<ClCompile Include="SuggestionsControl.cpp">
<DependentUpon>SuggestionsControl.xaml</DependentUpon>
</ClCompile>
Expand Down Expand Up @@ -361,6 +367,7 @@
<Midl Include="EmptyStringVisibilityConverter.idl" />
<Midl Include="IPaneContent.idl" />
<Midl Include="TerminalPaneContent.idl" />
<Midl Include="TerminalSettingsCache.idl" />
</ItemGroup>
<!-- ========================= Misc Files ======================== -->
<ItemGroup>
Expand Down
86 changes: 44 additions & 42 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3306,56 +3306,58 @@ namespace winrt::TerminalApp::implementation

// Refresh UI elements

// Mapping by GUID isn't _excellent_ because the defaults profile doesn't have a stable GUID; however,
// when we stabilize its guid this will become fully safe.
std::unordered_map<winrt::guid, std::pair<Profile, TerminalSettingsCreateResult>> profileGuidSettingsMap;
const auto profileDefaults{ _settings.ProfileDefaults() };
const auto allProfiles{ _settings.AllProfiles() };
_terminalSettingsCache = TerminalApp::TerminalSettingsCache{ _settings, *_bindings };

profileGuidSettingsMap.reserve(allProfiles.Size() + 1);
// // Mapping by GUID isn't _excellent_ because the defaults profile doesn't have a stable GUID; however,
// // when we stabilize its guid this will become fully safe.
// std::unordered_map<winrt::guid, std::pair<Profile, TerminalSettingsCreateResult>> profileGuidSettingsMap;
// const auto profileDefaults{ _settings.ProfileDefaults() };
// const auto allProfiles{ _settings.AllProfiles() };

// Include the Defaults profile for consideration
profileGuidSettingsMap.insert_or_assign(profileDefaults.Guid(), std::pair{ profileDefaults, nullptr });
for (const auto& newProfile : allProfiles)
{
// Avoid creating a TerminalSettings right now. They're not totally cheap, and we suspect that users with many
// panes may not be using all of their profiles at the same time. Lazy evaluation is king!
profileGuidSettingsMap.insert_or_assign(newProfile.Guid(), std::pair{ newProfile, nullptr });
}
// profileGuidSettingsMap.reserve(allProfiles.Size() + 1);

// // Include the Defaults profile for consideration
// profileGuidSettingsMap.insert_or_assign(profileDefaults.Guid(), std::pair{ profileDefaults, nullptr });
// for (const auto& newProfile : allProfiles)
// {
// // Avoid creating a TerminalSettings right now. They're not totally cheap, and we suspect that users with many
// // panes may not be using all of their profiles at the same time. Lazy evaluation is king!
// profileGuidSettingsMap.insert_or_assign(newProfile.Guid(), std::pair{ newProfile, nullptr });
// }

for (const auto& tab : _tabs)
{
if (auto terminalTab{ _GetTerminalTabImpl(tab) })
{
// Let the tab know that there are new settings. It's up to each content to decide what to do with them.
terminalTab->UpdateSettings(_settings);

// FURTHERMORE We need to do a bit more work here for terminal
// panes. They need to know about the profile that was used for
// them, and about the focused/unfocused settings.

// Manually enumerate the panes in each tab; this will let us recycle TerminalSettings
// objects but only have to iterate one time.
terminalTab->GetRootPane()->WalkTree([&](auto&& pane) {
// If the pane isn't a terminal pane, it won't have a profile.
if (const auto profile{ pane->GetProfile() })
{
const auto found{ profileGuidSettingsMap.find(profile.Guid()) };
// GH#2455: If there are any panes with controls that had been
// initialized with a Profile that no longer exists in our list of
// profiles, we'll leave it unmodified. The profile doesn't exist
// anymore, so we can't possibly update its settings.
if (found != profileGuidSettingsMap.cend())
{
auto& pair{ found->second };
if (!pair.second)
{
pair.second = TerminalSettings::CreateWithProfile(_settings, pair.first, *_bindings);
}
pane->UpdateTerminalSettings(pair.second, pair.first);
}
}
});
terminalTab->UpdateSettings(_settings, _terminalSettingsCache);

// // FURTHERMORE We need to do a bit more work here for terminal
// // panes. They need to know about the profile that was used for
// // them, and about the focused/unfocused settings.

// // Manually enumerate the panes in each tab; this will let us recycle TerminalSettings
// // objects but only have to iterate one time.
// terminalTab->GetRootPane()->WalkTree([&](auto&& pane) {
// // If the pane isn't a terminal pane, it won't have a profile.
// if (const auto profile{ pane->GetProfile() })
// {
// const auto found{ profileGuidSettingsMap.find(profile.Guid()) };
// // GH#2455: If there are any panes with controls that had been
// // initialized with a Profile that no longer exists in our list of
// // profiles, we'll leave it unmodified. The profile doesn't exist
// // anymore, so we can't possibly update its settings.
// if (found != profileGuidSettingsMap.cend())
// {
// auto& pair{ found->second };
// if (!pair.second)
// {
// pair.second = TerminalSettings::CreateWithProfile(_settings, pair.first, *_bindings);
// }
// pane->UpdateTerminalSettings(pair.second, pair.first);
// }
// }
// });

// Update the icon of the tab for the currently focused profile in that tab.
// Only do this for TerminalTabs. Other types of tabs won't have multiple panes
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ namespace winrt::TerminalApp::implementation

TerminalApp::ContentManager _manager{ nullptr };

TerminalApp::TerminalSettingsCache _terminalSettingsCache{ nullptr };

struct StashedDragData
{
winrt::com_ptr<winrt::TerminalApp::implementation::TabBase> draggedTab{ nullptr };
Expand Down
10 changes: 6 additions & 4 deletions src/cascadia/TerminalApp/TerminalPaneContent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,15 @@ namespace winrt::TerminalApp::implementation
// Do nothing. We'll later be updated manually by
// UpdateTerminalSettings, which we need for profile and
// focused/unfocused settings.
assert(false); // If you hit this, you done goofed.
}

void TerminalPaneContent::UpdateTerminalSettings(const TerminalSettingsCreateResult& settings,
const Profile& profile)
void TerminalPaneContent::UpdateTerminalSettings(const TerminalApp::TerminalSettingsCache& cache)
{
_profile = profile;
_control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings());
if (const auto& settings{ cache.TryLookup(_profile) })
{
_control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings());
}
}

// Method Description:
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalApp/TerminalPaneContent.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetNewTerminalArgs(const bool asContent) const;

void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
void UpdateTerminalSettings(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings,
const winrt::Microsoft::Terminal::Settings::Model::Profile& profile);
void UpdateTerminalSettings(const TerminalApp::TerminalSettingsCache& cache);

void MarkAsDefterm();

Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPaneContent.idl
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
// Licensed under the MIT license.

import "IPaneContent.idl";
import "TerminalSettingsCache.idl";

namespace TerminalApp
{
[default_interface] runtimeclass TerminalPaneContent : IPaneContent, ISnappable
{
Microsoft.Terminal.Control.TermControl GetTerminal();

void UpdateTerminalSettings(Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult settings,
Microsoft.Terminal.Settings.Model.Profile profile);
void UpdateTerminalSettings(TerminalSettingsCache cache);

void MarkAsDefterm();

Expand Down
57 changes: 57 additions & 0 deletions src/cascadia/TerminalApp/TerminalSettingsCache.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

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

namespace winrt
{
namespace MUX = Microsoft::UI::Xaml;
namespace WUX = Windows::UI::Xaml;
namespace MTSM = Microsoft::Terminal::Settings::Model;
}

namespace winrt::TerminalApp::implementation
{
TerminalSettingsCache::TerminalSettingsCache(const MTSM::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings) :
_settings{ settings },
_bindings{ bindings }
{
// Mapping by GUID isn't _excellent_ because the defaults profile doesn't have a stable GUID; however,
// when we stabilize its guid this will become fully safe.
const auto profileDefaults{ _settings.ProfileDefaults() };
const auto allProfiles{ _settings.AllProfiles() };

profileGuidSettingsMap.reserve(allProfiles.Size() + 1);

// Include the Defaults profile for consideration
profileGuidSettingsMap.insert_or_assign(profileDefaults.Guid(), std::pair{ profileDefaults, nullptr });
for (const auto& newProfile : allProfiles)
{
// Avoid creating a TerminalSettings right now. They're not totally cheap, and we suspect that users with many
// panes may not be using all of their profiles at the same time. Lazy evaluation is king!
profileGuidSettingsMap.insert_or_assign(newProfile.Guid(), std::pair{ newProfile, nullptr });
}
}

MTSM::TerminalSettingsCreateResult TerminalSettingsCache::TryLookup(const MTSM::Profile& profile)
{
const auto found{ profileGuidSettingsMap.find(profile.Guid()) };
// GH#2455: If there are any panes with controls that had been
// initialized with a Profile that no longer exists in our list of
// profiles, we'll leave it unmodified. The profile doesn't exist
// anymore, so we can't possibly update its settings.
if (found != profileGuidSettingsMap.cend())
{
auto& pair{ found->second };
if (!pair.second)
{
pair.second = MTSM::TerminalSettings::CreateWithProfile(_settings, pair.first, _bindings);
}
return pair.second;
}

return nullptr;
}
}
48 changes: 48 additions & 0 deletions src/cascadia/TerminalApp/TerminalSettingsCache.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Class Name:
- ContentManager.h
Abstract:
- This is a helper class for tracking all of the terminal "content" instances of
the Terminal. These are all the ControlInteractivity & ControlCore's of each
of our TermControls. These are each assigned a GUID on creation, and stored in
a map for later lookup.
- This is used to enable moving panes between windows. TermControl's are not
thread-agile, so they cannot be reused on other threads. However, the content
is. This helper, which exists as a singleton across all the threads in the
Terminal app, allows each thread to create content, assign it to a
TermControl, detach it from that control, and reattach to new controls on
other threads.
- When you want to create a new TermControl, call CreateCore to instantiate a
new content with a GUID for later reparenting.
- Detach can be used to temporarily remove a content from its hosted
TermControl. After detaching, you can still use LookupCore &
TermControl::AttachContent to re-attach to the content.
--*/
#pragma once

#include "TerminalSettingsCache.g.h"
#include <inc/cppwinrt_utils.h>

namespace winrt::TerminalApp::implementation
{
class TerminalSettingsCache : public TerminalSettingsCacheT<TerminalSettingsCache>
{
public:
TerminalSettingsCache(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings);
Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult TryLookup(const Microsoft::Terminal::Settings::Model::Profile& profile);

private:
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };
TerminalApp::AppKeyBindings _bindings{ nullptr };
std::unordered_map<winrt::guid, std::pair<Microsoft::Terminal::Settings::Model::Profile, Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult>> profileGuidSettingsMap;
};
}

namespace winrt::TerminalApp::factory_implementation
{
BASIC_FACTORY(TerminalSettingsCache);
}
12 changes: 12 additions & 0 deletions src/cascadia/TerminalApp/TerminalSettingsCache.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import "AppKeyBindings.idl";

// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
namespace TerminalApp
{
[default_interface] runtimeclass TerminalSettingsCache
{
TerminalSettingsCache(Microsoft.Terminal.Settings.Model.CascadiaSettings settings, AppKeyBindings bindings);
Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult TryLookup(Microsoft.Terminal.Settings.Model.Profile profile);
}
}
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ namespace winrt::TerminalApp::implementation
// of the settings that apply to all tabs.
// Return Value:
// - <none>
void TerminalTab::UpdateSettings(const CascadiaSettings& settings)
void TerminalTab::UpdateSettings(const CascadiaSettings& settings, const TerminalApp::TerminalSettingsCache& cache)
{
ASSERT_UI_THREAD();

Expand All @@ -275,7 +275,7 @@ namespace winrt::TerminalApp::implementation

// Update the settings on all our panes.
_rootPane->WalkTree([&](auto pane) {
pane->UpdateSettings(settings);
pane->UpdateSettings(settings, cache);
return false;
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace winrt::TerminalApp::implementation
bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
bool FocusPane(const uint32_t id);

void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::TerminalSettingsCache& cache);
void UpdateTitle();

void Shutdown() override;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// (The window has a min. size that ensures that there's always a scrollbar thumb.)
if (drawableRange < 0)
{
assert(false);
// assert(false);
return;
}

Expand Down

0 comments on commit 0a11643

Please sign in to comment.