Use RCSS syntax for color-string conversion, format colors using hexa…

…decimal notation
mikke89 · Feb 3, 2024 · f0654d1 · f0654d1
1 parent ffa9dff
commit f0654d1
Show file tree

Hide file tree

Showing 13 changed files with 98 additions and 75 deletions.
diff --git a/Include/RmlUi/Core/TypeConverter.h b/Include/RmlUi/Core/TypeConverter.h
@@ -79,6 +79,16 @@ class TypeConverter<Unit, String> {
 public:
 	RMLUICORE_API static bool Convert(const Unit& src, String& dest);
 };
+template <>
+class TypeConverter<Colourb, String> {
+public:
+	RMLUICORE_API static bool Convert(const Colourb& src, String& dest);
+};
+template <>
+class TypeConverter<String, Colourb> {
+public:
+	RMLUICORE_API static bool Convert(const String& src, Colourb& dest);
+};
 
 template <>
 class TypeConverter<TransformPtr, TransformPtr> {

diff --git a/Include/RmlUi/Core/TypeConverter.inl b/Include/RmlUi/Core/TypeConverter.inl
@@ -302,7 +302,6 @@ STRING_VECTOR_CONVERTER(Vector3f, float, 3);
 STRING_VECTOR_CONVERTER(Vector4i, int, 4);
 STRING_VECTOR_CONVERTER(Vector4f, float, 4);
 STRING_VECTOR_CONVERTER(Colourf, float, 4);
-STRING_VECTOR_CONVERTER(Colourb, byte, 4);
 
 /////////////////////////////////////////////////
 // To String Converters
@@ -440,7 +439,6 @@ VECTOR_STRING_CONVERTER(Vector3f, float, 3);
 VECTOR_STRING_CONVERTER(Vector4i, int, 4);
 VECTOR_STRING_CONVERTER(Vector4f, float, 4);
 VECTOR_STRING_CONVERTER(Colourf, float, 4);
-VECTOR_STRING_CONVERTER(Colourb, byte, 4);
 
 template <typename SourceType>
 class TypeConverter<SourceType, String> {

diff --git a/Samples/basic/databinding/src/main.cpp b/Samples/basic/databinding/src/main.cpp
@@ -230,7 +230,7 @@ namespace InvadersExample {
 
 		// Register a custom getter for the Colourb type.
 		constructor.RegisterScalar<Rml::Colourb>(
-			[](const Rml::Colourb& color, Rml::Variant& variant) { variant = "rgba(" + Rml::ToString(color) + ')'; });
+			[](const Rml::Colourb& color, Rml::Variant& variant) { variant = Rml::ToString(color); });
 		// Register a transform function for formatting time
 		constructor.RegisterTransformFunc("format_time", [](const Rml::VariantList& arguments) -> Rml::Variant {
 			if (arguments.empty())

diff --git a/Samples/invaders/src/HighScores.h b/Samples/invaders/src/HighScores.h
@@ -64,7 +64,7 @@ class HighScores {
 		int score;
 		int wave;
 
-		Rml::String GetColour() { return "rgba(" + Rml::ToString(colour) + ')'; }
+		Rml::String GetColour() { return Rml::ToString(colour); }
 	};
 	using ScoreList = Rml::Vector<Score>;
 	ScoreList scores;

diff --git a/Samples/luainvaders/src/HighScores.h b/Samples/luainvaders/src/HighScores.h
@@ -64,7 +64,7 @@ class HighScores {
 		int score;
 		int wave;
 
-		Rml::String GetColour() { return "rgba(" + Rml::ToString(colour) + ')'; }
+		Rml::String GetColour() { return Rml::ToString(colour); }
 	};
 	using ScoreList = Rml::Vector<Score>;
 	ScoreList scores;

diff --git a/Source/Core/PropertyDefinition.cpp b/Source/Core/PropertyDefinition.cpp
@@ -156,13 +156,6 @@ bool PropertyDefinition::GetValue(String& value, const Property& property) const
 	}
 	break;
 
-	case Unit::COLOUR:
-	{
-		Colourb colour = property.value.Get<Colourb>();
-		value = CreateString(32, "rgba(%d,%d,%d,%d)", colour.red, colour.green, colour.blue, colour.alpha);
-	}
-	break;
-
 	default: value += ToString(property.unit); break;
 	}
 

diff --git a/Source/Core/PropertyParserColour.cpp b/Source/Core/PropertyParserColour.cpp
@@ -31,37 +31,50 @@
 
 namespace Rml {
 
-PropertyParserColour::PropertyParserColour()
-{
-	html_colours["black"] = Colourb(0, 0, 0);
-	html_colours["silver"] = Colourb(192, 192, 192);
-	html_colours["gray"] = Colourb(128, 128, 128);
-	html_colours["grey"] = Colourb(128, 128, 128);
-	html_colours["white"] = Colourb(255, 255, 255);
-	html_colours["maroon"] = Colourb(128, 0, 0);
-	html_colours["red"] = Colourb(255, 0, 0);
-	html_colours["orange"] = Colourb(255, 165, 0);
-	html_colours["purple"] = Colourb(128, 0, 128);
-	html_colours["fuchsia"] = Colourb(255, 0, 255);
-	html_colours["green"] = Colourb(0, 128, 0);
-	html_colours["lime"] = Colourb(0, 255, 0);
-	html_colours["olive"] = Colourb(128, 128, 0);
-	html_colours["yellow"] = Colourb(255, 255, 0);
-	html_colours["navy"] = Colourb(0, 0, 128);
-	html_colours["blue"] = Colourb(0, 0, 255);
-	html_colours["teal"] = Colourb(0, 128, 128);
-	html_colours["aqua"] = Colourb(0, 255, 255);
-	html_colours["transparent"] = Colourb(0, 0, 0, 0);
-}
+const PropertyParserColour::ColourMap PropertyParserColour::html_colours = {
+	{"black", Colourb(0, 0, 0)},
+	{"silver", Colourb(192, 192, 192)},
+	{"gray", Colourb(128, 128, 128)},
+	{"grey", Colourb(128, 128, 128)},
+	{"white", Colourb(255, 255, 255)},
+	{"maroon", Colourb(128, 0, 0)},
+	{"red", Colourb(255, 0, 0)},
+	{"orange", Colourb(255, 165, 0)},
+	{"purple", Colourb(128, 0, 128)},
+	{"fuchsia", Colourb(255, 0, 255)},
+	{"green", Colourb(0, 128, 0)},
+	{"lime", Colourb(0, 255, 0)},
+	{"olive", Colourb(128, 128, 0)},
+	{"yellow", Colourb(255, 255, 0)},
+	{"navy", Colourb(0, 0, 128)},
+	{"blue", Colourb(0, 0, 255)},
+	{"teal", Colourb(0, 128, 128)},
+	{"aqua", Colourb(0, 255, 255)},
+	{"transparent", Colourb(0, 0, 0, 0)},
+};
+
+PropertyParserColour::PropertyParserColour() {}
 
 PropertyParserColour::~PropertyParserColour() {}
 
 bool PropertyParserColour::ParseValue(Property& property, const String& value, const ParameterMap& /*parameters*/) const
+{
+	Colourb colour;
+	if (!ParseColour(colour, value))
+		return false;
+
+	property.value = Variant(colour);
+	property.unit = Unit::COLOUR;
+
+	return true;
+}
+
+bool PropertyParserColour::ParseColour(Colourb& colour, const String& value)
 {
 	if (value.empty())
 		return false;
 
-	Colourb colour;
+	colour = {};
 
 	// Check for a hex colour.
 	if (value[0] == '#')
@@ -155,9 +168,6 @@ bool PropertyParserColour::ParseValue(Property& property, const String& value, c
 			colour = (*iterator).second;
 	}
 
-	property.value = Variant(colour);
-	property.unit = Unit::COLOUR;
-
 	return true;
 }
 

diff --git a/Source/Core/PropertyParserColour.h b/Source/Core/PropertyParserColour.h
@@ -52,9 +52,12 @@ class PropertyParserColour : public PropertyParser {
 	/// @return True if the value was parsed successfully, false otherwise.
 	bool ParseValue(Property& property, const String& value, const ParameterMap& parameters) const override;
 
+	/// Parse a colour directly.
+	static bool ParseColour(Colourb& colour, const String& value);
+
 private:
 	using ColourMap = UnorderedMap<String, Colourb>;
-	ColourMap html_colours;
+	static const ColourMap html_colours;
 };
 
 } // namespace Rml

diff --git a/Source/Core/TypeConverter.cpp b/Source/Core/TypeConverter.cpp
@@ -37,6 +37,7 @@
 #include "../../Include/RmlUi/Core/StyleSheetTypes.h"
 #include "../../Include/RmlUi/Core/Transform.h"
 #include "../../Include/RmlUi/Core/TransformPrimitive.h"
+#include "PropertyParserColour.h"
 #include "PropertyParserDecorator.h"
 #include "TransformUtilities.h"
 
@@ -247,8 +248,7 @@ bool TypeConverter<ColorStopList, String>::Convert(const ColorStopList& src, Str
 	for (size_t i = 0; i < src.size(); i++)
 	{
 		const ColorStop& stop = src[i];
-		const Colourb color = stop.color.ToNonPremultiplied();
-		dest += CreateString(32, "rgba(%d,%d,%d,%d)", color.red, color.green, color.blue, color.alpha);
+		dest += ToString(stop.color.ToNonPremultiplied());
 
 		if (Any(stop.position.unit & Unit::NUMBER_LENGTH_PERCENT))
 			dest += " " + ToString(stop.position.number) + ToString(stop.position.unit);
@@ -281,7 +281,7 @@ bool TypeConverter<BoxShadowList, String>::Convert(const BoxShadowList& src, Str
 		if (shadow.inset)
 			temp += " inset";
 
-		dest += "rgba(" + ToString(shadow.color.ToNonPremultiplied()) + ')' + temp;
+		dest += ToString(shadow.color.ToNonPremultiplied()) + temp;
 
 		if (i < src.size() - 1)
 		{
@@ -292,4 +292,17 @@ bool TypeConverter<BoxShadowList, String>::Convert(const BoxShadowList& src, Str
 	return true;
 }
 
+bool TypeConverter<Colourb, String>::Convert(const Colourb& src, String& dest)
+{
+	if (src.alpha == 255)
+		return FormatString(dest, 32, "#%02hhx%02hhx%02hhx", src.red, src.green, src.blue) > 0;
+	else
+		return FormatString(dest, 32, "#%02hhx%02hhx%02hhx%02hhx", src.red, src.green, src.blue, src.alpha) > 0;
+}
+
+bool TypeConverter<String, Colourb>::Convert(const String& src, Colourb& dest)
+{
+	return PropertyParserColour::ParseColour(dest, src);
+}
+
 } // namespace Rml
diff --git a/Tests/Source/UnitTests/Animation.cpp b/Tests/Source/UnitTests/Animation.cpp
@@ -89,7 +89,7 @@ TEST_CASE("animation.decorator")
 			"horizontal-gradient(transparent transparent)",
 			"horizontal-gradient(white white)",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 		{
 			"",
@@ -98,7 +98,7 @@ TEST_CASE("animation.decorator")
 			"horizontal-gradient(transparent transparent) border-box",
 			"horizontal-gradient(white white) border-box",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63)) border-box",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f) border-box",
 		},
 		{
 			"",
@@ -107,7 +107,7 @@ TEST_CASE("animation.decorator")
 			"none",
 			"horizontal-gradient(transparent transparent)",
 
-			"horizontal-gradient(horizontal rgba(220,220,220,191) rgba(220,220,220,191))",
+			"horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf)",
 		},
 		{
 			"",
@@ -116,8 +116,7 @@ TEST_CASE("animation.decorator")
 			"none",
 			"horizontal-gradient(transparent transparent), horizontal-gradient(transparent transparent)",
 
-			"horizontal-gradient(horizontal rgba(220,220,220,191) rgba(220,220,220,191)), horizontal-gradient(horizontal rgba(220,220,220,191) "
-			"rgba(220,220,220,191))",
+			"horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf), horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf)",
 		},
 		{
 			"",
@@ -126,8 +125,7 @@ TEST_CASE("animation.decorator")
 			"horizontal-gradient(transparent transparent), horizontal-gradient(transparent transparent)",
 			"none",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63)), horizontal-gradient(horizontal rgba(127,127,127,63) "
-			"rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f), horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 
 		/// Only rule declaration
@@ -138,7 +136,7 @@ TEST_CASE("animation.decorator")
 			"from_rule",
 			"to_rule",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 		{
 			"",
@@ -147,7 +145,7 @@ TEST_CASE("animation.decorator")
 			"from_rule",
 			"to_rule",
 
-			"horizontal-gradient(horizontal rgba(220,220,220,191) rgba(220,220,220,191))",
+			"horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf)",
 		},
 		{
 			"start-color: transparent; stop-color: transparent;",
@@ -156,7 +154,7 @@ TEST_CASE("animation.decorator")
 			"from_rule",
 			"to_rule",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 
 		/// Mix rule and standard declaration
@@ -167,7 +165,7 @@ TEST_CASE("animation.decorator")
 			"from_rule",
 			"horizontal-gradient(white white)",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 		{
 			"",
@@ -176,7 +174,7 @@ TEST_CASE("animation.decorator")
 			"none",
 			"to_rule",
 
-			"horizontal-gradient(horizontal rgba(220,220,220,191) rgba(220,220,220,191))",
+			"horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf)",
 		},
 		{
 			"start-color: transparent; stop-color: transparent;",
@@ -185,7 +183,7 @@ TEST_CASE("animation.decorator")
 			"from_rule",
 			"none",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 		{
 			"",
@@ -194,8 +192,7 @@ TEST_CASE("animation.decorator")
 			"from_rule, to_rule",
 			"horizontal-gradient(transparent transparent), horizontal-gradient(transparent transparent)",
 
-			"horizontal-gradient(horizontal rgba(220,220,220,191) rgba(220,220,220,191)), horizontal-gradient(horizontal rgba(220,220,220,191) "
-			"rgba(220,220,220,191))",
+			"horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf), horizontal-gradient(horizontal #dcdcdcbf #dcdcdcbf)",
 		},
 		{
 			"",
@@ -204,8 +201,7 @@ TEST_CASE("animation.decorator")
 			"horizontal-gradient(transparent transparent), horizontal-gradient(transparent transparent)",
 			"from_rule, to_rule",
 
-			"horizontal-gradient(horizontal rgba(127,127,127,63) rgba(127,127,127,63)), horizontal-gradient(horizontal rgba(127,127,127,63) "
-			"rgba(127,127,127,63))",
+			"horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f), horizontal-gradient(horizontal #7f7f7f3f #7f7f7f3f)",
 		},
 	};
 
@@ -305,7 +301,7 @@ TEST_CASE("animation.filter")
 		{
 			"drop-shadow(#000 30px 20px 0px)",
 			"drop-shadow(#f00 30px 20px 4px)", // colors interpolated in linear space
-			"drop-shadow(rgba(127,0,0,255) 30px 20px 1px)",
+			"drop-shadow(#7f0000 30px 20px 1px)",
 		},
 		{
 			"opacity(0) brightness(2)",

diff --git a/Tests/Source/UnitTests/DataExpression.cpp b/Tests/Source/UnitTests/DataExpression.cpp
@@ -121,7 +121,7 @@ TEST_CASE("Data expressions")
 
 	CHECK(TestExpression("'a' | to_upper") == "A");
 	CHECK(TestExpression("!!10 - 1 ? 'hello' : 'world' | to_upper") == "WORLD");
-	CHECK(TestExpression("(color_name) + (': rgba(' + color_value + ')')") == "color: rgba(180, 100, 255, 255)");
+	CHECK(TestExpression("(color_name) + (': ' + color_value)") == "color: #b464ff");
 	CHECK(TestExpression("'hello world' | to_upper | concatenate(5 + 12 == 17 ? 'yes' : 'no', 9*2)") == "HELLO WORLD,yes,18");
 	CHECK(TestExpression("true == false") == "0");
 	CHECK(TestExpression("true != false") == "1");

diff --git a/Tests/Source/UnitTests/Element.cpp b/Tests/Source/UnitTests/Element.cpp
@@ -258,21 +258,21 @@ TEST_CASE("Element")
 	SUBCASE("CloneManual")
 	{
 		Element* element = document->GetFirstChild();
-		REQUIRE(element->GetProperty<String>("background-color") == "255, 0, 0, 255");
-		CHECK(element->Clone()->GetProperty<String>("background-color") == "255, 0, 0, 255");
+		REQUIRE(element->GetProperty<String>("background-color") == "#ff0000");
+		CHECK(element->Clone()->GetProperty<String>("background-color") == "#ff0000");
 
 		element->SetProperty("background-color", "#0f0");
-		CHECK(element->Clone()->GetProperty<String>("background-color") == "0, 255, 0, 255");
+		CHECK(element->Clone()->GetProperty<String>("background-color") == "#00ff00");
 
 		element->RemoveProperty("background-color");
 		Element* clone = document->AppendChild(element->Clone());
 		context->Update();
-		CHECK(clone->GetProperty<String>("background-color") == "255, 255, 255, 255");
+		CHECK(clone->GetProperty<String>("background-color") == "#ffffff");
 
 		element->SetClass("blue", true);
 		clone = document->AppendChild(element->Clone());
 		context->Update();
-		CHECK(clone->GetProperty<String>("background-color") == "0, 0, 255, 255");
+		CHECK(clone->GetProperty<String>("background-color") == "#0000ff");
 	}
 
 	SUBCASE("SetInnerRML")