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.

Sign up for GitHub

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

Unify padding parsers, make SUI operate on struct Thickness #18300

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Unify padding parsers, make SUI operate on struct Thickness #18300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3330bba
Unify our `padding` parsers, make SUI operate on `Thickness` structs
DHowett Dec 10, 2024
f2017ee
Yeah I usually use -IgnoreXaml
DHowett Dec 10, 2024
bbfdec6
Audit this
DHowett Dec 10, 2024
6528401
Merge remote-tracking branch 'origin/main' into dev/duhowett/thiccness
DHowett Jan 9, 2025
529ef31
Wait, nobody is using MaxValueFromPaddingString
DHowett Jan 9, 2025
8fe23e6
just be normal; address PR feedback
DHowett Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 3 additions & 60 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
auto settings{ _core.Settings() };

// Apply padding as swapChainPanel's margin
const auto newMargin = ParseThicknessFromPadding(settings.Padding());
const auto newMargin = StringToXamlThickness(settings.Padding());
SwapChainPanel().Margin(newMargin);

// Apply settings for scrollbar
Expand Down Expand Up @@ -2921,7 +2921,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

float height = rows * static_cast<float>(actualFontSize.height);
const auto thickness = ParseThicknessFromPadding(padding);
const auto thickness = StringToXamlThickness(padding);
// GH#2061 - make sure to account for the size the padding _will be_ scaled to
width += scale * static_cast<float>(thickness.Left + thickness.Right);
height += scale * static_cast<float>(thickness.Top + thickness.Bottom);
Expand Down Expand Up @@ -2957,7 +2957,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
width += scrollbarSize;
}

const auto thickness = ParseThicknessFromPadding(padding);
const auto thickness = StringToXamlThickness(padding);
// GH#2061 - make sure to account for the size the padding _will be_ scaled to
width += scale * static_cast<float>(thickness.Left + thickness.Right);
height += scale * static_cast<float>(thickness.Top + thickness.Bottom);
Expand Down Expand Up @@ -3056,63 +3056,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_core.WindowVisibilityChanged(showOrHide);
}

// Method Description:
// - Create XAML Thickness object based on padding props provided.
// Used for controlling the TermControl XAML Grid container's Padding prop.
// Arguments:
// - padding: 2D padding values
// Single Double value provides uniform padding
// Two Double values provide isometric horizontal & vertical padding
// Four Double values provide independent padding for 4 sides of the bounding rectangle
// Return Value:
// - Windows::UI::Xaml::Thickness object
Windows::UI::Xaml::Thickness TermControl::ParseThicknessFromPadding(const hstring padding)
{
const auto singleCharDelim = L',';
std::wstringstream tokenStream(padding.c_str());
std::wstring token;
uint8_t paddingPropIndex = 0;
std::array<double, 4> thicknessArr = {};
size_t* idx = nullptr;

// Get padding values till we run out of delimiter separated values in the stream
// or we hit max number of allowable values (= 4) for the bounding rectangle
// Non-numeral values detected will default to 0
// std::getline will not throw exception unless flags are set on the wstringstream
// std::stod will throw invalid_argument exception if the input is an invalid double value
// std::stod will throw out_of_range exception if the input value is more than DBL_MAX
try
{
for (; std::getline(tokenStream, token, singleCharDelim) && (paddingPropIndex < thicknessArr.size()); paddingPropIndex++)
{
// std::stod internally calls wcstod which handles whitespace prefix (which is ignored)
// & stops the scan when first char outside the range of radix is encountered
// We'll be permissive till the extent that stod function allows us to be by default
// Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail
thicknessArr[paddingPropIndex] = std::stod(token, idx);
}
}
catch (...)
{
// If something goes wrong, even if due to a single bad padding value, we'll reset the index & return default 0 padding
paddingPropIndex = 0;
LOG_CAUGHT_EXCEPTION();
}

switch (paddingPropIndex)
{
case 1:
return ThicknessHelper::FromUniformLength(thicknessArr[0]);
case 2:
return ThicknessHelper::FromLengths(thicknessArr[0], thicknessArr[1], thicknessArr[0], thicknessArr[1]);
// No case for paddingPropIndex = 3, since it's not a norm to provide just Left, Top & Right padding values leaving out Bottom
case 4:
return ThicknessHelper::FromLengths(thicknessArr[0], thicknessArr[1], thicknessArr[2], thicknessArr[3]);
default:
return Thickness();
}
}

