From e89da5894032df0370402d6f7a4b6ed0ed4b2fde Mon Sep 17 00:00:00 2001 From: patrick96 Date: Tue, 1 Dec 2020 14:49:41 +0100 Subject: [PATCH] fix(builder): Properly apply alpha for fg and bg It was not quite clear how try_apply_alpha should behave if the given color was not ALPHA_ONLY. The implementation just returned 'this'. However, the build relied on it returning the given color. This broke all bg and fg settings in the entire bar. To clear this up, we switch this around take the alpha channel of 'this' and also return 'this' if it isn't ALPHA_ONLY. Fixes #2255 --- include/utils/color.hpp | 4 ++-- src/components/bar.cpp | 6 +----- src/components/builder.cpp | 4 ++-- src/utils/color.cpp | 19 ++++++++++--------- tests/unit_tests/utils/color.cpp | 20 +++++++++++++++----- 5 files changed, 30 insertions(+), 23 deletions(-) diff --git a/include/utils/color.hpp b/include/utils/color.hpp index bdf46ec1..e48e19e1 100644 --- a/include/utils/color.hpp +++ b/include/utils/color.hpp @@ -35,8 +35,8 @@ class rgba { uint8_t blue_i() const; bool has_color() const; - rgba apply_alpha(rgba other) const; - rgba try_apply_alpha(rgba other) const; + rgba apply_alpha_to(rgba other) const; + rgba try_apply_alpha_to(rgba other) const; private: /** diff --git a/src/components/bar.cpp b/src/components/bar.cpp index 94d2bf07..4fdfbce5 100644 --- a/src/components/bar.cpp +++ b/src/components/bar.cpp @@ -205,11 +205,7 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const * These are the base colors of the bar and cannot be alpha only * In that case, we just use the alpha channel on the default value. */ - if (color.type() == rgba::ALPHA_ONLY) { - return def.apply_alpha(color); - } else { - return color; - } + return color.try_apply_alpha_to(def); } catch (const exception& err) { throw application_error(sstream() << "Failed to set " << key << " (reason: " << err.what() << ")"); } diff --git a/src/components/builder.cpp b/src/components/builder.cpp index df81685c..2ed7648e 100644 --- a/src/components/builder.cpp +++ b/src/components/builder.cpp @@ -259,7 +259,7 @@ void builder::font_close() { * Insert tag to alter the current background color */ void builder::background(rgba color) { - color = m_bar.background.try_apply_alpha(color); + color = color.try_apply_alpha_to(m_bar.background); auto hex = color_util::simplify_hex(color); m_colors[syntaxtag::B] = hex; @@ -278,7 +278,7 @@ void builder::background_close() { * Insert tag to alter the current foreground color */ void builder::color(rgba color) { - color = m_bar.foreground.try_apply_alpha(color); + color = color.try_apply_alpha_to(m_bar.foreground); auto hex = color_util::simplify_hex(color); m_colors[syntaxtag::F] = hex; diff --git a/src/utils/color.cpp b/src/utils/color.cpp index 0abfda62..3123e2e5 100644 --- a/src/utils/color.cpp +++ b/src/utils/color.cpp @@ -140,21 +140,22 @@ bool rgba::has_color() const { } /** - * Replaces the current alpha channel with the alpha channel of the other color - * - * Useful for ALPHA_ONLY colors + * Applies the alpha channel of this color to the given color. */ -rgba rgba::apply_alpha(rgba other) const { - uint32_t val = (m_value & 0x00FFFFFF) | (((uint32_t)other.alpha_i()) << 24); +rgba rgba::apply_alpha_to(rgba other) const { + uint32_t val = (other.value() & 0x00FFFFFF) | (((uint32_t)alpha_i()) << 24); return rgba(val, m_type); } /** - * Calls apply_alpha, if the given color is ALPHA_ONLY. + * If this is an ALPHA_ONLY color, applies this alpha channel to the other + * color, otherwise just returns this. + * + * \returns the new color if this is ALPHA_ONLY or a copy of this otherwise. */ -rgba rgba::try_apply_alpha(rgba other) const { - if (other.type() == ALPHA_ONLY) { - return apply_alpha(other); +rgba rgba::try_apply_alpha_to(rgba other) const { + if (m_type == ALPHA_ONLY) { + return apply_alpha_to(other); } return *this; diff --git a/tests/unit_tests/utils/color.cpp b/tests/unit_tests/utils/color.cpp index 8ce76e17..29c0e3f8 100644 --- a/tests/unit_tests/utils/color.cpp +++ b/tests/unit_tests/utils/color.cpp @@ -96,16 +96,26 @@ TEST(Rgba, channel) { EXPECT_EQ(0x99 / 255.0, rgba{0x88449933}.green_d()); } -TEST(Rgba, applyAlpha) { - rgba v{0xCC123456}; - rgba modified = v.apply_alpha(rgba{0xAA000000, rgba::ALPHA_ONLY}); +TEST(Rgba, applyAlphaTo) { + rgba v{0xAA000000, rgba::ALPHA_ONLY}; + rgba modified = v.apply_alpha_to(rgba{0xCC123456, rgba::ALPHA_ONLY}); EXPECT_EQ(0xAA123456, modified.value()); - v = rgba{0x00123456}; - modified = v.apply_alpha(rgba{0xCC999999}); + v = rgba{0xCC999999}; + modified = v.apply_alpha_to(rgba{0x00123456}); EXPECT_EQ(0xCC123456, modified.value()); } +TEST(Rgba, tryApplyAlphaTo) { + rgba v{0xAA000000, rgba::ALPHA_ONLY}; + rgba modified = v.try_apply_alpha_to(rgba{0xCC123456, rgba::ALPHA_ONLY}); + EXPECT_EQ(0xAA123456, modified.value()); + + v = rgba{0xCC999999}; + modified = v.try_apply_alpha_to(rgba{0x00123456}); + EXPECT_EQ(0xCC999999, modified.value()); +} + TEST(ColorUtil, simplify) { EXPECT_EQ("#111", color_util::simplify_hex("#FF111111")); EXPECT_EQ("#234", color_util::simplify_hex("#ff223344"));