-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for setting the window frame color #15441
Changes from 5 commits
f1ed087
7ac1bc9
bed8f94
e581f75
c90bff2
dfd73e0
425c0d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4434,6 +4434,19 @@ namespace winrt::TerminalApp::implementation | |
|
||
til::color bgColor = backgroundSolidBrush.Color(); | ||
|
||
const auto getTerminalBrush = [this]() -> Media::Brush { | ||
if (const auto& control{ _GetActiveControl() }) | ||
{ | ||
return control.BackgroundBrush(); | ||
} | ||
else if (auto settingsTab = _GetFocusedTab().try_as<TerminalApp::SettingsTab>()) | ||
{ | ||
return settingsTab.Content().try_as<Settings::Editor::MainPage>().BackgroundBrush(); | ||
} | ||
return nullptr; | ||
}; | ||
const auto terminalBrush = getTerminalBrush(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was about to comment asking why this lambda is needed, when you can also just do this: Brush terminalBrush{ nullptr };
if (const auto& control{ _GetActiveControl() })
{
terminalBrush = control.BackgroundBrush();
}
else if (auto settingsTab = _GetFocusedTab().try_as<TerminalApp::SettingsTab>())
{
terminalBrush = settingsTab.Content().try_as<Settings::Editor::MainPage>().BackgroundBrush();
} but then I realized the code only moved. Still, you could consider making this change since it got changed anyways. |
||
|
||
if (_settings.GlobalSettings().UseAcrylicInTabRow()) | ||
{ | ||
const auto acrylicBrush = Media::AcrylicBrush(); | ||
|
@@ -4448,18 +4461,6 @@ namespace winrt::TerminalApp::implementation | |
theme.TabRow().UnfocusedBackground()) : | ||
ThemeColor{ nullptr } }) | ||
{ | ||
const auto terminalBrush = [this]() -> Media::Brush { | ||
if (const auto& control{ _GetActiveControl() }) | ||
{ | ||
return control.BackgroundBrush(); | ||
} | ||
else if (auto settingsTab = _GetFocusedTab().try_as<TerminalApp::SettingsTab>()) | ||
{ | ||
return settingsTab.Content().try_as<Settings::Editor::MainPage>().BackgroundBrush(); | ||
} | ||
return nullptr; | ||
}(); | ||
|
||
const auto themeBrush{ tabRowBg.Evaluate(res, terminalBrush, true) }; | ||
bgColor = ThemeColor::ColorFromBrush(themeBrush); | ||
TitlebarBrush(themeBrush); | ||
|
@@ -4489,11 +4490,27 @@ namespace winrt::TerminalApp::implementation | |
tabImpl->ThemeColor(tabBackground, tabUnfocusedBackground, bgColor); | ||
} | ||
} | ||
|
||
// Update the new tab button to have better contrast with the new color. | ||
// In theory, it would be convenient to also change these for the | ||
// inactive tabs as well, but we're leaving that as a follow up. | ||
_SetNewTabButtonColor(bgColor, bgColor); | ||
|
||
// Third: the window frame. This is basically the same logic as the tab row background. | ||
// We'll set our `FrameBrush` property, for the window to later use. | ||
const auto windowTheme{ theme.Window() }; | ||
if (auto windowFrame{ windowTheme ? (_activated ? windowTheme.Frame() : | ||
windowTheme.UnfocusedFrame()) : | ||
ThemeColor{ nullptr } }) | ||
{ | ||
const auto themeBrush{ windowFrame.Evaluate(res, terminalBrush, true) }; | ||
FrameBrush(themeBrush); | ||
} | ||
else | ||
{ | ||
// Nothing was set in the theme - fall back to null. The window will | ||
// use that as an indication to use the default window frame. | ||
FrameBrush(nullptr); | ||
} | ||
} | ||
|
||
// Function Description: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,7 +127,8 @@ namespace winrt::TerminalApp::implementation | |
void WindowVisibilityChanged(const bool showOrHide); | ||
|
||
winrt::TerminalApp::TaskbarState TaskbarState(); | ||
winrt::Windows::UI::Xaml::Media::Brush TitlebarBrush(); | ||
winrt::Windows::UI::Xaml::Media::Brush TitlebarBrush() { return _root ? _root->TitlebarBrush() : nullptr; } | ||
winrt::Windows::UI::Xaml::Media::Brush FrameBrush() { return _root ? _root->FrameBrush() : nullptr; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be moved into the .cpp file. |
||
void WindowActivated(const bool activated); | ||
|
||
bool GetMinimizeToNotificationArea(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,8 @@ using namespace std::chrono_literals; | |
// "If the high-order bit is 1, the key is down; otherwise, it is up." | ||
static constexpr short KeyPressed{ gsl::narrow_cast<short>(0x8000) }; | ||
|
||
constexpr const auto FrameUpdateInterval = std::chrono::milliseconds(16); | ||
|
||
AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, | ||
winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, | ||
const Remoting::WindowManager& manager, | ||
|
@@ -38,6 +40,8 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, | |
_peasant{ peasant }, | ||
_desktopManager{ winrt::try_create_instance<IVirtualDesktopManager>(__uuidof(VirtualDesktopManager)) } | ||
{ | ||
_started = std::chrono::high_resolution_clock::now(); | ||
|
||
_HandleCommandlineArgs(args); | ||
|
||
_HandleSessionRestore(!args.Content().empty()); | ||
|
@@ -419,6 +423,10 @@ void AppHost::Close() | |
// After calling _window->Close() we should avoid creating more WinUI related actions. | ||
// I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. | ||
_revokers = {}; | ||
if (_frameTimer) | ||
{ | ||
_frameTimer.Tick(_frameTimerToken); | ||
} | ||
_showHideWindowThrottler.reset(); | ||
_window->Close(); | ||
|
||
|
@@ -1036,24 +1044,144 @@ static bool _isActuallyDarkTheme(const auto requestedTheme) | |
} | ||
} | ||
|
||
// DwmSetWindowAttribute(... DWMWA_BORDER_COLOR.. ) doesn't work on Windows 10, | ||
// but it _will_ spew to the debug console. This helper just no-ops the call on | ||
// Windows 10, so that we don't even get that spew | ||
void _frameColorHelper(const HWND h, const COLORREF color) | ||
{ | ||
static const bool isWindows11 = []() { | ||
OSVERSIONINFOEXW osver{}; | ||
osver.dwOSVersionInfoSize = sizeof(osver); | ||
osver.dwBuildNumber = 22000; | ||
|
||
DWORDLONG dwlConditionMask = 0; | ||
VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); | ||
|
||
if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) | ||
{ | ||
return true; | ||
} | ||
return false; | ||
}(); | ||
|
||
if (isWindows11) | ||
{ | ||
LOG_IF_FAILED(DwmSetWindowAttribute(h, DWMWA_BORDER_COLOR, &color, sizeof(color))); | ||
} | ||
} | ||
|
||
void AppHost::_updateTheme() | ||
{ | ||
auto theme = _appLogic.Theme(); | ||
|
||
_window->OnApplicationThemeChanged(theme.RequestedTheme()); | ||
|
||
const auto windowTheme{ theme.Window() }; | ||
|
||
const auto b = _windowLogic.TitlebarBrush(); | ||
const auto color = ThemeColor::ColorFromBrush(b); | ||
const auto colorOpacity = b ? color.A / 255.0 : 0.0; | ||
const auto brushOpacity = _opacityFromBrush(b); | ||
const auto opacity = std::min(colorOpacity, brushOpacity); | ||
_window->UseMica(theme.Window() ? theme.Window().UseMica() : false, opacity); | ||
_window->UseMica(windowTheme ? windowTheme.UseMica() : false, opacity); | ||
|
||
// This is a hack to make the window borders dark instead of light. | ||
// It must be done before WM_NCPAINT so that the borders are rendered with | ||
// the correct theme. | ||
// For more information, see GH#6620. | ||
LOG_IF_FAILED(TerminalTrySetDarkTheme(_window->GetHandle(), _isActuallyDarkTheme(theme.RequestedTheme()))); | ||
|
||
// Update the window frame. If `rainbowFrame:true` is enabled, then that | ||
// will be used. Otherwise we'll try to use the `FrameBrush` set in the | ||
// terminal window, as that will have the right color for the ThemeColor for | ||
// this setting. If that value is null, then revert to the default frame | ||
// color. | ||
if (windowTheme) | ||
{ | ||
if (windowTheme.RainbowFrame()) | ||
{ | ||
_startFrameTimer(); | ||
} | ||
else if (const auto b{ _windowLogic.FrameBrush() }) | ||
{ | ||
_stopFrameTimer(); | ||
const auto color = ThemeColor::ColorFromBrush(b); | ||
COLORREF ref{ til::color{ color } }; | ||
_frameColorHelper(_window->GetHandle(), ref); | ||
} | ||
else | ||
{ | ||
_stopFrameTimer(); | ||
// DWMWA_COLOR_DEFAULT is the magic "reset to the default" value | ||
_frameColorHelper(_window->GetHandle(), DWMWA_COLOR_DEFAULT); | ||
} | ||
} | ||
} | ||
|
||
void AppHost::_startFrameTimer() | ||
{ | ||
// Instantiate the frame color timer, if we haven't already. We'll only ever | ||
// create one instance of this. We'll set up the callback for the timers as | ||
// _updateFrameColor, which will actually handle setting the colors. If we | ||
// already have a timer, just start that one. | ||
|
||
if (_frameTimer == nullptr) | ||
{ | ||
_frameTimer = winrt::Windows::UI::Xaml::DispatcherTimer(); | ||
_frameTimer.Interval(FrameUpdateInterval); | ||
_frameTimerToken = _frameTimer.Tick({ this, &AppHost::_updateFrameColor }); | ||
} | ||
_frameTimer.Start(); | ||
} | ||
|
||
void AppHost::_stopFrameTimer() | ||
{ | ||
if (_frameTimer) | ||
{ | ||
_frameTimer.Stop(); | ||
} | ||
} | ||
|
||
// Method Description: | ||
// - Updates the color of the window frame to cycle through all the colors. This | ||
// is called as the `_frameTimer` Tick callback, roughly 60 times per second. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW This is one of the things we should probably put into the renderer and not the UI parts. Over RDP it'll run too fast (<= 30Hz) and on modern PCs too slow (>= 120Hz). It's similar to our scrollbar updates, etc. |
||
void AppHost::_updateFrameColor(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::Foundation::IInspectable&) | ||
{ | ||
// First, a couple helper functions: | ||
static const auto fmod_1 = [](const float x) -> float { | ||
float integer = floor(x); | ||
return x - integer; | ||
}; | ||
|
||
static const auto saturateAndToColor = [](const float a, const float b, const float c) -> til::color { | ||
return til::color{ | ||
base::saturated_cast<uint8_t>(255.f * std::clamp(a, 0.f, 1.f)), | ||
base::saturated_cast<uint8_t>(255.f * std::clamp(b, 0.f, 1.f)), | ||
base::saturated_cast<uint8_t>(255.f * std::clamp(c, 0.f, 1.f)) | ||
}; | ||
}; | ||
|
||
// Helper for converting a hue [0, 1) to an RGB value. | ||
// Credit to https://www.chilliant.com/rgb2hsv.html | ||
static const auto hueToRGB = [&](const float H) -> til::color { | ||
float R = abs(H * 6 - 3) - 1; | ||
float G = 2 - abs(H * 6 - 2); | ||
float B = 2 - abs(H * 6 - 4); | ||
return saturateAndToColor(R, G, B); | ||
}; | ||
|
||
// Now, the main body of work. | ||
// - Convert the time delta between when we were started and now, to a hue. This will cycle us through all the colors. | ||
// - Convert that hue to an RGB value. | ||
// - Set the frame's color to that RGB color. | ||
const auto now = std::chrono::high_resolution_clock::now(); | ||
const std::chrono::duration<float> delta{ now - _started }; | ||
const auto millis = delta.count() / 4; // divide by four, to make the effect slower. Otherwise it flashes way to fast. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should say seconds. It might be better to do the division below where the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I put the division after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh what I meant is something like: float unused;
const seconds = std::chrono::duration<float>(now - _started).count();
const auto hue = std::modf(seconds / 4.0f, &unused);
const auto color = hueToRGB(hue); BTW technically, when you use the generic overloads of the math functions you always have to prefix them with |
||
|
||
const auto color = hueToRGB(fmod_1(millis)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can also use |
||
// Don't log this one. If it failed, chances are so will the next one, and | ||
// we really don't want to just log 60x/s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does log the error though! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, wrote that comment before making |
||
_frameColorHelper(_window->GetHandle(), color); | ||
} | ||
|
||
void AppHost::_HandleSettingsChanged(const winrt::Windows::Foundation::IInspectable& /*sender*/, | ||
|
@@ -1203,6 +1331,10 @@ void AppHost::_PropertyChangedHandler(const winrt::Windows::Foundation::IInspect | |
_updateTheme(); | ||
} | ||
} | ||
else if (e.PropertyName() == L"FrameBrush") | ||
{ | ||
_updateTheme(); | ||
} | ||
} | ||
|
||
winrt::fire_and_forget AppHost::_WindowInitializedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mark it as
experimental.
so we declare it as "no strings attached"-kinda support. Should we really put this into 1.18?