// Method Description:
// - Get the modifier keys that are currently pressed. This can be used to
// find out which modifiers (ctrl, alt, shift) are pressed in events that
Expand Down
134 changes: 25 additions & 109 deletions src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
else if (viewModelProperty == L"Padding")
{
_parsedPadding = StringToXamlThickness(_profile.Padding());
_NotifyChanges(L"LeftPadding", L"TopPadding", L"RightPadding", L"BottomPadding");
}
});
Expand All @@ -157,6 +158,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_unfocusedAppearanceViewModel = winrt::make<implementation::AppearanceViewModel>(profile.UnfocusedAppearance().try_as<AppearanceConfig>());
}

_parsedPadding = StringToXamlThickness(_profile.Padding());
_defaultAppearanceViewModel.IsDefault(true);
}

Expand Down Expand Up @@ -217,145 +219,59 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void ProfileViewModel::LeftPadding(double value) noexcept
{
const hstring& padding = _GetNewPadding(PaddingDirection::Left, value);

Padding(padding);
if (std::abs(_parsedPadding.Left - value) >= .0001)
Copy link
Member Author

Choose a reason for hiding this comment

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

@lhecker I was so horribly indoctrinated at a young age - this can't STILL be right, can it?

{
_parsedPadding.Left = value;
Padding(XamlThicknessToOptimalString(_parsedPadding));
}
}

double ProfileViewModel::LeftPadding() const noexcept
{
return _GetPaddingValue(PaddingDirection::Left);
return _parsedPadding.Left;
}

void ProfileViewModel::TopPadding(double value) noexcept
{
const hstring& padding = _GetNewPadding(PaddingDirection::Top, value);

Padding(padding);
if (std::abs(_parsedPadding.Top - value) >= .0001)
{
_parsedPadding.Top = value;
Padding(XamlThicknessToOptimalString(_parsedPadding));
}
}

double ProfileViewModel::TopPadding() const noexcept
{
return _GetPaddingValue(PaddingDirection::Top);
return _parsedPadding.Top;
}

void ProfileViewModel::RightPadding(double value) noexcept
{
const hstring& padding = _GetNewPadding(PaddingDirection::Right, value);

Padding(padding);
if (std::abs(_parsedPadding.Right - value) >= .0001)
{
_parsedPadding.Right = value;
Padding(XamlThicknessToOptimalString(_parsedPadding));
}
}

double ProfileViewModel::RightPadding() const noexcept
{
return _GetPaddingValue(PaddingDirection::Right);
return _parsedPadding.Right;
}

void ProfileViewModel::BottomPadding(double value) noexcept
{
const hstring& padding = _GetNewPadding(PaddingDirection::Bottom, value);

Padding(padding);
}

double ProfileViewModel::BottomPadding() const noexcept
{
return _GetPaddingValue(PaddingDirection::Bottom);
}

winrt::hstring ProfileViewModel::_GetNewPadding(PaddingDirection paddingDirection, double newPaddingValue) const
{
std::array<double, 4> values{};
std::wstring_view padding{ Padding() };
uint32_t paddingIndex = static_cast<uint32_t>(paddingDirection);

try
{
uint32_t index = 0;
for (const auto& token : til::split_iterator{ padding, L',' })
{
auto curVal = std::stod(std::wstring{ token });

if (paddingIndex == index)
{
curVal = newPaddingValue;
}

values[index++] = curVal;

if (index >= values.size())
{
break;
}
}
}
catch (...)
if (std::abs(_parsedPadding.Bottom - value) >= .0001)
{
values.fill(0);
LOG_CAUGHT_EXCEPTION();
_parsedPadding.Bottom = value;
Padding(XamlThicknessToOptimalString(_parsedPadding));
}

const auto result = fmt::format(FMT_COMPILE(L"{:.6f}"), fmt::join(values, L","));

return winrt::hstring{ result };
}

double ProfileViewModel::_GetPaddingValue(PaddingDirection paddingDirection) const
double ProfileViewModel::BottomPadding() const noexcept
{
std::wstring_view padding{ Padding() };
uint32_t paddingIndex = static_cast<uint32_t>(paddingDirection);
std::array<double, 4> paddingValues{};
double paddingValue = 0.;
uint32_t index = 0;

try
{
for (const auto& token : til::split_iterator{ padding, L',' })
{
auto curVal = std::stod(std::wstring{ token });

paddingValues[index++] = curVal;

if (index >= paddingValues.size())
{
break;
}
}
}
catch (...)
{
paddingValue = 0.;
LOG_CAUGHT_EXCEPTION();
}

// Padding: 8
if (index == 1)
{
paddingValue = paddingValues[0];
}
// Padding: 8, 4
else if (index == 2)
{
if (paddingDirection == PaddingDirection::Left ||
paddingDirection == PaddingDirection::Right)
{
paddingValue = paddingValues[0];
}
else if (paddingDirection == PaddingDirection::Top ||
paddingDirection == PaddingDirection::Bottom)
{
paddingValue = paddingValues[1];
}
}
// Padding: 8, 4, 8, 4
else
{
paddingValue = paddingValues[paddingIndex];
}

return paddingValue;
return _parsedPadding.Bottom;
}

