From 0b240803361758b6383c2605e54b6d556813c494 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Thu, 3 Mar 2022 22:19:37 +0100 Subject: [PATCH] fix: Allow negative values for tray and bar offset --- CHANGELOG.md | 3 +++ include/utils/units.hpp | 3 ++- src/components/bar.cpp | 18 ++++++++++-------- src/utils/units.cpp | 9 ++++++--- tests/unit_tests/utils/units.cpp | 29 ++++++++--------------------- 5 files changed, 29 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cfb3324..69509043 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/include/utils/units.hpp b/include/utils/units.hpp index 9066ed42..9e2ee414 100644 --- a/include/utils/units.hpp +++ b/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); diff --git a/src/components/bar.cpp b/src/components/bar.cpp index 72716579..b1f63bb4 100644 --- a/src/components/bar.cpp +++ b/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.bottom = units_utils::percentage_with_offset_to_pixel(margin_bottom, m_opts.monitor->h, m_opts.dpi_y); + m_opts.strut.top = + 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 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.h = units_utils::percentage_with_offset_to_pixel(h, m_opts.monitor->h, m_opts.dpi_y); + 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_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); diff --git a/src/utils/units.cpp b/src/utils/units.cpp index 5bb201cd..5030be8a 100644 --- a/src/utils/units.cpp +++ b/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( - std::max(0, math_util::percentage_to_value(g_format.percentage, max) + offset_pixel)); + return static_cast(math_util::percentage_to_value(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(0, percentage_with_offset_to_pixel(g_format, max, dpi)); } extent_type parse_extent_unit(const string& str) { diff --git a/tests/unit_tests/utils/units.cpp b/tests/unit_tests/utils/units.cpp index d2aec33b..814546be 100644 --- a/tests/unit_tests/utils/units.cpp +++ b/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> {}; + public ::testing::WithParamInterface> {}; -vector> to_pixels_no_offset_list = { +vector> 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> to_pixels_no_offset_list = { {1, percentage_with_offset{0., extent_val{extent_type::PIXEL, 1}}}, }; -vector> to_pixels_with_offset_list = { +vector> 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> to_pixels_with_units_list = { +vector> 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"));