Unify padding parsers, make SUI operate on struct Thickness

src/cascadia/TerminalControl/TermControl.cpp

@@ -902,7 +902,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
@@ -2858,7 +2858,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);
@@ -2894,7 +2894,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);
@@ -2993,63 +2993,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

src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp

@@ -83,6 +83,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");
             }
         });
@@ -104,150 +105,65 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             _unfocusedAppearanceViewModel = winrt::make<implementation::AppearanceViewModel>(profile.UnfocusedAppearance().try_as<AppearanceConfig>());
         }
 
+        _parsedPadding = StringToXamlThickness(_profile.Padding());
         _defaultAppearanceViewModel.IsDefault(true);
     }
 
     void ProfileViewModel::LeftPadding(double value) noexcept
     {
-        const hstring& padding = _GetNewPadding(PaddingDirection::Left, value);
-
-        Padding(padding);
+        if (std::abs(_parsedPadding.Left - value) >= .0001)
+        {
+            _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);

src/cascadia/TerminalSettingsEditor/ProfileViewModel.h

@@ -147,22 +147,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         winrt::hstring _lastIcon;
         Editor::AppearanceViewModel _defaultAppearanceViewModel;
 
+        winrt::Windows::UI::Xaml::Thickness _parsedPadding;
+
         static Windows::Foundation::Collections::IObservableVector<Editor::Font> _MonospaceFontList;
         static Windows::Foundation::Collections::IObservableVector<Editor::Font> _FontList;
 
         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;
     };
 
     struct DeleteProfileEventArgs :

src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml

@@ -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}">

src/cascadia/inc/cppwinrt_utils.h

@@ -288,4 +288,69 @@ std::vector<wil::com_ptr<T>> SafeArrayToOwningVector(SAFEARRAY* safeArray)
     namespace nameSpace::factory_implementation                                                                   \
     {                                                                                                             \
         BASIC_FACTORY(className);                                                                                 \
-    }\
+    }
+
+#ifdef WINRT_Windows_UI_Xaml_H
+
+inline ::winrt::hstring XamlThicknessToOptimalString(const ::winrt::Windows::UI::Xaml::Thickness& t)
+{
+    if (t.Left == t.Right)
+    {
+        if (t.Top == t.Bottom)
+        {
+            if (t.Top == t.Left)
+            {
+                return ::winrt::hstring{ fmt::format(FMT_COMPILE(L"{}"), t.Left) };
+            }
+            return ::winrt::hstring{ fmt::format(FMT_COMPILE(L"{},{}"), t.Left, t.Top) };
+        }
+        // fall through
+    }
+    return ::winrt::hstring{ fmt::format(FMT_COMPILE(L"{},{},{},{}"), t.Left, t.Top, t.Right, t.Bottom) };
+}
+
+inline ::winrt::Windows::UI::Xaml::Thickness StringToXamlThickness(std::wstring_view padding)
+try
+{
+    uintptr_t count{ 0 };
+    double t[4]{ 0. }; // left, top, right, bottom
+    wchar_t buf[17];
+    for (const auto& token : til::split_iterator{ padding, L',' })
+    {
+        const auto l{ std::min(token.size(), std::extent_v<decltype(buf)> - 1) };
+#pragma warning(suppress : 26459) // You called an STL function '...' with a raw pointer parameter at position '...' that may be unsafe ... (stl.1).
+        std::copy_n(token.data(), l, &buf[0]); // the length of buf is controlled for above
+        til::at(buf, l) = L'\0';
+        til::at(t, count++) = std::wcstod(&buf[0], nullptr);
+        if (count >= 4)
+        {
+            break;
+        }
+    }
+
+#pragma warning(push)
+#pragma warning(disable : 26446) // Prefer to use gsl::at() instead of unchecked subscript operator (bounds.4).
+    switch (count)
+    {
+    case 1: // one input = all 4 values are the same
+        t[1] = t[0]; // top = left
+        __fallthrough;
+    case 2: // two inputs = top/bottom and left/right are the same
+        t[2] = t[0]; // right = left
+        t[3] = t[1]; // bottom = top
+        __fallthrough;
+    case 4: // four inputs = fully specified
+        break;
+    default:
+        return {};
+    }
+    return { t[0], t[1], t[2], t[3] };
+#pragma warning(pop)
+}
+catch (...)
+{
+    LOG_CAUGHT_EXCEPTION();
+    return {};
+}
+
+#endif