Browse Source

Fix string conversion of ColorStopList and BoxShadowList, fixes #590

Michael Ragazzon 1 year ago
parent
commit
ff651070d1
3 changed files with 75 additions and 9 deletions
  1. 15 2
      Include/RmlUi/Core/Colour.h
  2. 4 3
      Source/Core/TypeConverter.cpp
  3. 56 4
      Tests/Source/UnitTests/Properties.cpp

+ 15 - 2
Include/RmlUi/Core/Colour.h

@@ -106,7 +106,7 @@ public:
 		typename = typename std::enable_if_t<!IsPremultiplied::value && std::is_same<ColourType, byte>::value>>
 	inline Colour<ColourType, AlphaDefault, true> ToPremultiplied() const
 	{
-		return Colour<ColourType, AlphaDefault, true>{
+		return {
 			ColourType((red * alpha) / 255),
 			ColourType((green * alpha) / 255),
 			ColourType((blue * alpha) / 255),
@@ -119,7 +119,7 @@ public:
 	inline Colour<ColourType, AlphaDefault, true> ToPremultiplied(float opacity) const
 	{
 		const float new_alpha = alpha * opacity;
-		return Colour<ColourType, AlphaDefault, true>{
+		return {
 			ColourType(red * (new_alpha / 255.f)),
 			ColourType(green * (new_alpha / 255.f)),
 			ColourType(blue * (new_alpha / 255.f)),
@@ -127,6 +127,19 @@ public:
 		};
 	}
 
+	// Convert color to non-premultiplied alpha.
+	template <typename IsPremultiplied = std::integral_constant<bool, PremultipliedAlpha>,
+		typename = typename std::enable_if_t<IsPremultiplied::value && std::is_same<ColourType, byte>::value>>
+	inline Colour<ColourType, AlphaDefault, false> ToNonPremultiplied() const
+	{
+		return {
+			ColourType(alpha > 0 ? (red * 255) / alpha : 0),
+			ColourType(alpha > 0 ? (green * 255) / alpha : 0),
+			ColourType(alpha > 0 ? (blue * 255) / alpha : 0),
+			ColourType(alpha),
+		};
+	}
+
 	ColourType red, green, blue, alpha;
 
 #if defined(RMLUI_COLOUR_USER_EXTRA)

+ 4 - 3
Source/Core/TypeConverter.cpp

@@ -46,7 +46,7 @@ bool TypeConverter<Unit, String>::Convert(const Unit& src, String& dest)
 {
 	switch (src)
 	{
-	// clang-format off
+		// clang-format off
 	case Unit::NUMBER:  dest = "";    return true;
 	case Unit::PERCENT: dest = "%";   return true;
 
@@ -247,7 +247,8 @@ bool TypeConverter<ColorStopList, String>::Convert(const ColorStopList& src, Str
 	for (size_t i = 0; i < src.size(); i++)
 	{
 		const ColorStop& stop = src[i];
-		dest += ToString(stop.color);
+		const Colourb color = stop.color.ToNonPremultiplied();
+		dest += CreateString(32, "rgba(%d,%d,%d,%d)", color.red, color.green, color.blue, color.alpha);
 
 		if (Any(stop.position.unit & Unit::NUMBER_LENGTH_PERCENT))
 			dest += " " + ToString(stop.position.number) + ToString(stop.position.unit);
@@ -280,7 +281,7 @@ bool TypeConverter<BoxShadowList, String>::Convert(const BoxShadowList& src, Str
 		if (shadow.inset)
 			temp += " inset";
 
-		dest += "rgba(" + ToString(shadow.color) + ')' + temp;
+		dest += "rgba(" + ToString(shadow.color.ToNonPremultiplied()) + ')' + temp;
 
 		if (i < src.size() - 1)
 		{

+ 56 - 4
Tests/Source/UnitTests/Properties.cpp

@@ -32,6 +32,8 @@
 #include <RmlUi/Core/DecorationTypes.h>
 #include <RmlUi/Core/Element.h>
 #include <RmlUi/Core/ElementDocument.h>
+#include <RmlUi/Core/PropertyDictionary.h>
+#include <RmlUi/Core/StyleSheetSpecification.h>
 #include <RmlUi/Core/StyleSheetTypes.h>
 #include <doctest.h>
 
@@ -95,7 +97,7 @@ TEST_CASE("Properties")
 
 	SUBCASE("gradient")
 	{
-		auto ParseGradient = [&](const String& value) -> ColorStopList {
+		auto ParseGradient = [&](const String& value) -> Property {
 			document->SetProperty("decorator", "linear-gradient(" + value + ")");
 			auto decorators = document->GetProperty<DecoratorsPtr>("decorator");
 			if (!decorators || decorators->list.size() != 1)
@@ -103,19 +105,21 @@ TEST_CASE("Properties")
 			for (auto& id_property : decorators->list.front().properties.GetProperties())
 			{
 				if (id_property.second.unit == Unit::COLORSTOPLIST)
-					return id_property.second.Get<ColorStopList>();
+					return id_property.second;
 			}
 			return {};
 		};
 
 		struct GradientTestCase {
 			String value;
+			String expected_parsed_string;
 			ColorStopList expected_color_stops;
 		};
 
 		GradientTestCase test_cases[] = {
 			{
 				"red, blue",
+				"rgba(255,0,0,255), rgba(0,0,255,255)",
 				{
 					ColorStop{ColourbPremultiplied(255, 0, 0), NumericValue{}},
 					ColorStop{ColourbPremultiplied(0, 0, 255), NumericValue{}},
@@ -123,6 +127,7 @@ TEST_CASE("Properties")
 			},
 			{
 				"red 5px, blue 50%",
+				"rgba(255,0,0,255) 5px, rgba(0,0,255,255) 50%",
 				{
 					ColorStop{ColourbPremultiplied(255, 0, 0), NumericValue{5.f, Unit::PX}},
 					ColorStop{ColourbPremultiplied(0, 0, 255), NumericValue{50.f, Unit::PERCENT}},
@@ -130,6 +135,7 @@ TEST_CASE("Properties")
 			},
 			{
 				"red, #00f 50%, rgba(0, 255,0, 150) 10dp",
+				"rgba(255,0,0,255), rgba(0,0,255,255) 50%, rgba(0,255,0,150) 10dp",
 				{
 					ColorStop{ColourbPremultiplied(255, 0, 0), NumericValue{}},
 					ColorStop{ColourbPremultiplied(0, 0, 255), NumericValue{50.f, Unit::PERCENT}},
@@ -138,6 +144,7 @@ TEST_CASE("Properties")
 			},
 			{
 				"red 50px 20%, blue 10in",
+				"rgba(255,0,0,255) 50px, rgba(255,0,0,255) 20%, rgba(0,0,255,255) 10in",
 				{
 					ColorStop{ColourbPremultiplied(255, 0, 0), NumericValue{50.f, Unit::PX}},
 					ColorStop{ColourbPremultiplied(255, 0, 0), NumericValue{20.f, Unit::PERCENT}},
@@ -148,10 +155,55 @@ TEST_CASE("Properties")
 
 		for (const GradientTestCase& test_case : test_cases)
 		{
-			const ColorStopList result = ParseGradient(test_case.value);
-			CHECK(result == test_case.expected_color_stops);
+			const Property result = ParseGradient(test_case.value);
+			CHECK(result.ToString() == test_case.expected_parsed_string);
+			CHECK(result.Get<ColorStopList>() == test_case.expected_color_stops);
 		}
 	}
 
 	Rml::Shutdown();
 }
+
+TEST_CASE("Property.ToString")
+{
+	TestsSystemInterface system_interface;
+	TestsRenderInterface render_interface;
+	SetRenderInterface(&render_interface);
+	SetSystemInterface(&system_interface);
+
+	Rml::Initialise();
+
+	CHECK(Property(5.2f, Unit::CM).ToString() == "5.2cm");
+	CHECK(Property(150, Unit::PERCENT).ToString() == "150%");
+	CHECK(Property(Colourb{170, 187, 204, 255}, Unit::COLOUR).ToString() == "170, 187, 204, 255");
+
+	auto ParsedValue = [](const String& name, const String& value) -> String {
+		PropertyDictionary properties;
+		StyleSheetSpecification::ParsePropertyDeclaration(properties, name, value);
+		REQUIRE(properties.GetNumProperties() == 1);
+		return properties.GetProperties().begin()->second.ToString();
+	};
+
+	CHECK(ParsedValue("width", "10px") == "10px");
+	CHECK(ParsedValue("width", "10.00em") == "10em");
+	CHECK(ParsedValue("width", "auto") == "auto");
+
+	CHECK(ParsedValue("background-color", "#abc") == "rgba(170,187,204,255)");
+	CHECK(ParsedValue("background-color", "red") == "rgba(255,0,0,255)");
+
+	CHECK(ParsedValue("transform", "translateX(10px)") == "translateX(10px)");
+	CHECK(ParsedValue("transform", "translate(20in, 50em)") == "translate(20in, 50em)");
+
+	CHECK(ParsedValue("box-shadow", "2px 2px 0px, #00ff 4px 4px 2em") == "rgba(0, 0, 0, 255) 2px 2px 0px, rgba(0, 0, 255, 255) 4px 4px 2em");
+
+	// Due to conversion to and from premultiplied alpha, some color information is lost.
+	CHECK(ParsedValue("box-shadow", "#fff0 2px 2px 0px") == "rgba(0, 0, 0, 0) 2px 2px 0px");
+
+	CHECK(ParsedValue("decorator", "linear-gradient(110deg, #fff3, #fff 10%, #c33 250dp, #3c3, #33c, #000 90%, #0003) border-box") ==
+		"linear-gradient(110deg, #fff3, #fff 10%, #c33 250dp, #3c3, #33c, #000 90%, #0003) border-box");
+
+	CHECK(ParsedValue("filter", "drop-shadow(#000 30px 20px 5px) opacity(0.2) sepia(0.2)") ==
+		"drop-shadow(#000 30px 20px 5px) opacity(0.2) sepia(0.2)");
+
+	Rml::Shutdown();
+}