Skip to content
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

Merged
merged 7 commits into from
Jun 6, 2023

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

Add support for $theme.window.frame, .unfocusedFrame, and .rainbowFrame. The first two accept a ThemeColor to set the window frame, using DwmSetWindowAttribute with DWMWA_BORDER_COLOR. rainbowFrame accepts a bool. When enabled, it'll cycle the color of the frame through all the hues, ala this gif (but, constantly, instead of just when the window moves).

This only works on Windows 11.

Validation Steps Performed

  • Works on Windows 11
  • Doesn't explode on Windows 10

PR Checklist

other details

There's probably some impact to perf with rainbowFrame. It's one DispatcherTimer per window. That could probably be optimized somehow to like, one per process, but meh?

some sample json for copypasta

{
    "name": "Accent Titlebar",
    "tab":
    {
        "background": "terminalBackground",
        "unfocusedBackground": "#00000000",
        "showCloseButton": "hover"
    },
    "tabRow":
    {
        "background": "accent"
    },
    "window":
    {
        "applicationTheme": "system",
        "frame": "accent",
        // "frame": "terminalBackground",
        "unfocusedFrame": "#ff0000",
        // "unfocusedFrame": null,
        "rainbowFrame": true
    }
},

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-Theming Anything related to the theming of elements of the window Product-Terminal The new Windows Terminal. labels May 25, 2023
@zadjii-msft zadjii-msft added this to To Cherry Pick in 1.18 Servicing Pipeline via automation May 25, 2023
@@ -1813,6 +1813,19 @@
"description": "True if the Terminal should use a Mica backdrop for the window. This will apply underneath all controls (including the terminal panes and the titlebar)",
"type": "boolean",
"default": false
},
"rainbowFrame": {
Copy link
Member

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?

}
return nullptr;
};
const auto terminalBrush = getTerminalBrush();
Copy link
Member

Choose a reason for hiding this comment

The 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.

Comment on lines 130 to 131
winrt::Windows::UI::Xaml::Media::Brush TitlebarBrush() { return _root ? _root->TitlebarBrush() : nullptr; }
winrt::Windows::UI::Xaml::Media::Brush FrameBrush() { return _root ? _root->FrameBrush() : nullptr; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be moved into the .cpp file.


// 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.
Copy link
Member

Choose a reason for hiding this comment

The 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.

Comment on lines 1182 to 1183
// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does log the error though!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, wrote that comment before making _frameColorHelper

// - 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.
Copy link
Member

Choose a reason for hiding this comment

The 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 fmod_1 is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I put the division after the modf, then it only loops through the first 25% of the hues

Copy link
Member

Choose a reason for hiding this comment

The 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 std:: (modf is for double; modff is for float). The C header does not have such generic overloads and the MSVC CRT just happens to define them.

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.

const auto color = hueToRGB(fmod_1(millis));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use modf here.

@carlos-zamora carlos-zamora removed this from To Cherry Pick in 1.18 Servicing Pipeline Jun 6, 2023
@carlos-zamora carlos-zamora merged commit 8f83322 into main Jun 6, 2023
15 checks passed
@carlos-zamora carlos-zamora deleted the dev/migrie/f/captain-america-number-1 branch June 6, 2023 23:17
@HotCakeX
Copy link

I love this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Theming Anything related to the theming of elements of the window Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rainbow Frame
4 participants