From af7e8c9d39a80ec165bf913a276534a51bf6bbf9 Mon Sep 17 00:00:00 2001 From: Twan van Laarhoven Date: Sun, 26 Apr 2020 21:41:35 +0200 Subject: [PATCH] Switch (back) to our own Color type instead of using wxColour. The reason is that wxColour's default constructor creates an invalid color (what is that even?). It is nicer to just have default be transparent. --- src/data/field/color.cpp | 9 ++++-- src/gfx/color.cpp | 46 +++++++++++++++++++------------ src/gfx/color.hpp | 47 ++++++++++++++++++++++++++++---- src/gui/about_window.cpp | 12 ++++---- src/gui/control/gallery_list.cpp | 2 +- src/gui/set/cards_panel.cpp | 2 +- src/gui/set/stats_panel.cpp | 2 +- src/gui/value/color.cpp | 4 +-- src/render/text/element.cpp | 10 +++++-- src/script/functions/image.cpp | 2 +- src/script/value.cpp | 6 ++-- src/util/prec.hpp | 1 - src/util/rotation.cpp | 4 +-- 13 files changed, 101 insertions(+), 46 deletions(-) diff --git a/src/data/field/color.cpp b/src/data/field/color.cpp index ce7a85c0..4022f282 100644 --- a/src/data/field/color.cpp +++ b/src/data/field/color.cpp @@ -41,7 +41,12 @@ IMPLEMENT_REFLECTION(ColorField) { IMPLEMENT_REFLECTION(ColorField::Choice) { REFLECT_IF_READING_SINGLE_VALUE { REFLECT_NAMELESS(name); - color = parse_color(name); + auto col = parse_color(name); + if (col) { + color = *col; + } else { + // TODO: handler.warning(_("Not a valid color value: ") + name); + } } else { REFLECT(name); REFLECT(color); @@ -78,7 +83,7 @@ ColorValue::ColorValue(const ColorFieldP& field) : Value(field) , value( !field->initial.isDefault() ? field->initial() : !field->choices.empty() ? field->choices[0]->color - : *wxBLACK + : Color() , true) {} diff --git a/src/gfx/color.cpp b/src/gfx/color.cpp index 1afe9fcf..cb246d41 100644 --- a/src/gfx/color.cpp +++ b/src/gfx/color.cpp @@ -11,11 +11,14 @@ // ----------------------------------------------------------------------------- : Parsing etc. -template <> void Reader::handle(Color& col) { - col = parse_color(getValue()); - if (!col.Ok()) { - col = Color(0,0,0,0); - warning(_("Not a valid color value")); +template <> void Reader::handle(Color& out) { + String const& str = getValue(); + auto col = parse_color(str); + if (!col.has_value()) { + out = Color(); + warning(_("Not a valid color value: ") + str); + } else { + out = *col; } } @@ -24,7 +27,7 @@ template <> void Writer::handle(const Color& col) { } -Color parse_color(const String& v) { +optional parse_color(const String& v) { UInt r,g,b,a; if (wxSscanf(v.c_str(),_("rgb(%u,%u,%u)"),&r,&g,&b)) { return Color(r, g, b); @@ -33,7 +36,13 @@ Color parse_color(const String& v) { } else if (v == _("transparent")) { return Color(0,0,0,0); } else { - return Color(v); + // Try to find a named color + wxColour c = wxTheColourDatabase->Find(v); + if (c.Ok()) { + return Color(c); + } else { + return optional(); + } } } @@ -49,7 +58,7 @@ String format_color(Color col) { // ----------------------------------------------------------------------------- : Color utility functions -Color lerp(const Color& a, const Color& b, double t) { +Color lerp(Color a, Color b, double t) { return Color(static_cast( a.Red() + (b.Red() - a.Red() ) * t ), static_cast( a.Green() + (b.Green() - a.Green()) * t ), static_cast( a.Blue() + (b.Blue() - a.Blue() ) * t ), @@ -79,21 +88,22 @@ Color hsl2rgb(double h, double s, double l) { } -Color darken(const Color& c) { +Color darken(Color c) { return Color( - c.Red() * 8 / 10, - c.Green() * 8 / 10, - c.Blue() * 8 / 10 + c.r * 8 / 10, + c.g * 8 / 10, + c.b * 8 / 10, + c.a ); } -Color saturate(const Color& c, double amount) { - int r = c.Red(), g = c.Green(), b = c.Blue(); - double l = (r + g + b) / 3; +Color saturate(Color c, double amount) { + double l = (c.r + c.g + c.b) / 3; return Color( - col(static_cast( (r - amount * l) / (1 - amount) )), - col(static_cast( (g - amount * l) / (1 - amount) )), - col(static_cast( (b - amount * l) / (1 - amount) )) + col(static_cast( (c.r - amount * l) / (1 - amount) )), + col(static_cast( (c.g - amount * l) / (1 - amount) )), + col(static_cast( (c.b - amount * l) / (1 - amount) )), + c.a ); } diff --git a/src/gfx/color.hpp b/src/gfx/color.hpp index dc336559..295a88a1 100644 --- a/src/gfx/color.hpp +++ b/src/gfx/color.hpp @@ -14,9 +14,45 @@ // ----------------------------------------------------------------------------- : Includes #include +#include + +// ----------------------------------------------------------------------------- : Colors + +// Colors. +// Ideally we would just use wxColour, but that class is stupid because it has an "invalid" state, +// and the default constructor makes invalid values. +struct Color { + union { + struct { + unsigned char r, g, b, a; + }; + uint32_t packed; + }; + + inline Color() : r(0), g(0), b(0), a(0) {} + inline Color(unsigned char r, unsigned char g, unsigned char b, unsigned char a = 255) : r(r), g(g), b(b), a(a) {} + inline Color(wxColour color) : r(color.Red()), g(color.Green()), b(color.Blue()), a(color.Alpha()) {} + + inline operator wxColour() const { return wxColour(r, g, b, a); } + inline operator wxPen() const { return wxPen((wxColour)*this); } + inline operator wxBrush() const { return wxBrush((wxColour)*this); } + + inline bool operator == (Color const& that) const { + return packed == that.packed; + } + inline bool operator != (Color const& that) const { + return packed != that.packed; + } + + inline constexpr unsigned char Red() const { return r; } + inline constexpr unsigned char Green() const { return g; } + inline constexpr unsigned char Blue() const { return b; } + inline constexpr unsigned char Alpha() const { return a; } +}; // ----------------------------------------------------------------------------- // RGB Color, packed into 3 bytes +// These are used for arrays in wxImage // ----------------------------------------------------------------------------- // stupid headers stealing useful names @@ -37,6 +73,7 @@ struct RGB { RGB() {} RGB(Byte x) : r(x), g(x), b(x) {} RGB(Byte r, Byte g, Byte b) : r(r), g(g), b(b) {} + RGB(Color x) : r(x.Red()), g(x.Green()), b(x.Blue()) {} RGB(wxColour const& x) : r(x.Red()), g(x.Green()), b(x.Blue()) {} inline int total() { return r+g+b; } @@ -65,7 +102,7 @@ struct RGB { // ----------------------------------------------------------------------------- : Parsing /// Parse a color -Color parse_color(const String& value); +optional parse_color(const String& value); /// Convert a Color to a string String format_color(Color col); @@ -77,19 +114,19 @@ inline int top(int x) { return min(255, x); } ///< top range check for color inline int col(int x) { return top(bot(x)); } ///< top and bottom range check for color values /// Linear interpolation between colors -Color lerp(const Color& a, const Color& b, double t); +Color lerp(Color a, Color b, double t); /// convert HSL to RGB, h,s,l must be in range [0...1) Color hsl2rgb(double h, double s, double l); /// A darker version of a color -Color darken(const Color& c); +Color darken(Color c); /// A black or white color, that contrasts with c -Color contrasting_color(const Color& c); +Color contrasting_color(Color c); /// A saturated version of a color -Color saturate(const Color& c, double amount); +Color saturate(Color c, double amount); /// Recolor: /** diff --git a/src/gui/about_window.cpp b/src/gui/about_window.cpp index 4bf45f5e..f8a50de6 100644 --- a/src/gui/about_window.cpp +++ b/src/gui/about_window.cpp @@ -40,7 +40,7 @@ void AboutWindow::draw(DC& dc) { wxSize ws = GetClientSize(); // draw background dc.SetPen (*wxTRANSPARENT_PEN); - dc.SetBrush(Color(240,247,255)); + dc.SetBrush(wxColor(240,247,255)); dc.DrawRectangle(0, 0, ws.GetWidth(), ws.GetHeight()); // draw logo dc.DrawBitmap(logo, (ws.GetWidth() - logo.GetWidth()) / 2, 5); @@ -48,11 +48,11 @@ void AboutWindow::draw(DC& dc) { dc.DrawBitmap(logo2, ws.GetWidth() - logo2.GetWidth(), ws.GetHeight() - logo2.GetHeight()); #endif // draw version box - dc.SetPen (wxPen(Color(184,29,19), 2)); - dc.SetBrush(Color(114,197,224)); + dc.SetPen (wxPen(wxColor(184,29,19), 2)); + dc.SetBrush(wxColor(114,197,224)); dc.DrawRectangle(28, 104, 245, 133); - dc.SetTextBackground(Color(114,197,224)); - dc.SetTextForeground(Color(0,0,0)); + dc.SetTextBackground(wxColor(114,197,224)); + dc.SetTextForeground(wxColor(0,0,0)); // draw version info dc.SetFont(wxFont(9, wxFONTFAMILY_SWISS, wxFONTSTYLE_NORMAL, wxFONTWEIGHT_NORMAL, false, _("Arial"))); dc.DrawText(_("Version: ") + app_version.toString() + version_suffix, 34, 110); @@ -202,7 +202,7 @@ void HoverButton::draw(DC& dc) { // clear background (for transparent button images) wxSize ws = GetClientSize(); dc.SetPen(*wxTRANSPARENT_PEN); - dc.SetBrush(background != wxNullColour ? background : wxSystemSettings::GetColour(wxSYS_COLOUR_3DFACE)); + dc.SetBrush(wxBrush(background.Alpha()>0 ? wxColour(background) : wxSystemSettings::GetColour(wxSYS_COLOUR_3DFACE))); dc.DrawRectangle(0, 0, ws.GetWidth(), ws.GetHeight()); // draw button dc.DrawBitmap(*toDraw(), 0, 0, true); diff --git a/src/gui/control/gallery_list.cpp b/src/gui/control/gallery_list.cpp index c878c0d3..c302e68c 100644 --- a/src/gui/control/gallery_list.cpp +++ b/src/gui/control/gallery_list.cpp @@ -310,7 +310,7 @@ void GalleryList::OnDraw(DC& dc) { } #else Color c = selected ? ( focused - ? wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT) + ? Color(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT)) : lerp(wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW), wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT), subcolumnActivity(j)) ) diff --git a/src/gui/set/cards_panel.cpp b/src/gui/set/cards_panel.cpp index b6b44f12..bfa704a5 100644 --- a/src/gui/set/cards_panel.cpp +++ b/src/gui/set/cards_panel.cpp @@ -44,7 +44,7 @@ CardsPanel::CardsPanel(Window* parent, int id) card_list = new FilteredImageCardList(splitter, ID_CARD_LIST); nodes_panel = new wxPanel(splitter, wxID_ANY); notes = new TextCtrl(nodes_panel, ID_NOTES, true); - collapse_notes = new HoverButton(nodes_panel, ID_COLLAPSE_NOTES, _("btn_collapse"), wxNullColour, false); + collapse_notes = new HoverButton(nodes_panel, ID_COLLAPSE_NOTES, _("btn_collapse"), Color(), false); collapse_notes->SetExtraStyle(wxWS_EX_PROCESS_UI_UPDATES); filter = nullptr; editor->next_in_tab_order = card_list; diff --git a/src/gui/set/stats_panel.cpp b/src/gui/set/stats_panel.cpp index f4d04391..3c3767f9 100644 --- a/src/gui/set/stats_panel.cpp +++ b/src/gui/set/stats_panel.cpp @@ -260,7 +260,7 @@ void StatDimensionList::drawItem(DC& dc, int x, int y, size_t item) { int cx = x + subcolumns[j].offset.x + subcolumns[j].size.x/2; int cy = y + subcolumns[j].offset.y + subcolumns[j].size.y/2; dc.SetPen(*wxTRANSPARENT_PEN); - dc.SetBrush(prefered ? wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT) + dc.SetBrush(prefered ? Color(wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT)) : lerp(wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT),wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW),0.5)); dc.DrawCircle(cx,cy,6); } diff --git a/src/gui/value/color.cpp b/src/gui/value/color.cpp index f5e67a6a..a0a3c6cb 100644 --- a/src/gui/value/color.cpp +++ b/src/gui/value/color.cpp @@ -151,6 +151,6 @@ void ColorValueEditor::change(const Defaultable& c) { addAction(value_action(valueP(), c)); } void ColorValueEditor::changeCustom() { - Color c = wxGetColourFromUser(0, value().value()); - if (c.Ok()) change(c); + wxColour c = wxGetColourFromUser(0, value().value()); + if (c.Ok()) change(Color(c)); } diff --git a/src/render/text/element.cpp b/src/render/text/element.cpp index 8e394819..c230abbf 100644 --- a/src/render/text/element.cpp +++ b/src/render/text/element.cpp @@ -127,9 +127,13 @@ struct TextElementsFromString { else if (is_substr(text, tag_start, _( ":"), tag_start); if (colon < pos - 1 && text.GetChar(colon) == _(':')) { - Color c = parse_color(text.substr(colon+1, pos-colon-2)); - if (!c.Ok()) c = style.font.color; - colors.push_back(c); + auto c = parse_color(text.substr(colon+1, pos-colon-2)); + if (c) { + colors.push_back(*c); + } else { + queue_message(MESSAGE_WARNING, _("Invalid color in tagged string: ") + text.substr(colon + 1, pos - colon - 2)); + colors.push_back(style.font.color); + } } } else if (is_substr(text, tag_start, _("(input,red,green,blue,white); } else { SCRIPT_PARAM(Color, color); diff --git a/src/script/value.cpp b/src/script/value.cpp index 1b8d9ece..6458bdd6 100644 --- a/src/script/value.cpp +++ b/src/script/value.cpp @@ -341,11 +341,11 @@ public: } } Color toColor() const override { - Color c = parse_color(value); - if (!c.Ok()) { + optional c = parse_color(value); + if (!c.has_value()) { throw ScriptErrorConversion(value, typeName(), _TYPE_("color")); } - return c; + return *c; } wxDateTime toDateTime() const override { wxDateTime date; diff --git a/src/util/prec.hpp b/src/util/prec.hpp index 13ecb4e5..1302b298 100644 --- a/src/util/prec.hpp +++ b/src/util/prec.hpp @@ -52,7 +52,6 @@ typedef wxWindow Window; typedef wxBitmap Bitmap; typedef wxImage Image; -typedef wxColour Color; typedef wxDC DC; typedef wxDateTime DateTime; diff --git a/src/util/rotation.cpp b/src/util/rotation.cpp index 5ed13d6c..62dede58 100644 --- a/src/util/rotation.cpp +++ b/src/util/rotation.cpp @@ -182,11 +182,11 @@ RotatedDC::RotatedDC(DC& dc, const Rotation& rotation, RenderQuality quality) // ----------------------------------------------------------------------------- : RotatedDC : Drawing -void RotatedDC::DrawText (const String& text, const RealPoint& pos, int blur_radius, int boldness, double stretch_) { +void RotatedDC::DrawText(const String& text, const RealPoint& pos, int blur_radius, int boldness, double stretch_) { DrawText(text, pos, dc.GetTextForeground(), blur_radius, boldness, stretch_); } -void RotatedDC::DrawText (const String& text, const RealPoint& pos, Color color, int blur_radius, int boldness, double stretch_) { +void RotatedDC::DrawText(const String& text, const RealPoint& pos, Color color, int blur_radius, int boldness, double stretch_) { if (text.empty()) return; if (color.Alpha() == 0) return; if (quality >= QUALITY_AA) {