Skip to content

Commit

Permalink
Flash the pane dark when BEL is emitted in a light terminal (#13707)
Browse files Browse the repository at this point in the history
Adds a variable `_isBackgroundLight` that is updated when the background
color is changed. When it is `true`, the BEL indicator flash will darken
the screen instead of brightening.

`_isColorLight(bg)` returns `true` if the average of `r`, `g`, and `b`
is >127

I was unsure of an appropriate way to change the color of the
`CompositionLight` based on the background, so I changed it to always be
gray and adjusted the intensity values of the original animation to have
roughly the same visual effect as the white.

## Validation Steps Performed
* Tested the two flashes on the default color schemes and some custom
  background colors to see if they look consistent
* Used tracepoints and visual to check that the right animation is used
  (including multiple tabs, split windows with different themes, and
  changing settings while window is open)

References #9270
Closes #13450
  • Loading branch information
Fyrebright authored Aug 11, 2022
1 parent 8721573 commit ea04823
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 5 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,7 @@ LTLTLTLTL
ltrim
ltype
LUID
luma
lval
LVB
LVERTICAL
Expand Down
37 changes: 33 additions & 4 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Firing it manually makes sure it does.
_BackgroundBrush = RootGrid().Background();
_PropertyChangedHandlers(*this, Windows::UI::Xaml::Data::PropertyChangedEventArgs{ L"BackgroundBrush" });

_isBackgroundLight = _isColorLight(bg);
}

bool TermControl::_isColorLight(til::color bg) noexcept
{
// Checks if the current background color is light enough
// to need a dark version of the visual bell indicator
// This is a poor man's Rec. 601 luma.
const auto l = 30 * bg.r + 59 * bg.g + 11 * bg.b;
return l > 12750;
}

// Method Description:
Expand Down Expand Up @@ -2699,15 +2710,25 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Initialize the animation if it does not exist
// We only initialize here instead of in the ctor because depending on the bell style setting,
// we may never need this animation
if (!_bellLightAnimation)
if (!_bellLightAnimation && !_isBackgroundLight)
{
_bellLightAnimation = Window::Current().Compositor().CreateScalarKeyFrameAnimation();
// Add key frames and a duration to our bell light animation
_bellLightAnimation.InsertKeyFrame(0.0, 2.0);
_bellLightAnimation.InsertKeyFrame(1.0, 1.0);
_bellLightAnimation.InsertKeyFrame(0.0, 4.0);
_bellLightAnimation.InsertKeyFrame(1.0, 1.9);
_bellLightAnimation.Duration(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(TerminalWarningBellInterval)));
}

// Likewise, initialize the dark version of the animation only if required
if (!_bellDarkAnimation && _isBackgroundLight)
{
_bellDarkAnimation = Window::Current().Compositor().CreateScalarKeyFrameAnimation();
// reversing the order of the intensity values produces a similar effect as the light version
_bellDarkAnimation.InsertKeyFrame(0.0, 1.0);
_bellDarkAnimation.InsertKeyFrame(1.0, 2.0);
_bellDarkAnimation.Duration(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(TerminalWarningBellInterval)));
}

// Similar to the animation, only initialize the timer here
if (!_bellLightTimer)
{
Expand All @@ -2726,7 +2747,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation

// Switch on the light and animate the intensity to fade out
VisualBellLight::SetIsTarget(RootGrid(), true);
BellLight().CompositionLight().StartAnimation(L"Intensity", _bellLightAnimation);

if (_isBackgroundLight)
{
BellLight().CompositionLight().StartAnimation(L"Intensity", _bellDarkAnimation);
}
else
{
BellLight().CompositionLight().StartAnimation(L"Intensity", _bellLightAnimation);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool _pointerPressedInBounds{ false };

winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation{ nullptr };
winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellDarkAnimation{ nullptr };
Windows::UI::Xaml::DispatcherTimer _bellLightTimer{ nullptr };

std::optional<Windows::UI::Xaml::DispatcherTimer> _cursorTimer;
Expand All @@ -209,6 +210,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;
bool _showMarksInScrollbar{ false };

bool _isBackgroundLight{ false };

inline bool _IsClosing() const noexcept
{
#ifndef NDEBUG
Expand All @@ -230,6 +233,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _InitializeBackgroundBrush();
winrt::fire_and_forget _coreBackgroundColorChanged(const IInspectable& sender, const IInspectable& args);
void _changeBackgroundColor(til::color bg);
static bool _isColorLight(til::color bg) noexcept;
void _changeBackgroundOpacity();

bool _InitializeTerminal();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/XamlLights.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
if (!CompositionLight())
{
auto spotLight{ Window::Current().Compositor().CreateAmbientLight() };
spotLight.Color(Windows::UI::Colors::White());
spotLight.Color(Windows::UI::Colors::Gray());
CompositionLight(spotLight);
}
}
Expand Down

0 comments on commit ea04823

Please sign in to comment.