Model::TerminalSettings ProfileViewModel::TermSettings() const
{
return Model::TerminalSettings::CreateForPreview(_appSettings, _profile);
Expand Down
13 changes: 2 additions & 11 deletions src/cascadia/TerminalSettingsEditor/ProfileViewModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
Editor::AppearanceViewModel _defaultAppearanceViewModel;
Windows::UI::Core::CoreDispatcher _dispatcher;

winrt::Windows::UI::Xaml::Thickness _parsedPadding;

void _InitializeCurrentBellSounds();
void _PrepareModelForBellSoundModification();
void _MarkDuplicateBellSoundDirectories();
Expand All @@ -185,17 +187,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

Model::CascadiaSettings _appSettings;
Editor::AppearanceViewModel _unfocusedAppearanceViewModel;

enum class PaddingDirection
{
Left = 0,
Top = 1,
Right = 2,
Bottom = 3
};

winrt::hstring _GetNewPadding(PaddingDirection paddingDirection, double newPaddingValue) const;
double _GetPaddingValue(PaddingDirection paddingDirection) const;
void _UpdateBuiltInIcons();
void _DeduceCurrentIconType();
void _DeduceCurrentBuiltInIcon();
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
<!-- Padding -->
<local:SettingContainer x:Uid="Profile_Padding"
ClearSettingValue="{x:Bind Profile.ClearPadding}"
CurrentValue="{x:Bind Profile.Padding, Mode=OneWay}"
HasSettingValue="{x:Bind Profile.HasPadding, Mode=OneWay}"
SettingOverrideSource="{x:Bind Profile.PaddingOverrideSource, Mode=OneWay}"
Style="{StaticResource ExpanderSettingContainerStyle}">
Expand Down
32 changes: 0 additions & 32 deletions src/cascadia/UIHelpers/Converters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,36 +77,4 @@ namespace winrt::Microsoft::Terminal::UI::implementation
{
return fontWeight.Weight;
}

double Converters::MaxValueFromPaddingString(const winrt::hstring& paddingString)
{
std::wstring buffer;
double maxVal = 0;

auto& errnoRef = errno; // Nonzero cost, pay it once

// Get padding values till we run out of delimiter separated values in the stream
// Non-numeral values detected will default to 0
// std::stod will throw invalid_argument exception if the input is an invalid double value
// std::stod will throw out_of_range exception if the input value is more than DBL_MAX
for (const auto& part : til::split_iterator{ std::wstring_view{ paddingString }, L',' })
{
buffer.assign(part);

// wcstod handles whitespace prefix (which is ignored) & stops the
// scan when first char outside the range of radix is encountered.
// We'll be permissive till the extent that stod function allows us to be by default
// Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail
errnoRef = 0;
wchar_t* end;
const double val = wcstod(buffer.c_str(), &end);

if (end != buffer.c_str() && errnoRef != ERANGE)
{
maxVal = std::max(maxVal, val);
}
}

return maxVal;
}
}
1 change: 0 additions & 1 deletion src/cascadia/UIHelpers/Converters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ namespace winrt::Microsoft::Terminal::UI::implementation
static winrt::Windows::UI::Text::FontWeight DoubleToFontWeight(double value);
static winrt::Windows::UI::Xaml::Media::SolidColorBrush ColorToBrush(winrt::Windows::UI::Color color);
static double FontWeightToDouble(winrt::Windows::UI::Text::FontWeight fontWeight);
static double MaxValueFromPaddingString(const winrt::hstring& paddingString);
};
}

Expand Down
1 change: 0 additions & 1 deletion src/cascadia/UIHelpers/Converters.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,5 @@ namespace Microsoft.Terminal.UI
static Windows.UI.Text.FontWeight DoubleToFontWeight(Double value);
static Windows.UI.Xaml.Media.SolidColorBrush ColorToBrush(Windows.UI.Color color);
static Double FontWeightToDouble(Windows.UI.Text.FontWeight fontWeight);
static Double MaxValueFromPaddingString(String paddingString);
}
}
Loading
Loading