Index: source/graphics/Color.h =================================================================== --- source/graphics/Color.h +++ source/graphics/Color.h @@ -53,6 +53,7 @@ /** * Try to parse @p Value as a color. Returns true on success, false otherwise. + * Leaves the color unchanged if it failed. * @param value Should be "r g b" or "r g b a" where each value is an integer in [0,255]. * @param defaultAlpha The alpha value that is used if the format of @p Value is "r g b". */ Index: source/graphics/Color.cpp =================================================================== --- source/graphics/Color.cpp +++ source/graphics/Color.cpp @@ -92,6 +92,9 @@ } } +/** + * Important: This function does not modify the value if parsing fails. + */ bool CColor::ParseString(const CStr8& value, int defaultAlpha) { const size_t NUM_VALS = 4; Index: source/gui/CButton.cpp =================================================================== --- source/gui/CButton.cpp +++ source/gui/CButton.cpp @@ -104,6 +104,5 @@ DrawButton(m_CachedActualSize, bz, sprite, sprite_over, sprite_pressed, sprite_disabled, cell_id); - CGUIColor color = ChooseColor(); - DrawText(0, color, m_TextPos, bz+0.1f); + DrawText(0, ChooseColor(), m_TextPos, bz + 0.1f); } Index: source/gui/CChart.cpp =================================================================== --- source/gui/CChart.cpp +++ source/gui/CChart.cpp @@ -108,9 +108,7 @@ ADD(m_CachedActualSize.left, m_CachedActualSize.top); ADD(rect.left, rect.top - m_AxisWidth); #undef ADD - CGUIColor axis_color(0.5f, 0.5f, 0.5f, 1.f); - GUI::GetSetting(this, "axis_color", axis_color); - DrawTriangleStrip(shader, axis_color, vertices); + DrawTriangleStrip(shader, GUI::GetSetting(this, "axis_color"), vertices); } void CChart::Draw() Index: source/gui/CDropDown.cpp =================================================================== --- source/gui/CDropDown.cpp +++ source/gui/CDropDown.cpp @@ -476,7 +476,6 @@ GUI::GetSetting(this, "button_width", button_width); int cell_id, selected = 0; - CGUIColor color; bool enabled; GUI::GetSetting(this, "enabled", enabled); @@ -486,7 +485,6 @@ GUI::GetSetting(this, "cell_id", cell_id); GUI::GetSetting(this, "selected", selected); - GUI::GetSetting(this, enabled ? "textcolor_selected" : "textcolor_disabled", color); m_pGUI->DrawSprite(sprite, cell_id, bz, m_CachedActualSize); @@ -519,6 +517,8 @@ CRect cliparea(m_CachedActualSize.left, m_CachedActualSize.top, m_CachedActualSize.right-button_width, m_CachedActualSize.bottom); + const CGUIColor& color = GUI::GetSetting(this, enabled ? "textcolor_selected" : "textcolor_disabled"); + CPos pos(m_CachedActualSize.left, m_CachedActualSize.top); DrawText(selected, color, pos, bz+0.1f, cliparea); } Index: source/gui/CGUI.h =================================================================== --- source/gui/CGUI.h +++ source/gui/CGUI.h @@ -235,10 +235,14 @@ const SGUIStyle& GetStyle(const CStr& name) const { return m_Styles.at(name); } /** - * Get pre-defined color (if it exists) - * Returns false if it fails. + * Check if a predefined color of that name exists. */ - bool GetPreDefinedColor(const CStr& name, CGUIColor& Output) const; + bool HasPreDefinedColor(const CStr& name) const { return (m_PreDefinedColors.count(name) != 0); } + + /** + * Resolve the predefined color if it exists, otherwise throws an exception. + */ + const CGUIColor& GetPreDefinedColor(const CStr& name) const { return m_PreDefinedColors.at(name); } shared_ptr GetScriptInterface() { return m_ScriptInterface; }; JS::Value GetGlobalObject() { return m_ScriptInterface->GetGlobalObject(); }; @@ -560,14 +564,6 @@ // Tooltip GUITooltip m_Tooltip; - /** - * This is a bank of custom colors, it is simply a look up table that - * will return a color object when someone inputs the name of that - * color. Of course the colors have to be declared in XML, there are - * no hard-coded values. - */ - std::map m_PreDefinedColors; - //@} //-------------------------------------------------------- /** @name Objects */ @@ -626,6 +622,9 @@ // rule out unintentional modification and copy, especially during Draw calls. //-------------------------------------------------------- + // Colors + std::map m_PreDefinedColors; + // Sprites std::map m_Sprites; Index: source/gui/CGUI.cpp =================================================================== --- source/gui/CGUI.cpp +++ source/gui/CGUI.cpp @@ -484,16 +484,6 @@ return &it->second; } -bool CGUI::GetPreDefinedColor(const CStr& name, CGUIColor& Output) const -{ - std::map::const_iterator cit = m_PreDefinedColors.find(name); - if (cit == m_PreDefinedColors.end()) - return false; - - Output = cit->second; - return true; -} - /** * @callgraph */ @@ -1120,19 +1110,13 @@ } else if (attr_name == "backcolor") { - CGUIColor color; - if (!GUI::ParseString(this, attr_value, color)) + if (!GUI::ParseString(this, attr_value, Image->m_BackColor)) LOGERROR("GUI: Error parsing '%s' (\"%s\")", attr_name, utf8_from_wstring(attr_value)); - else - Image->m_BackColor = color; } else if (attr_name == "bordercolor") { - CGUIColor color; - if (!GUI::ParseString(this, attr_value, color)) + if (!GUI::ParseString(this, attr_value, Image->m_BorderColor)) LOGERROR("GUI: Error parsing '%s' (\"%s\")", attr_name, utf8_from_wstring(attr_value)); - else - Image->m_BorderColor = color; } else if (attr_name == "border") { @@ -1175,10 +1159,8 @@ if (attr_name == "add_color") { - CGUIColor color; - if (!color.ParseString(this, attr.Value, 0)) + if (!effects.m_AddColor.ParseString(this, attr.Value, 0)) LOGERROR("GUI: Error parsing '%s' (\"%s\")", attr_name, attr.Value); - else effects.m_AddColor = color; } else if (attr_name == "grayscale") effects.m_Greyscale = true; @@ -1353,8 +1335,6 @@ void CGUI::Xeromyces_ReadColor(XMBElement Element, CXeromyces* pFile) { XMBAttributeList attributes = Element.GetAttributes(); - - CGUIColor color; CStr name = attributes.GetNamedItem(pFile->GetAttributeID("name")); // Try parsing value @@ -1362,12 +1342,15 @@ if (value.empty()) return; - // Try setting color to value - if (!color.ParseString(this, value)) + CColor color; + if (color.ParseString(value)) { - LOGERROR("GUI: Unable to create custom color '%s'. Invalid color syntax.", name.c_str()); - return; + m_PreDefinedColors.erase(name); + m_PreDefinedColors.emplace( + std::piecewise_construct, + std::forward_as_tuple(name), + std::forward_as_tuple(color.r, color.g, color.b, color.a)); } - - m_PreDefinedColors[name] = color; + else + LOGERROR("GUI: Unable to create custom color '%s'. Invalid color syntax.", name.c_str()); } Index: source/gui/CGUIColor.h =================================================================== --- source/gui/CGUIColor.h +++ source/gui/CGUIColor.h @@ -28,6 +28,12 @@ */ struct CGUIColor : CColor { + // Take advantage of compiler warnings if unintentionally copying this + NONCOPYABLE(CGUIColor); + + // Defines move semantics so that the structs using the class can use it. + MOVABLE(CGUIColor); + CGUIColor() : CColor() {} CGUIColor(float r, float g, float b, float a) : CColor(r, g, b, a) {} Index: source/gui/CGUIColor.cpp =================================================================== --- source/gui/CGUIColor.cpp +++ source/gui/CGUIColor.cpp @@ -23,5 +23,17 @@ bool CGUIColor::ParseString(const CGUI* pGUI, const CStr& value, int defaultAlpha) { - return (pGUI != nullptr && pGUI->GetPreDefinedColor(value, *this)) || CColor::ParseString(value, defaultAlpha); + if (pGUI != nullptr && pGUI->HasPreDefinedColor(value)) + { + const CGUIColor& color = pGUI->GetPreDefinedColor(value); + + // Explicit copy assignment + r = color.r; + g = color.g; + b = color.b; + a = color.a; + return true; + } + + return CColor::ParseString(value, defaultAlpha); } Index: source/gui/CGUIText.cpp =================================================================== --- source/gui/CGUIText.cpp +++ source/gui/CGUIText.cpp @@ -452,9 +452,7 @@ if (tc.m_pSpriteCall) continue; - CGUIColor color = tc.m_UseCustomColor ? tc.m_Color : DefaultColor; - - textRenderer.Color(color); + textRenderer.Color(tc.m_UseCustomColor ? tc.m_Color : DefaultColor); textRenderer.Font(tc.m_Font); textRenderer.Put(floorf(pos.x + tc.m_Pos.x), floorf(pos.y + tc.m_Pos.y), &tc.m_String); } Index: source/gui/CInput.cpp =================================================================== --- source/gui/CInput.cpp +++ source/gui/CInput.cpp @@ -1176,10 +1176,9 @@ IGUIScrollBarOwner::Draw(); CStrW font_name_w; - CGUIColor color, color_selected; GUI::GetSetting(this, "font", font_name_w); - GUI::GetSetting(this, "textcolor", color); - GUI::GetSetting(this, "textcolor_selected", color_selected); + const CGUIColor& color = GUI::GetSetting(this, "textcolor"); + const CGUIColor& color_selected = GUI::GetSetting(this, "textcolor_selected"); CStrIntern font_name(font_name_w.ToUTF8()); const CStrW& pCaption = GUI::GetSetting(this, "caption"); Index: source/gui/CList.cpp =================================================================== --- source/gui/CList.cpp +++ source/gui/CList.cpp @@ -369,10 +369,8 @@ } } - CGUIColor color; - GUI::GetSetting(this, _textcolor, color); - const CGUIList& pList = GUI::GetSetting(this, "list"); + const CGUIColor& color = GUI::GetSetting(this, _textcolor); for (size_t i = 0; i < pList.m_Items.size(); ++i) { @@ -394,7 +392,7 @@ cliparea.left = GetScrollBar(0).GetOuterRect().right; } - DrawText(i, color, rect.TopLeft() - CPos(0.f, scroll - m_ItemsYPositions[i]), bz+0.1f, cliparea); + DrawText(i, color, rect.TopLeft() - CPos(0.f, scroll - m_ItemsYPositions[i]), bz + 0.1f, cliparea); } } } Index: source/gui/COList.cpp =================================================================== --- source/gui/COList.cpp +++ source/gui/COList.cpp @@ -223,11 +223,8 @@ if (attr_name == "color") { - CGUIColor color; - if (!GUI::ParseString(m_pGUI, attr_value.FromUTF8(), color)) + if (!GUI::ParseString(m_pGUI, attr_value.FromUTF8(), column.m_TextColor)) LOGERROR("GUI: Error parsing '%s' (\"%s\")", attr_name.c_str(), attr_value.c_str()); - else - column.m_TextColor = color; } else if (attr_name == "id") { @@ -375,8 +372,7 @@ int selectedColumnOrder; GUI::GetSetting(this, "selected_column_order", selectedColumnOrder); - CGUIColor color; - GUI::GetSetting(this, _textcolor, color); + const CGUIColor& color = GUI::GetSetting(this, _textcolor); float xpos = 0; for (size_t col = 0; col < m_Columns.size(); ++col) Index: source/gui/CText.cpp =================================================================== --- source/gui/CText.cpp +++ source/gui/CText.cpp @@ -230,13 +230,12 @@ bool enabled; GUI::GetSetting(this, "enabled", enabled); - CGUIColor color; - GUI::GetSetting(this, enabled ? "textcolor" : "textcolor_disabled", color); + const CGUIColor& color = GUI::GetSetting(this, enabled ? "textcolor" : "textcolor_disabled"); if (scrollbar) - DrawText(0, color, m_CachedActualSize.TopLeft() - CPos(0.f, scroll), bz+0.1f, cliparea); + DrawText(0, color, m_CachedActualSize.TopLeft() - CPos(0.f, scroll), bz + 0.1f, cliparea); else - DrawText(0, color, m_TextPos, bz+0.1f, cliparea); + DrawText(0, color, m_TextPos, bz + 0.1f, cliparea); } bool CText::MouseOverIcon() Index: source/gui/CTooltip.cpp =================================================================== --- source/gui/CTooltip.cpp +++ source/gui/CTooltip.cpp @@ -160,8 +160,6 @@ m_pGUI->DrawSprite(sprite, 0, z, m_CachedActualSize); - CGUIColor color; - GUI::GetSetting(this, "textcolor", color); - + const CGUIColor& color = GUI::GetSetting(this, "textcolor"); DrawText(0, color, m_CachedActualSize.TopLeft(), z+0.1f); } Index: source/gui/GUIRenderer.h =================================================================== --- source/gui/GUIRenderer.h +++ source/gui/GUIRenderer.h @@ -53,8 +53,8 @@ CRect m_Vertices; float m_DeltaZ; - CGUIColor m_BorderColor; // == CGUIColor() for no border - CGUIColor m_BackColor; + CGUIColor* m_BorderColor; // == nullptr for no border + CGUIColor* m_BackColor; }; class DrawCalls : public std::vector Index: source/gui/GUIRenderer.cpp =================================================================== --- source/gui/GUIRenderer.cpp +++ source/gui/GUIRenderer.cpp @@ -141,16 +141,9 @@ if (SpriteName.Find("color:") != -1) { CStrW value = wstring_from_utf8(SpriteName.AfterLast("color:").BeforeFirst(":")); - CGUIColor color; - - // Check color is valid - if (!GUI::ParseString(pGUI, value, color)) - { - LOGERROR("GUI: Error parsing sprite 'color' (\"%s\")", utf8_from_wstring(value)); - return; - } SGUIImage* Image = new SGUIImage; + CGUIColor* color; // If we are using a mask, this is an effect. // Otherwise we can fallback to the "back color" attribute @@ -159,10 +152,17 @@ { Image->m_TextureName = TextureName; Image->m_Effects = std::make_shared(); - Image->m_Effects->m_SolidColor = color; + color = &Image->m_Effects->m_SolidColor; } else - Image->m_BackColor = color; + color = &Image->m_BackColor; + + // Check color is valid + if (!GUI::ParseString(pGUI, value, *color)) + { + LOGERROR("GUI: Error parsing sprite 'color' (\"%s\")", utf8_from_wstring(value)); + return; + } CClientArea ca(CRect(0, 0, 0, 0), CRect(0, 0, 100, 100)); Image->m_Size = ca; @@ -231,8 +231,8 @@ Call.m_EnableBlending = !(fabs((*cit)->m_BackColor.a - 1.0f) < 0.0000001f); } - Call.m_BackColor = (*cit)->m_BackColor; - Call.m_BorderColor = (*cit)->m_Border ? (*cit)->m_BorderColor : CGUIColor(); + Call.m_BackColor = &(*cit)->m_BackColor; + Call.m_BorderColor = (*cit)->m_Border ? &(*cit)->m_BorderColor : nullptr; Call.m_DeltaZ = (*cit)->m_DeltaZ; if (!Call.m_HasTexture) @@ -411,7 +411,7 @@ } else { - shader->Uniform(str_color, cit->m_BackColor); + shader->Uniform(str_color, *cit->m_BackColor); if (cit->m_EnableBlending) { @@ -439,9 +439,9 @@ shader->VertexPointer(3, GL_FLOAT, 3*sizeof(float), &data[0]); glDrawArrays(GL_TRIANGLES, 0, 6); - if (cit->m_BorderColor != CGUIColor()) + if (cit->m_BorderColor != nullptr) { - shader->Uniform(str_color, cit->m_BorderColor); + shader->Uniform(str_color, *cit->m_BorderColor); data.clear(); ADD(Verts.left + 0.5f, Verts.top + 0.5f, Z + cit->m_DeltaZ); Index: source/gui/GUItypes.h =================================================================== --- source/gui/GUItypes.h +++ source/gui/GUItypes.h @@ -30,7 +30,6 @@ TYPE(i32) TYPE(u32) TYPE(float) -TYPE(CGUIColor) TYPE(CStr) TYPE(CStrW) TYPE(EAlign) @@ -40,6 +39,7 @@ #ifndef GUITYPE_IGNORE_NONCOPYABLE TYPE(CClientArea) +TYPE(CGUIColor) TYPE(CGUIList) TYPE(CGUISeries) TYPE(CGUISpriteInstance) Index: source/gui/IGUIButtonBehavior.h =================================================================== --- source/gui/IGUIButtonBehavior.h +++ source/gui/IGUIButtonBehavior.h @@ -82,7 +82,7 @@ * textcolor_pressed -- pressed * textcolor_over -- hovered */ - CGUIColor ChooseColor(); + const CGUIColor& ChooseColor(); protected: Index: source/gui/IGUIButtonBehavior.cpp =================================================================== --- source/gui/IGUIButtonBehavior.cpp +++ source/gui/IGUIButtonBehavior.cpp @@ -132,36 +132,26 @@ } } -CGUIColor IGUIButtonBehavior::ChooseColor() +const CGUIColor& IGUIButtonBehavior::ChooseColor() { - CGUIColor color, color2; - // Yes, the object must possess these settings. They are standard - GUI::GetSetting(this, "textcolor", color); + const CGUIColor& color = GUI::GetSetting(this, "textcolor"); bool enabled; GUI::GetSetting(this, "enabled", enabled); if (!enabled) - { - GUI::GetSetting(this, "textcolor_disabled", color2); - return color2 || color; - } - else if (m_MouseHovering) + return GUI::GetSetting(this, "textcolor_disabled") || color; + + if (m_MouseHovering) { if (m_Pressed) - { - GUI::GetSetting(this, "textcolor_pressed", color2); - return color2 || color; - } + return GUI::GetSetting(this, "textcolor_pressed") || color; else - { - GUI::GetSetting(this, "textcolor_over", color2); - return color2 || color; - } + return GUI::GetSetting(this, "textcolor_over") || color; } - else - return color; + + return color; } void IGUIButtonBehavior::DrawButton(const CRect& rect, const float& z, CGUISpriteInstance& sprite, CGUISpriteInstance& sprite_over, CGUISpriteInstance& sprite_pressed, CGUISpriteInstance& sprite_disabled, int cell_id)