From 4dfbba11c4f84728b31c1b7c628283730d87da41 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Fri, 27 Nov 2020 20:35:31 +0100 Subject: [PATCH] Make rgba class immutable --- include/utils/color.hpp | 47 ++++++++++++++++++++---------- src/components/builder.cpp | 12 ++------ src/components/parser.cpp | 2 +- src/utils/color.cpp | 25 ++++++++++++++-- tests/unit_tests/utils/color.cpp | 50 ++++++++++++++++---------------- 5 files changed, 82 insertions(+), 54 deletions(-) diff --git a/include/utils/color.hpp b/include/utils/color.hpp index 7d18ec59..84ac2380 100644 --- a/include/utils/color.hpp +++ b/include/utils/color.hpp @@ -6,21 +6,12 @@ POLYBAR_NS -struct rgba { - /** - * Color value in the form ARGB or A000 depending on the type - */ - uint32_t m_value; - - /** - * NONE marks this instance as invalid. If such a color is encountered, it - * should be treated as if no color was set. - * - * ALPHA_ONLY is used for color strings that only have an alpha channel (#AA) - * these kinds should be combined with another color that has RGB channels - * before they are used to render anything. - */ - enum color_type { NONE, ARGB, ALPHA_ONLY } m_type{NONE}; +/** + * Represents immutable 32-bit color values. + */ +class rgba { + public: + enum color_type { NONE, ARGB, ALPHA_ONLY }; explicit rgba(); explicit rgba(uint32_t value, color_type type = ARGB); @@ -30,6 +21,9 @@ struct rgba { operator uint32_t() const; bool operator==(const rgba& other) const; + uint32_t value() const; + color_type type() const; + double a() const; double r() const; double g() const; @@ -41,7 +35,28 @@ struct rgba { uint8_t b_int() const; bool has_color() const; - void apply_alpha(rgba other); + rgba apply_alpha(rgba other) const; + rgba try_apply_alpha(rgba other) const; + + private: + /** + * Color value in the form ARGB or A000 depending on the type + * + * Cannot be const because we have to assign to it in the constructor and initializer lists are not possible. + */ + uint32_t m_value; + + /** + * NONE marks this instance as invalid. If such a color is encountered, it + * should be treated as if no color was set. + * + * ALPHA_ONLY is used for color strings that only have an alpha channel (#AA) + * these kinds should be combined with another color that has RGB channels + * before they are used to render anything. + * + * Cannot be const because we have to assign to it in the constructor and initializer lists are not possible. + */ + color_type m_type{NONE}; }; namespace color_util { diff --git a/src/components/builder.cpp b/src/components/builder.cpp index e01227e3..df81685c 100644 --- a/src/components/builder.cpp +++ b/src/components/builder.cpp @@ -259,11 +259,7 @@ void builder::font_close() { * Insert tag to alter the current background color */ void builder::background(rgba color) { - if (color.m_type == color.ALPHA_ONLY) { - rgba bg = m_bar.background; - bg.apply_alpha(color); - color = bg; - } + color = m_bar.background.try_apply_alpha(color); auto hex = color_util::simplify_hex(color); m_colors[syntaxtag::B] = hex; @@ -282,11 +278,7 @@ void builder::background_close() { * Insert tag to alter the current foreground color */ void builder::color(rgba color) { - if (color.m_type == color.ALPHA_ONLY) { - rgba fg = m_bar.foreground; - fg.apply_alpha(color); - color = fg; - } + color = m_bar.foreground.try_apply_alpha(color); auto hex = color_util::simplify_hex(color); m_colors[syntaxtag::F] = hex; diff --git a/src/components/parser.cpp b/src/components/parser.cpp index e3ef69ca..eb5069a9 100644 --- a/src/components/parser.cpp +++ b/src/components/parser.cpp @@ -223,7 +223,7 @@ rgba parser::parse_color(const string& s, rgba fallback) { if (!s.empty() && s[0] != '-') { rgba ret = rgba{s}; - if (!ret.has_color() || ret.m_type == rgba::ALPHA_ONLY) { + if (!ret.has_color() || ret.type() == rgba::ALPHA_ONLY) { logger::make().warn( "Invalid color in formatting tag detected: \"%s\", using fallback \"%s\". This is an issue with one of your " "formatting tags. If it is not, please report this as a bug.", diff --git a/src/utils/color.cpp b/src/utils/color.cpp index 2ec28448..8fdbc565 100644 --- a/src/utils/color.cpp +++ b/src/utils/color.cpp @@ -98,6 +98,15 @@ rgba::operator uint32_t() const { return m_value; } + +uint32_t rgba::value() const { + return this->m_value; +} + +rgba::color_type rgba::type() const { + return m_type; +} + double rgba::a() const { return a_int() / 255.0; } @@ -139,8 +148,20 @@ bool rgba::has_color() const { * * Useful for ALPHA_ONLY colors */ -void rgba::apply_alpha(rgba other) { - m_value = (m_value & 0x00FFFFFF) | (other.a_int() << 24); +rgba rgba::apply_alpha(rgba other) const { + uint32_t val = (m_value & 0x00FFFFFF) | (((uint32_t)other.a_int()) << 24); + return rgba(val, m_type); +} + +/** + * Calls apply_alpha, if the given color is ALPHA_ONLY. + */ +rgba rgba::try_apply_alpha(rgba other) const { + if (other.type() == ALPHA_ONLY) { + return apply_alpha(other); + } + + return *this; } string color_util::simplify_hex(string hex) { diff --git a/tests/unit_tests/utils/color.cpp b/tests/unit_tests/utils/color.cpp index b28dabc9..ea0c51e7 100644 --- a/tests/unit_tests/utils/color.cpp +++ b/tests/unit_tests/utils/color.cpp @@ -12,46 +12,46 @@ TEST(Rgba, constructor) { EXPECT_FALSE(v.has_color()); v = rgba{"#12"}; - EXPECT_EQ(rgba::ALPHA_ONLY, v.m_type); + EXPECT_EQ(rgba::ALPHA_ONLY, v.type()); v = rgba{"#ff"}; - EXPECT_EQ(0xff000000, v.m_value); + EXPECT_EQ(0xff000000, (uint32_t)v.value()); v = rgba{"#fff"}; - EXPECT_EQ(0xffffffff, v.m_value); + EXPECT_EQ(0xffffffff, v.value()); v = rgba{"#890"}; - EXPECT_EQ(0xFF889900, v.m_value); + EXPECT_EQ(0xFF889900, v.value()); v = rgba{"#a890"}; - EXPECT_EQ(0xaa889900, v.m_value); + EXPECT_EQ(0xaa889900, v.value()); v = rgba{"#55888777"}; - EXPECT_EQ(0x55888777, v.m_value); + EXPECT_EQ(0x55888777, v.value()); v = rgba{"#88aaaaaa"}; - EXPECT_EQ(0x88aaaaaa, v.m_value); + EXPECT_EQ(0x88aaaaaa, v.value()); v = rgba{"#00aaaaaa"}; - EXPECT_EQ(0x00aaaaaa, v.m_value); + EXPECT_EQ(0x00aaaaaa, v.value()); v = rgba{"#00FFFFFF"}; - EXPECT_EQ(0x00FFFFFF, v.m_value); + EXPECT_EQ(0x00FFFFFF, v.value()); } TEST(Rgba, parse) { - EXPECT_EQ(0xffffffff, rgba{"#fff"}.m_value); - EXPECT_EQ(0xffffffff, rgba{"fff"}.m_value); - EXPECT_EQ(0xff112233, rgba{"#123"}.m_value); - EXPECT_EQ(0xff112233, rgba{"123"}.m_value); - EXPECT_EQ(0xff888888, rgba{"#888888"}.m_value); - EXPECT_EQ(0xff888888, rgba{"888888"}.m_value); - EXPECT_EQ(0x00aa00aa, rgba{"#00aa00aa"}.m_value); - EXPECT_EQ(0x00aa00aa, rgba{"00aa00aa"}.m_value); - EXPECT_EQ(0x11223344, rgba{"#1234"}.m_value); - EXPECT_EQ(0x11223344, rgba{"1234"}.m_value); - EXPECT_EQ(0xaa000000, rgba{"#aa"}.m_value); - EXPECT_EQ(0xaa000000, rgba{"aa"}.m_value); + EXPECT_EQ(0xffffffff, rgba{"#fff"}.value()); + EXPECT_EQ(0xffffffff, rgba{"fff"}.value()); + EXPECT_EQ(0xff112233, rgba{"#123"}.value()); + EXPECT_EQ(0xff112233, rgba{"123"}.value()); + EXPECT_EQ(0xff888888, rgba{"#888888"}.value()); + EXPECT_EQ(0xff888888, rgba{"888888"}.value()); + EXPECT_EQ(0x00aa00aa, rgba{"#00aa00aa"}.value()); + EXPECT_EQ(0x00aa00aa, rgba{"00aa00aa"}.value()); + EXPECT_EQ(0x11223344, rgba{"#1234"}.value()); + EXPECT_EQ(0x11223344, rgba{"1234"}.value()); + EXPECT_EQ(0xaa000000, rgba{"#aa"}.value()); + EXPECT_EQ(0xaa000000, rgba{"aa"}.value()); } TEST(Rgba, string) { @@ -114,12 +114,12 @@ TEST(Rgba, channel) { TEST(Rgba, applyAlpha) { rgba v{0xCC123456}; - v.apply_alpha(rgba{0xAA000000, rgba::ALPHA_ONLY}); - EXPECT_EQ(0xAA123456, v.m_value); + rgba modified = v.apply_alpha(rgba{0xAA000000, rgba::ALPHA_ONLY}); + EXPECT_EQ(0xAA123456, modified.value()); v = rgba{0x00123456}; - v.apply_alpha(rgba{0xCC999999}); - EXPECT_EQ(0xCC123456, v.m_value); + modified = v.apply_alpha(rgba{0xCC999999}); + EXPECT_EQ(0xCC123456, modified.value()); } TEST(ColorUtil, simplify) {