Skip to content

Commit

Permalink
Use float instead of double by default (#17100)
Browse files Browse the repository at this point in the history
While `double` is probably generally preferable for UI code,
our application is essentially a complex wrapper wrapper around
DWrite, D2D and D3D, all of which use `float` exclusively.

Of course it also uses XAML, but that one uses `float` for roughly
1/3rd of its API functions, so I'm not sure what it prefers.
Additionally, it's mostly a coincidence that we use WinUI/XAML for
Windows Terminal whereas DWrite/D2D/D3D are effectively essential.
This is demonstrated by the fact that we have a `HwndTerminal`,
while there's no alternative to e.g. D3D on Windows.

The goal of this PR is that DIP based calculations never end up
mixing `float` and `double`. This PR also changes opacity-related
values to `float` because I felt like that fits the theme.
  • Loading branch information
lhecker committed Apr 23, 2024
1 parent f49cf44 commit 99061ee
Show file tree
Hide file tree
Showing 45 changed files with 180 additions and 178 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/ActionPreviewHandlers.cpp
Expand Up @@ -127,7 +127,7 @@ namespace winrt::TerminalApp::implementation
auto originalOpacity{ control.BackgroundOpacity() };

// Apply the new opacity
control.AdjustOpacity(args.Opacity() / 100.0, args.Relative());
control.AdjustOpacity(args.Opacity() / 100.0f, args.Relative());

if (backup)
{
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Expand Up @@ -286,7 +286,7 @@ namespace winrt::TerminalApp::implementation
_SplitPane(terminalTab,
realArgs.SplitDirection(),
// This is safe, we're already filtering so the value is (0, 1)
::base::saturated_cast<float>(realArgs.SplitSize()),
realArgs.SplitSize(),
_MakePane(realArgs.ContentArgs(), duplicateFromTab));
args.Handled(true);
}
Expand Down Expand Up @@ -1247,7 +1247,7 @@ namespace winrt::TerminalApp::implementation
if (const auto& realArgs = args.ActionArgs().try_as<AdjustOpacityArgs>())
{
const auto res = _ApplyToActiveControls([&](auto& control) {
control.AdjustOpacity(realArgs.Opacity() / 100.0, realArgs.Relative());
control.AdjustOpacity(realArgs.Opacity() / 100.0f, realArgs.Relative());
});
args.Handled(res);
}
Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/TerminalApp/Pane.cpp
Expand Up @@ -151,7 +151,7 @@ Pane::BuildStartupState Pane::BuildStartupActions(uint32_t currentId, uint32_t n
// When creating a pane the split size is the size of the new pane
// and not position.
const auto splitDirection = _splitState == SplitState::Horizontal ? SplitDirection::Down : SplitDirection::Right;
const auto splitSize = (kind != BuildStartupKind::None && _IsLeaf() ? .5 : 1. - _desiredSplitPosition);
const auto splitSize = (kind != BuildStartupKind::None && _IsLeaf() ? 0.5f : 1.0f - _desiredSplitPosition);
SplitPaneArgs args{ SplitType::Manual, splitDirection, splitSize, terminalArgs };
actionAndArgs.Args(args);

Expand Down Expand Up @@ -1595,12 +1595,12 @@ void Pane::_CloseChildRoutine(const bool closeFirst)
const auto splitWidth = _splitState == SplitState::Vertical;

Size removedOriginalSize{
::base::saturated_cast<float>(removedChild->_root.ActualWidth()),
::base::saturated_cast<float>(removedChild->_root.ActualHeight())
static_cast<float>(removedChild->_root.ActualWidth()),
static_cast<float>(removedChild->_root.ActualHeight())
};
Size remainingOriginalSize{
::base::saturated_cast<float>(remainingChild->_root.ActualWidth()),
::base::saturated_cast<float>(remainingChild->_root.ActualHeight())
static_cast<float>(remainingChild->_root.ActualWidth()),
static_cast<float>(remainingChild->_root.ActualHeight())
};

// Remove both children from the grid
Expand Down Expand Up @@ -1902,7 +1902,7 @@ void Pane::_SetupEntranceAnimation()
// looks bad.
_secondChild->_root.Background(_themeResources.unfocusedBorderBrush);

const auto [firstSize, secondSize] = _CalcChildrenSizes(::base::saturated_cast<float>(totalSize));
const auto [firstSize, secondSize] = _CalcChildrenSizes(static_cast<float>(totalSize));

// This is safe to capture this, because it's only being called in the
// context of this method (not on another thread)
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Expand Up @@ -1947,8 +1947,8 @@ namespace winrt::TerminalApp::implementation
layout.LaunchMode({ mode });

// Only save the content size because the tab size will be added on load.
const auto contentWidth = ::base::saturated_cast<float>(_tabContent.ActualWidth());
const auto contentHeight = ::base::saturated_cast<float>(_tabContent.ActualHeight());
const auto contentWidth = static_cast<float>(_tabContent.ActualWidth());
const auto contentHeight = static_cast<float>(_tabContent.ActualHeight());
const winrt::Windows::Foundation::Size windowSize{ contentWidth, contentHeight };

layout.InitialSize(windowSize);
Expand Down Expand Up @@ -2358,8 +2358,8 @@ namespace winrt::TerminalApp::implementation
{
return;
}
const auto contentWidth = ::base::saturated_cast<float>(_tabContent.ActualWidth());
const auto contentHeight = ::base::saturated_cast<float>(_tabContent.ActualHeight());
const auto contentWidth = static_cast<float>(_tabContent.ActualWidth());
const auto contentHeight = static_cast<float>(_tabContent.ActualHeight());
const winrt::Windows::Foundation::Size availableSpace{ contentWidth, contentHeight };

const auto realSplitType = activeTab->PreCalculateCanSplit(splitDirection, splitSize, availableSpace);
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalApp/TitlebarControl.cpp
Expand Up @@ -44,12 +44,12 @@ namespace winrt::TerminalApp::implementation
});
}

