fix: Allow negative values for tray and bar offset

CHANGELOG.md

@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed installation of docs when some are not generated (man, html...) ([`#2612`](https://github.com/polybar/polybar/pull/2612))
 - Fix `LDFLAGS` not being respected ([`#2619`](https://github.com/polybar/polybar/pull/2619))
 
+### Fixed
+- `tray-offset-x`, `tray-offset-y`, `offset-x`, and `offset-y` were mistakenly capped below at 0 ([`#2620`](https://github.com/polybar/polybar/pull/2620))
+
 ## [3.6.0] - 2022-03-01
 ### Breaking
 - We added the backslash escape character (\\) for configuration values. This means that the literal backslash character now has special meaning in configuration files, therefore if you want to use it in a value as a literal backslash, you need to escape it with the backslash escape character. The parser logs an error if any unescaped backslashes are found in a value. This affects you only if you are using two consecutive backslashes in a config value, which will now be interpreted as a single literal backslash. ([`#2354`](https://github.com/polybar/polybar/issues/2354))

include/utils/units.hpp

@@ -21,7 +21,8 @@ namespace units_utils {
 
   string extent_to_string(extent_val extent);
 
-  unsigned percentage_with_offset_to_pixel(percentage_with_offset g_format, double max, double dpi);
+  int percentage_with_offset_to_pixel(percentage_with_offset g_format, double max, double dpi);
+  unsigned percentage_with_offset_to_pixel_nonnegative(percentage_with_offset g_format, double max, double dpi);
 
   spacing_type parse_spacing_unit(const string& str);
   spacing_val parse_spacing(const string& str);

src/components/bar.cpp

@@ -221,8 +221,10 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   // Load values used to adjust the struts atom
   auto margin_top = m_conf.get("global/wm", "margin-top", percentage_with_offset{});
   auto margin_bottom = m_conf.get("global/wm", "margin-bottom", percentage_with_offset{});
-  m_opts.strut.top = units_utils::percentage_with_offset_to_pixel(margin_top, m_opts.monitor->h, m_opts.dpi_y);
+  m_opts.strut.top =
-  m_opts.strut.bottom = units_utils::percentage_with_offset_to_pixel(margin_bottom, m_opts.monitor->h, m_opts.dpi_y);
+      units_utils::percentage_with_offset_to_pixel_nonnegative(margin_top, m_opts.monitor->h, m_opts.dpi_y);
+  m_opts.strut.bottom =
+      units_utils::percentage_with_offset_to_pixel_nonnegative(margin_bottom, m_opts.monitor->h, m_opts.dpi_y);
 
   // Load commands used for fallback click handlers
   vector<action> actions;
@@ -295,19 +297,19 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
 
   m_opts.borders.emplace(edge::TOP, border_settings{});
   m_opts.borders[edge::TOP].size =
-      units_utils::percentage_with_offset_to_pixel(border_top, m_opts.monitor->h, m_opts.dpi_y);
+      units_utils::percentage_with_offset_to_pixel_nonnegative(border_top, m_opts.monitor->h, m_opts.dpi_y);
   m_opts.borders[edge::TOP].color = parse_or_throw_color("border-top-color", border_color);
   m_opts.borders.emplace(edge::BOTTOM, border_settings{});
   m_opts.borders[edge::BOTTOM].size =
-      units_utils::percentage_with_offset_to_pixel(border_bottom, m_opts.monitor->h, m_opts.dpi_y);
+      units_utils::percentage_with_offset_to_pixel_nonnegative(border_bottom, m_opts.monitor->h, m_opts.dpi_y);
   m_opts.borders[edge::BOTTOM].color = parse_or_throw_color("border-bottom-color", border_color);
   m_opts.borders.emplace(edge::LEFT, border_settings{});
   m_opts.borders[edge::LEFT].size =
-      units_utils::percentage_with_offset_to_pixel(border_left, m_opts.monitor->w, m_opts.dpi_x);
+      units_utils::percentage_with_offset_to_pixel_nonnegative(border_left, m_opts.monitor->w, m_opts.dpi_x);
   m_opts.borders[edge::LEFT].color = parse_or_throw_color("border-left-color", border_color);
   m_opts.borders.emplace(edge::RIGHT, border_settings{});
   m_opts.borders[edge::RIGHT].size =
-      units_utils::percentage_with_offset_to_pixel(border_right, m_opts.monitor->w, m_opts.dpi_x);
+      units_utils::percentage_with_offset_to_pixel_nonnegative(border_right, m_opts.monitor->w, m_opts.dpi_x);
   m_opts.borders[edge::RIGHT].color = parse_or_throw_color("border-right-color", border_color);
 
   // Load geometry values
@@ -316,8 +318,8 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   auto offsetx = m_conf.get(m_conf.section(), "offset-x", percentage_with_offset{});
   auto offsety = m_conf.get(m_conf.section(), "offset-y", percentage_with_offset{});
 
-  m_opts.size.w = units_utils::percentage_with_offset_to_pixel(w, m_opts.monitor->w, m_opts.dpi_x);
+  m_opts.size.w = units_utils::percentage_with_offset_to_pixel_nonnegative(w, m_opts.monitor->w, m_opts.dpi_x);
-  m_opts.size.h = units_utils::percentage_with_offset_to_pixel(h, m_opts.monitor->h, m_opts.dpi_y);
+  m_opts.size.h = units_utils::percentage_with_offset_to_pixel_nonnegative(h, m_opts.monitor->h, m_opts.dpi_y);
   m_opts.offset.x = units_utils::percentage_with_offset_to_pixel(offsetx, m_opts.monitor->w, m_opts.dpi_x);
   m_opts.offset.y = units_utils::percentage_with_offset_to_pixel(offsety, m_opts.monitor->h, m_opts.dpi_y);
 

src/utils/units.cpp

@@ -39,11 +39,14 @@ namespace units_utils {
   /**
    * Converts a percentage with offset into pixels
    */
-  unsigned int percentage_with_offset_to_pixel(percentage_with_offset g_format, double max, double dpi) {
+  int percentage_with_offset_to_pixel(percentage_with_offset g_format, double max, double dpi) {
     int offset_pixel = extent_to_pixel(g_format.offset, dpi);
 
-    return static_cast<unsigned int>(
+    return static_cast<int>(math_util::percentage_to_value<double, double>(g_format.percentage, max) + offset_pixel);
-        std::max<double>(0, math_util::percentage_to_value<double, double>(g_format.percentage, max) + offset_pixel));
+  }
+
+  unsigned percentage_with_offset_to_pixel_nonnegative(percentage_with_offset g_format, double max, double dpi) {
+    return std::max<int>(0, percentage_with_offset_to_pixel(g_format, max, dpi));
   }
 
   extent_type parse_extent_unit(const string& str) {

tests/unit_tests/utils/units.cpp

@@ -6,7 +6,6 @@
 using namespace polybar;
 using namespace units_utils;
 
-
 namespace polybar {
   bool operator==(const extent_val lhs, const extent_val rhs) {
     return lhs.type == rhs.type && lhs.value == rhs.value;
@@ -24,9 +23,9 @@ namespace polybar {
  * value represents the format string. The max value is always 1000 and dpi is always 96
  */
 class GeomFormatToPixelsTest : public ::testing::Test,
-                               public ::testing::WithParamInterface<pair<unsigned, percentage_with_offset>> {};
+                               public ::testing::WithParamInterface<pair<int, percentage_with_offset>> {};
 
-vector<pair<unsigned, percentage_with_offset>> to_pixels_no_offset_list = {
+vector<pair<int, percentage_with_offset>> to_pixels_no_offset_list = {
     {1000, percentage_with_offset{100.}},
     {0, percentage_with_offset{0.}},
     {1000, percentage_with_offset{150.}},
@@ -36,34 +35,33 @@ vector<pair<unsigned, percentage_with_offset>> to_pixels_no_offset_list = {
     {1, percentage_with_offset{0., extent_val{extent_type::PIXEL, 1}}},
 };
 
-vector<pair<unsigned, percentage_with_offset>> to_pixels_with_offset_list = {
+vector<pair<int, percentage_with_offset>> to_pixels_with_pixels_list = {
     {1000, percentage_with_offset{100., ZERO_PX_EXTENT}},
     {1010, percentage_with_offset{100., extent_val{extent_type::PIXEL, 10}}},
     {990, percentage_with_offset{100., extent_val{extent_type::PIXEL, -10}}},
     {10, percentage_with_offset{0., extent_val{extent_type::PIXEL, 10}}},
     {1000, percentage_with_offset{99., extent_val{extent_type::PIXEL, 10}}},
-    {0, percentage_with_offset{1., extent_val{extent_type::PIXEL, -100}}},
+    {-90, percentage_with_offset{1., extent_val{extent_type::PIXEL, -100}}},
 };
 
-vector<pair<unsigned, percentage_with_offset>> to_pixels_with_units_list = {
+vector<pair<int, percentage_with_offset>> to_pixels_with_points_list = {
     {1013, percentage_with_offset{100., extent_val{extent_type::POINT, 10}}},
     {987, percentage_with_offset{100., extent_val{extent_type::POINT, -10}}},
     {1003, percentage_with_offset{99., extent_val{extent_type::POINT, 10}}},
     {13, percentage_with_offset{0., extent_val{extent_type::POINT, 10}}},
-    {0, percentage_with_offset{0, extent_val{extent_type::POINT, -10}}},
 };
 
 INSTANTIATE_TEST_SUITE_P(NoOffset, GeomFormatToPixelsTest, ::testing::ValuesIn(to_pixels_no_offset_list));
 
-INSTANTIATE_TEST_SUITE_P(WithOffset, GeomFormatToPixelsTest, ::testing::ValuesIn(to_pixels_with_offset_list));
+INSTANTIATE_TEST_SUITE_P(WithPixels, GeomFormatToPixelsTest, ::testing::ValuesIn(to_pixels_with_pixels_list));
 
-INSTANTIATE_TEST_SUITE_P(WithUnits, GeomFormatToPixelsTest, ::testing::ValuesIn(to_pixels_with_units_list));
+INSTANTIATE_TEST_SUITE_P(WithPoints, GeomFormatToPixelsTest, ::testing::ValuesIn(to_pixels_with_points_list));
 
 static constexpr int MAX_WIDTH = 1000;
 static constexpr int DPI = 96;
 
 TEST_P(GeomFormatToPixelsTest, correctness) {
-  unsigned exp = GetParam().first;
+  int exp = GetParam().first;
   percentage_with_offset geometry = GetParam().second;
   EXPECT_DOUBLE_EQ(exp, percentage_with_offset_to_pixel(geometry, MAX_WIDTH, DPI));
 }
@@ -83,17 +81,6 @@ TEST(UnitsUtils, extent_to_pixel) {
   EXPECT_EQ(0, extent_to_pixel_nonnegative({extent_type::POINT, -36}, 96));
 }
 
-TEST(UnitsUtils, percentage_with_offset_to_pixel) {
-  EXPECT_EQ(1100, percentage_with_offset_to_pixel({100, {extent_type::PIXEL, 100}}, 1000, 0));
-  EXPECT_EQ(1048, percentage_with_offset_to_pixel({100, {extent_type::POINT, 36}}, 1000, 96));
-
-  EXPECT_EQ(900, percentage_with_offset_to_pixel({100, {extent_type::PIXEL, -100}}, 1000, 0));
-  EXPECT_EQ(952, percentage_with_offset_to_pixel({100, {extent_type::POINT, -36}}, 1000, 96));
-
-  EXPECT_EQ(0, percentage_with_offset_to_pixel({0, {extent_type::PIXEL, -100}}, 1000, 0));
-  EXPECT_EQ(100, percentage_with_offset_to_pixel({0, {extent_type::PIXEL, 100}}, 1000, 0));
-}
-
 TEST(UnitsUtils, parse_extent_unit) {
   EXPECT_EQ(extent_type::PIXEL, parse_extent_unit("px"));
   EXPECT_EQ(extent_type::POINT, parse_extent_unit("pt"));