double TitlebarControl::CaptionButtonWidth()
float TitlebarControl::CaptionButtonWidth()
{
// Divide by three, since we know there are only three buttons. When
// Windows 12 comes along and adds another, we can update this /s
static auto width{ MinMaxCloseControl().ActualWidth() / 3.0 };
return width;
const auto minMaxCloseWidth = MinMaxCloseControl().ActualWidth();
return static_cast<float>(minMaxCloseWidth) / 3.0f;
}

IInspectable TitlebarControl::Content()
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TitlebarControl.h
Expand Up @@ -15,7 +15,7 @@ namespace winrt::TerminalApp::implementation
void PressButton(CaptionButton button);
winrt::fire_and_forget ClickButton(CaptionButton button);
void ReleaseButtons();
double CaptionButtonWidth();
float CaptionButtonWidth();

IInspectable Content();
void Content(IInspectable content);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TitlebarControl.idl
Expand Up @@ -27,7 +27,7 @@ namespace TerminalApp
void PressButton(CaptionButton button);
void ClickButton(CaptionButton button);
void ReleaseButtons();
Double CaptionButtonWidth { get; };
Single CaptionButtonWidth { get; };

IInspectable Content;
Windows.UI.Xaml.Controls.Border DragBar { get; };
Expand Down
24 changes: 10 additions & 14 deletions src/cascadia/TerminalControl/ControlCore.cpp
Expand Up @@ -677,7 +677,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}

void ControlCore::AdjustOpacity(const double adjustment)
void ControlCore::AdjustOpacity(const float adjustment)
{
if (adjustment == 0)
{
Expand All @@ -694,11 +694,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - focused (default == true): Whether the window is focused or unfocused.
// Return Value:
// - <none>
void ControlCore::_setOpacity(const double opacity, bool focused)
void ControlCore::_setOpacity(const float opacity, bool focused)
{
const auto newOpacity = std::clamp(opacity,
0.0,
1.0);
const auto newOpacity = std::clamp(opacity, 0.0f, 1.0f);

if (newOpacity == Opacity())
{
Expand All @@ -713,7 +711,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_runtimeFocusedOpacity = focused ? newOpacity : _runtimeFocusedOpacity;

// Manually turn off acrylic if they turn off transparency.
_runtimeUseAcrylic = newOpacity < 1.0 && _settings->UseAcrylic();
_runtimeUseAcrylic = newOpacity < 1.0f && _settings->UseAcrylic();

// Update the renderer as well. It might need to fall back from
// cleartype -> grayscale if the BG is transparent / acrylic.
Expand Down Expand Up @@ -881,7 +879,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Method Description:
// - Updates the appearance of the current terminal.
// - INVARIANT: This method can only be called if the caller DOES NOT HAVE writing lock on the terminal.
void ControlCore::ApplyAppearance(const bool& focused)
void ControlCore::ApplyAppearance(const bool focused)
{
const auto lock = _terminal->LockForWriting();
const auto& newAppearance{ focused ? _settings->FocusedAppearance() : _settings->UnfocusedAppearance() };
Expand All @@ -900,10 +898,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Incase EnableUnfocusedAcrylic is disabled and Focused Acrylic is set to true,
// the terminal should ignore the unfocused opacity from settings.
// The Focused Opacity from settings should be ignored if overridden at runtime.
bool useFocusedRuntimeOpacity = focused || (!_settings->EnableUnfocusedAcrylic() && UseAcrylic());
double newOpacity = useFocusedRuntimeOpacity ?
FocusedOpacity() :
newAppearance->Opacity();
const auto useFocusedRuntimeOpacity = focused || (!_settings->EnableUnfocusedAcrylic() && UseAcrylic());
const auto newOpacity = useFocusedRuntimeOpacity ? FocusedOpacity() : newAppearance->Opacity();
_setOpacity(newOpacity, focused);

// No need to update Acrylic if UnfocusedAcrylic is disabled
Expand Down Expand Up @@ -1422,8 +1418,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
const auto fontSize = _actualFont.GetSize();
return {
::base::saturated_cast<float>(fontSize.width),
::base::saturated_cast<float>(fontSize.height)
static_cast<float>(fontSize.width),
static_cast<float>(fontSize.height)
};
}

Expand Down Expand Up @@ -2350,7 +2346,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return _settings->HasUnfocusedAppearance();
}

void ControlCore::AdjustOpacity(const double opacityAdjust, const bool relative)
void ControlCore::AdjustOpacity(const float opacityAdjust, const bool relative)
{
if (relative)
{
Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/TerminalControl/ControlCore.h
Expand Up @@ -91,7 +91,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void Detach();

void UpdateSettings(const Control::IControlSettings& settings, const IControlAppearance& newAppearance);
void ApplyAppearance(const bool& focused);
void ApplyAppearance(const bool focused);
Control::IControlSettings Settings();
Control::IControlAppearance FocusedAppearance() const;
Control::IControlAppearance UnfocusedAppearance() const;
Expand Down Expand Up @@ -136,7 +136,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void LostFocus();

void ToggleShaderEffects();
void AdjustOpacity(const double adjustment);
void AdjustOpacity(const float adjustment);
void ResumeRendering();

void SetHoveredCell(Core::Point terminalPosition);
Expand Down Expand Up @@ -243,7 +243,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
hstring ReadEntireBuffer() const;
Control::CommandHistoryContext CommandHistory() const;

void AdjustOpacity(const double opacity, const bool relative);
void AdjustOpacity(const float opacity, const bool relative);

void WindowVisibilityChanged(const bool showOrHide);

Expand All @@ -258,8 +258,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool ShouldShowSelectCommand();
bool ShouldShowSelectOutput();

RUNTIME_SETTING(double, Opacity, _settings->Opacity());
RUNTIME_SETTING(double, FocusedOpacity, FocusedAppearance().Opacity());
RUNTIME_SETTING(float, Opacity, _settings->Opacity());
RUNTIME_SETTING(float, FocusedOpacity, FocusedAppearance().Opacity());
RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic());

// -------------------------------- WinRT Events ---------------------------------
Expand Down Expand Up @@ -399,7 +399,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _updateAntiAliasingMode();
void _connectionOutputHandler(const hstring& hstr);
void _updateHoveredCell(const std::optional<til::point> terminalPosition);
void _setOpacity(const double opacity, const bool focused = true);
void _setOpacity(const float opacity, const bool focused = true);

bool _isBackgroundTransparent();
void _focusChanged(bool focused);
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalControl/ControlCore.idl
Expand Up @@ -88,7 +88,7 @@ namespace Microsoft.Terminal.Control

Windows.Foundation.Size FontSize { get; };
UInt16 FontWeight { get; };
Double Opacity { get; };
Single Opacity { get; };
Boolean UseAcrylic { get; };

Boolean TryMarkModeKeybinding(Int16 vkey,
Expand Down Expand Up @@ -149,7 +149,7 @@ namespace Microsoft.Terminal.Control
String ReadEntireBuffer();
CommandHistoryContext CommandHistory();

void AdjustOpacity(Double Opacity, Boolean relative);
void AdjustOpacity(Single Opacity, Boolean relative);
void WindowVisibilityChanged(Boolean showOrHide);

void ColorSelection(SelectionColor fg, SelectionColor bg, Microsoft.Terminal.Core.MatchMode matchMode);
Expand Down
16 changes: 8 additions & 8 deletions src/cascadia/TerminalControl/ControlInteractivity.cpp
Expand Up @@ -518,7 +518,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void ControlInteractivity::_mouseTransparencyHandler(const int32_t mouseDelta) const
{
// Transparency is on a scale of [0.0,1.0], so only increment by .01.
const auto effectiveDelta = mouseDelta < 0 ? -.01 : .01;
const auto effectiveDelta = mouseDelta < 0 ? -.01f : .01f;
_core->AdjustOpacity(effectiveDelta);
}

Expand Down Expand Up @@ -554,7 +554,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// underneath us. We wouldn't know - we don't want the overhead of
// another ScrollPositionChanged handler. If the scrollbar should be
// somewhere other than where it is currently, then start from that row.
const auto currentInternalRow = ::base::saturated_cast<int>(::std::round(_internalScrollbarPosition));
const auto currentInternalRow = std::lround(_internalScrollbarPosition);
const auto currentCoreRow = _core->ScrollOffset();
const auto currentOffset = currentInternalRow == currentCoreRow ?
_internalScrollbarPosition :
Expand All @@ -564,13 +564,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// However, for us, the signs are flipped.
// With one of the precision mice, one click is always a multiple of 120 (WHEEL_DELTA),
// but the "smooth scrolling" mode results in non-int values
const auto rowDelta = mouseDelta / (-1.0 * WHEEL_DELTA);
const auto rowDelta = mouseDelta / (-1.0f * WHEEL_DELTA);

// WHEEL_PAGESCROLL is a Win32 constant that represents the "scroll one page
// at a time" setting. If we ignore it, we will scroll a truly absurd number
// of rows.
const auto rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? ::base::saturated_cast<double>(_core->ViewHeight()) : _rowsToScroll };
auto newValue = (rowsToScroll * rowDelta) + (currentOffset);
const auto rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? _core->ViewHeight() : _rowsToScroll };
const auto newValue = rowsToScroll * rowDelta + currentOffset;

// Update the Core's viewport position, and raise a
// ScrollPositionChanged event to update the scrollbar
Expand Down Expand Up @@ -600,17 +600,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - newValue: The new top of the viewport
// Return Value:
// - <none>
void ControlInteractivity::UpdateScrollbar(const double newValue)
void ControlInteractivity::UpdateScrollbar(const float newValue)
{
// Set this as the new value of our internal scrollbar representation.
// We're doing this so we can accumulate fractional amounts of a row to
// scroll each time the mouse scrolls.
_internalScrollbarPosition = std::clamp<double>(newValue, 0.0, _core->BufferHeight());
_internalScrollbarPosition = std::clamp(newValue, 0.0f, static_cast<float>(_core->BufferHeight()));

// If the new scrollbar position, rounded to an int, is at a different
// row, then actually update the scroll position in the core, and raise
// a ScrollPositionChanged to inform the control.
auto viewTop = ::base::saturated_cast<int>(::std::round(_internalScrollbarPosition));
const auto viewTop = std::lround(_internalScrollbarPosition);
if (viewTop != _core->ScrollOffset())
{
_core->UserScrollViewport(viewTop);
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/ControlInteractivity.h
Expand Up @@ -78,7 +78,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const Core::Point pixelPosition,
const Control::MouseButtonState state);

void UpdateScrollbar(const double newValue);
void UpdateScrollbar(const float newValue);

#pragma endregion

Expand Down Expand Up @@ -110,8 +110,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
std::unique_ptr<::Microsoft::Console::Render::UiaEngine> _uiaEngine;

winrt::com_ptr<ControlCore> _core{ nullptr };
unsigned int _rowsToScroll;
double _internalScrollbarPosition{ 0.0 };
UINT _rowsToScroll = 3;
float _internalScrollbarPosition = 0;

// If this is set, then we assume we are in the middle of panning the
// viewport via touch input.
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.idl
Expand Up @@ -64,7 +64,7 @@ namespace Microsoft.Terminal.Control
Microsoft.Terminal.Core.Point pixelPosition,
MouseButtonState state);

void UpdateScrollbar(Double newValue);
void UpdateScrollbar(Single newValue);

Boolean ManglePathsForWsl { get; };

Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalControl/EventArgs.h
Expand Up @@ -133,12 +133,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
struct TransparencyChangedEventArgs : public TransparencyChangedEventArgsT<TransparencyChangedEventArgs>
{
public:
TransparencyChangedEventArgs(const double opacity) :
TransparencyChangedEventArgs(const float opacity) :
_Opacity(opacity)
{
}

WINRT_PROPERTY(double, Opacity);
WINRT_PROPERTY(float, Opacity);
};

struct UpdateSearchResultsEventArgs : public UpdateSearchResultsEventArgsT<UpdateSearchResultsEventArgs>
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/EventArgs.idl
Expand Up @@ -75,7 +75,7 @@ namespace Microsoft.Terminal.Control

runtimeclass TransparencyChangedEventArgs
{
Double Opacity { get; };
Single Opacity { get; };
}

enum SearchState
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalControl/HwndTerminal.cpp
Expand Up @@ -1090,9 +1090,9 @@ til::rect HwndTerminal::GetPadding() const noexcept
return {};
}

double HwndTerminal::GetScaleFactor() const noexcept
float HwndTerminal::GetScaleFactor() const noexcept
{
return static_cast<double>(_currentDpi) / static_cast<double>(USER_DEFAULT_SCREEN_DPI);
return static_cast<float>(_currentDpi) / static_cast<float>(USER_DEFAULT_SCREEN_DPI);
}

void HwndTerminal::ChangeViewport(const til::inclusive_rect& NewWindow)
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/HwndTerminal.hpp
Expand Up @@ -146,7 +146,7 @@ struct HwndTerminal : ::Microsoft::Console::Types::IControlAccessibilityInfo
// Inherited via IControlAccessibilityInfo
til::size GetFontSize() const noexcept override;
til::rect GetBounds() const noexcept override;
double GetScaleFactor() const noexcept override;
float GetScaleFactor() const noexcept override;
void ChangeViewport(const til::inclusive_rect& NewWindow) override;
HRESULT GetHostUiaProvider(IRawElementProviderSimple** provider) noexcept override;
til::rect GetPadding() const noexcept override;
Expand Down

0 comments on commit 99061ee

Please sign in to comment.