Transform animations:

  - Rotations can now be interpolated (without decomposing) when their rotation axes align.
  - Fixed a bug where the first animation key did not have its transform prepared.
  - Fixed a bug where translate3d and scale3d did not convert to their own generic type.

Include/RmlUi/Core/Variant.h

@@ -106,7 +106,7 @@ public:
 	template< typename T >
 	bool GetInto(T& value) const;
 
-	/// Returns a reference to the variants underlying type.
+	/// Returns a reference to the variant's underlying type.
 	/// @warning: Undefined behavior if T does not represent the underlying type of the variant.
 	template< typename T>
 	const T& GetReference() const;

Samples/basic/animation/data/animation.rml

@@ -74,8 +74,9 @@
 		
 		/* -- TRANSFORM TESTS */
 		#generic {
-			/* Note the translateX vs translateY in animation target, and scaleX vs scaleY. In 
-			   order to match, they are converted to their generic forms, translate3d and scale3d. */
+			/* Note the translateX vs translateY in animation target, and rotateZ vs rotate3d, scaleX vs scaleY.
+			   In order to match, they are converted to their generic forms, translate3d, rotate3d, and scale3d.
+			   For rotate3d to match another rotation, their rotation axes must align. */
 			transform: translateX(100px) rotateZ(70deg) scaleX(1.3);
 		}
 		#combine {

Samples/basic/animation/src/main.cpp

@@ -88,7 +88,7 @@ public:
 			// Transform tests
 			{
 				auto el = document->GetElementById("generic");
-				auto p = Transform::MakeProperty({ Transforms::TranslateY{50, Property::PX}, Transforms::RotateZ{-90, Property::DEG}, Transforms::ScaleY{0.8f} });
+				auto p = Transform::MakeProperty({ Transforms::TranslateY{50, Property::PX}, Transforms::Rotate3D{0, 0, 1, -90, Property::DEG}, Transforms::ScaleY{0.8f} });
 				el->Animate("transform", p, 1.5f, Tween{Tween::Sine, Tween::InOut}, -1, true);
 			}
 			{

Source/Core/Element.cpp

@@ -2170,7 +2170,7 @@ ElementAnimationList::iterator Element::StartAnimation(PropertyId property_id, c
 	{
 		ElementAnimationOrigin origin = (origin_is_animation_property ? ElementAnimationOrigin::Animation : ElementAnimationOrigin::User);
 		double start_time = Clock::GetElapsedTime() + (double)delay;
-		*it = ElementAnimation{ property_id, origin, value, start_time, 0.0f, num_iterations, alternate_direction };
+		*it = ElementAnimation{ property_id, origin, value, *this, start_time, 0.0f, num_iterations, alternate_direction };
 	}
 	else
 	{
@@ -2219,7 +2219,7 @@ bool Element::StartTransition(const Transition & transition, const Property& sta
 	{
 		// Add transition as new animation
 		animations.push_back(
-			ElementAnimation{ transition.id, ElementAnimationOrigin::Transition, start_value, start_time, 0.0f, 1, false }
+			ElementAnimation{ transition.id, ElementAnimationOrigin::Transition, start_value, *this, start_time, 0.0f, 1, false }
 		);
 		it = (animations.end() - 1);
 	}
@@ -2230,7 +2230,7 @@ bool Element::StartTransition(const Transition & transition, const Property& sta
 		f = 1.0f - (1.0f - f)*transition.reverse_adjustment_factor;
 		duration = duration * f;
 		// Replace old transition
-		*it = ElementAnimation{ transition.id, ElementAnimationOrigin::Transition, start_value, start_time, 0.0f, 1, false };
+		*it = ElementAnimation{ transition.id, ElementAnimationOrigin::Transition, start_value, *this, start_time, 0.0f, 1, false };
 	}
 
 	bool result = it->AddKey(duration, target_value, *this, transition.tween, true);

Source/Core/ElementAnimation.cpp

@@ -171,23 +171,25 @@ static PrepareTransformResult PrepareTransformPair(Transform& t0, Transform& t1,
 	{
 		PrepareTransformResult result = PrepareTransformResult::Unchanged;
 		bool same_primitives = true;
+
 		for (size_t i = 0; i < prims0.size(); i++)
 		{
 			auto p0_type = prims0[i].primitive.type;
 			auto p1_type = prims1[i].primitive.type;
-			if (p0_type != p1_type)
+
+			// See if they are the same or can be converted to a matching generic type.
+			if (Primitive::TryConvertToMatchingGenericType(prims0[i], prims1[i]))
 			{
-				// They are not the same, but see if we can convert them to their more generic form
-				if (!Primitive::TryConvertToMatchingGenericType(prims0[i], prims1[i]))
-				{
-					same_primitives = false;
-					break;
-				}
 				if (prims0[i].primitive.type != p0_type)
 					(int&)result |= (int)PrepareTransformResult::ChangedT0;
 				if (prims1[i].primitive.type != p1_type)
 					(int&)result |= (int)PrepareTransformResult::ChangedT1;
 			}
+			else
+			{
+				same_primitives = false;
+				break;
+			}
 		}
 		if (same_primitives)
 			return result;
@@ -225,14 +227,9 @@ static PrepareTransformResult PrepareTransformPair(Transform& t0, Transform& t1,
 			{
 				auto big_type = big[i_big].primitive.type;
 
-				if (small_type == big_type)
+				if (Primitive::TryConvertToMatchingGenericType(small[i_small], big[i_big]))
 				{
-					// Exact match
-					match_success = true;
-				}
-				else if (Primitive::TryConvertToMatchingGenericType(small[i_small], big[i_big]))
-				{
-					// They matched in their more generic form, one or both primitives converted
+					// They matched exactly or in their more generic form. One or both primitives may have been converted.
 					match_success = true;
 					if (big[i_big].primitive.type != big_type)
 						changed_big = true;
@@ -295,9 +292,41 @@ static PrepareTransformResult PrepareTransformPair(Transform& t0, Transform& t1,
 
 static bool PrepareTransforms(std::vector<AnimationKey>& keys, Element& element, int start_index)
 {
-	if (keys.size() < 2 || start_index < 1)
+	bool result = true;
+
+	// Prepare each transform individually.
+	for (int i = start_index; i < (int)keys.size(); i++)
+	{
+		AnimationKey& key = keys[i];
+		Property& property = keys[i].property;
+
+		if (!property.value.GetReference<TransformPtr>())
+			property.value = std::make_shared<Transform>();
+
+		bool must_decompose = false;
+		Transform& transform = *property.value.GetReference<TransformPtr>();
+
+		for (auto& primitive : transform.GetPrimitives())
+		{
+			if (!primitive.PrepareForInterpolation(element))
+			{
+				must_decompose = true;
+				break;
+			}
+		}
+
+		if (must_decompose)
+			result = CombineAndDecompose(transform, element);
+	}
+
+	if (!result)
 		return false;
 
+	// We don't need to prepare the transforms pairwise if we only have a single key added so far.
+	if (keys.size() < 2 || start_index < 1)
+		return true;
+
+	// Now, prepare the transforms pair-wise so they can be interpolated.
 	const int N = (int)keys.size();
 
 	int count_iterations = -1;
@@ -347,7 +376,7 @@ static bool PrepareTransforms(std::vector<AnimationKey>& keys, Element& element,
 }
 
 
-ElementAnimation::ElementAnimation(PropertyId property_id, ElementAnimationOrigin origin, const Property& current_value, double start_world_time, float duration, int num_iterations, bool alternate_direction)
+ElementAnimation::ElementAnimation(PropertyId property_id, ElementAnimationOrigin origin, const Property& current_value, Element& element, double start_world_time, float duration, int num_iterations, bool alternate_direction)
 	: property_id(property_id), duration(duration), num_iterations(num_iterations), alternate_direction(alternate_direction), last_update_world_time(start_world_time),
 	time_since_iteration_start(0.0f), current_iteration(0), reverse_direction(false), animation_complete(false), origin(origin)
 {
@@ -355,29 +384,35 @@ ElementAnimation::ElementAnimation(PropertyId property_id, ElementAnimationOrigi
 	{
 		Log::Message(Log::LT_WARNING, "Property in animation key did not have a definition (while adding key '%s').", current_value.ToString().c_str());
 	}
-	InternalAddKey(0.0f, current_value, Tween{});
+	InternalAddKey(0.0f, current_value, element, Tween{});
 }
 
-bool ElementAnimation::InternalAddKey(float time, const Property& property, Tween tween)
+
+bool ElementAnimation::InternalAddKey(float time, const Property& in_property, Element& element, Tween tween)
 {
 	int valid_properties = (Property::NUMBER_LENGTH_PERCENT | Property::ANGLE | Property::COLOUR | Property::TRANSFORM);
 
-	if (!(property.unit & valid_properties))
+	if (!(in_property.unit & valid_properties))
 	{
-		Log::Message(Log::LT_WARNING, "Property '%s' is not a valid target for interpolation.", property.ToString().c_str());
+		Log::Message(Log::LT_WARNING, "Property '%s' is not a valid target for interpolation.", in_property.ToString().c_str());
 		return false;
 	}
 
-	keys.emplace_back(time, property, tween);
-	AnimationKey& key = keys.back();
+	keys.emplace_back(time, in_property, tween);
+	bool result = true;
 
-	if (key.property.unit == Property::TRANSFORM)
+	if (keys.back().property.unit == Property::TRANSFORM)
 	{
-		if (!key.property.value.GetReference<TransformPtr>())
-			key.property.value = std::make_shared<Transform>();
+		result = PrepareTransforms(keys, element, (int)keys.size() - 1);
 	}
 
-	return true;
+	if (!result)
+	{
+		Log::Message(Log::LT_WARNING, "Could not add animation key with property '%s'.", in_property.ToString().c_str());
+		keys.pop_back();
+	}
+
+	return result;
 }
 
 
@@ -388,46 +423,15 @@ bool ElementAnimation::AddKey(float target_time, const Property & in_property, E
 		Log::Message(Log::LT_WARNING, "Element animation was not initialized properly, can't add key.");
 		return false;
 	}
-	if (!InternalAddKey(target_time, in_property, tween))
+	if (!InternalAddKey(target_time, in_property, element, tween))
 	{
 		return false;
 	}
 
-	bool result = true;
-	auto& property = keys.back().property;
-
-	if (property.unit == Property::TRANSFORM)
-	{
-		bool must_decompose = false;
-		Transform& transform = *property.value.GetReference<TransformPtr>();
-
-		for (auto& primitive : transform.GetPrimitives())
-		{
-			if (!primitive.PrepareForInterpolation(element))
-			{
-				must_decompose = true;
-				break;
-			}
-		}
-
-		if(must_decompose)
-			result = CombineAndDecompose(transform, element);
-
-		if (result)
-			result = PrepareTransforms(keys, element, (int)keys.size() - 1);
-	}
-
-	if(result)
-	{
-		if(extend_duration) 
-			duration = target_time;
-	}
-	else
-	{
-		keys.pop_back();
-	}
+	if (extend_duration)
+		duration = target_time;
 
-	return result;
+	return true;
 }
 
 float ElementAnimation::GetInterpolationFactorAndKeys(int* out_key0, int* out_key1) const

Source/Core/ElementAnimation.h

@@ -69,12 +69,12 @@ private:
 	bool animation_complete;
 	ElementAnimationOrigin origin;
 
-	bool InternalAddKey(float time, const Property& property, Tween tween);
+	bool InternalAddKey(float time, const Property& property, Element& element, Tween tween);
 
 	float GetInterpolationFactorAndKeys(int* out_key0, int* out_key1) const;
 public:
 	ElementAnimation() {}
-	ElementAnimation(PropertyId property_id, ElementAnimationOrigin origin, const Property& current_value, double start_world_time, float duration, int num_iterations, bool alternate_direction);
+	ElementAnimation(PropertyId property_id, ElementAnimationOrigin origin, const Property& current_value, Element& element, double start_world_time, float duration, int num_iterations, bool alternate_direction);
 
 	bool AddKey(float target_time, const Property & property, Element & element, Tween tween, bool extend_duration);
 

Source/Core/TransformPrimitive.cpp

@@ -490,9 +490,14 @@ struct PrepareVisitor
 	}
 	bool operator()(Rotate3D& p)
 	{
-		// Rotate3D must be resolved to a full matrix for interpolation. 
-		// There is an exception in CSS specs when the two interpolating rotation vectors are in the same direction, but for simplicity we ignore this optimization.
-		return false;
+		// Rotate3D can be interpolated if and only if their rotation axes point in the same direction.
+		// We normalize the rotation vector here for easy comparison, and return true here. Later on we make the
+		// pair-wise check in 'TryConvertToMatchingGenericType' to see if we need to decompose.
+		Vector3f vec = Vector3f(p.values[0], p.values[1], p.values[2]).Normalise();
+		p.values[0] = vec.x;
+		p.values[1] = vec.y;
+		p.values[2] = vec.z;
+		return true;
 	}
 	bool operator()(Matrix3D& p)
 	{
@@ -552,36 +557,29 @@ bool Primitive::PrepareForInterpolation(Element & e) noexcept
 
 
 
-enum class GenericType { None, Scale3D, Translate3D };
+enum class GenericType { None, Scale3D, Translate3D, Rotate3D };
 
 struct GetGenericTypeVisitor
 {
-	GenericType common_type = GenericType::None;
-
-	GenericType operator()(const TranslateX& p) { return GenericType::Translate3D; }
-	GenericType operator()(const TranslateY& p) { return GenericType::Translate3D; }
-	GenericType operator()(const TranslateZ& p) { return GenericType::Translate3D; }
-	GenericType operator()(const Translate2D& p) { return GenericType::Translate3D; }
-	GenericType operator()(const ScaleX& p) { return GenericType::Scale3D; }
-	GenericType operator()(const ScaleY& p) { return GenericType::Scale3D; }
-	GenericType operator()(const ScaleZ& p) { return GenericType::Scale3D; }
-	GenericType operator()(const Scale2D& p) { return GenericType::Scale3D; }
-
-	template <typename T>
-	GenericType operator()(const T& p) { return GenericType::None; }
-
 	GenericType run(const PrimitiveVariant& primitive)
 	{
 		switch (primitive.type)
 		{
-		case PrimitiveVariant::TRANSLATEX:  return this->operator()(primitive.translate_x); break;
-		case PrimitiveVariant::TRANSLATEY:  return this->operator()(primitive.translate_y); break;
-		case PrimitiveVariant::TRANSLATEZ:  return this->operator()(primitive.translate_z); break;
-		case PrimitiveVariant::TRANSLATE2D: return this->operator()(primitive.translate_2d); break;
-		case PrimitiveVariant::SCALEX:      return this->operator()(primitive.scale_x); break;
-		case PrimitiveVariant::SCALEY:      return this->operator()(primitive.scale_y); break;
-		case PrimitiveVariant::SCALEZ:      return this->operator()(primitive.scale_z); break;
-		case PrimitiveVariant::SCALE2D:     return this->operator()(primitive.scale_2d); break;
+		case PrimitiveVariant::TRANSLATEX:  return GenericType::Translate3D;
+		case PrimitiveVariant::TRANSLATEY:  return GenericType::Translate3D;
+		case PrimitiveVariant::TRANSLATEZ:  return GenericType::Translate3D;
+		case PrimitiveVariant::TRANSLATE2D: return GenericType::Translate3D;
+		case PrimitiveVariant::TRANSLATE3D: return GenericType::Translate3D;
+		case PrimitiveVariant::SCALEX:      return GenericType::Scale3D;
+		case PrimitiveVariant::SCALEY:      return GenericType::Scale3D;
+		case PrimitiveVariant::SCALEZ:      return GenericType::Scale3D;
+		case PrimitiveVariant::SCALE2D:     return GenericType::Scale3D;
+		case PrimitiveVariant::SCALE3D:     return GenericType::Scale3D;
+		case PrimitiveVariant::ROTATEX:     return GenericType::Rotate3D;
+		case PrimitiveVariant::ROTATEY:     return GenericType::Rotate3D;
+		case PrimitiveVariant::ROTATEZ:     return GenericType::Rotate3D;
+		case PrimitiveVariant::ROTATE2D:    return GenericType::Rotate3D;
+		case PrimitiveVariant::ROTATE3D:    return GenericType::Rotate3D;
 		default:
 			break;
 		}
@@ -600,12 +598,14 @@ struct ConvertToGenericTypeVisitor
 	Scale3D operator()(const ScaleY& p) { return Scale3D{  1.0f, p.values[0], 1.0f }; }
 	Scale3D operator()(const ScaleZ& p) { return Scale3D{  1.0f, 1.0f, p.values[0] }; }
 	Scale3D operator()(const Scale2D& p) { return Scale3D{ p.values[0], p.values[1], 1.0f }; }
+	Rotate3D operator()(const RotateX& p)  { return Rotate3D{ 1, 0, 0, p.values[0], Property::RAD }; }
+	Rotate3D operator()(const RotateY& p)  { return Rotate3D{ 0, 1, 0, p.values[0], Property::RAD }; }
+	Rotate3D operator()(const RotateZ& p)  { return Rotate3D{ 0, 0, 1, p.values[0], Property::RAD }; }
+	Rotate3D operator()(const Rotate2D& p) { return Rotate3D{ 0, 0, 1, p.values[0], Property::RAD }; }
 
 	template <typename T>
 	PrimitiveVariant operator()(const T& p) { RMLUI_ERROR; return p; }
 
-
-
 	PrimitiveVariant run(const PrimitiveVariant& primitive)
 	{
 		PrimitiveVariant result = primitive;
@@ -615,10 +615,17 @@ struct ConvertToGenericTypeVisitor
 		case PrimitiveVariant::TRANSLATEY:  result.type = PrimitiveVariant::TRANSLATE3D; result.translate_3d = this->operator()(primitive.translate_y);  break;
 		case PrimitiveVariant::TRANSLATEZ:  result.type = PrimitiveVariant::TRANSLATE3D; result.translate_3d = this->operator()(primitive.translate_z);  break;
 		case PrimitiveVariant::TRANSLATE2D: result.type = PrimitiveVariant::TRANSLATE3D; result.translate_3d = this->operator()(primitive.translate_2d); break;
+		case PrimitiveVariant::TRANSLATE3D: break;
 		case PrimitiveVariant::SCALEX:      result.type = PrimitiveVariant::SCALE3D;     result.scale_3d     = this->operator()(primitive.scale_x);      break;
 		case PrimitiveVariant::SCALEY:      result.type = PrimitiveVariant::SCALE3D;     result.scale_3d     = this->operator()(primitive.scale_y);      break;
 		case PrimitiveVariant::SCALEZ:      result.type = PrimitiveVariant::SCALE3D;     result.scale_3d     = this->operator()(primitive.scale_z);      break;
 		case PrimitiveVariant::SCALE2D:     result.type = PrimitiveVariant::SCALE3D;     result.scale_3d     = this->operator()(primitive.scale_2d);     break;
+		case PrimitiveVariant::SCALE3D:     break;
+		case PrimitiveVariant::ROTATEX:     result.type = PrimitiveVariant::ROTATE3D;    result.rotate_3d    = this->operator()(primitive.rotate_x);     break;
+		case PrimitiveVariant::ROTATEY:     result.type = PrimitiveVariant::ROTATE3D;    result.rotate_3d    = this->operator()(primitive.rotate_y);     break;
+		case PrimitiveVariant::ROTATEZ:     result.type = PrimitiveVariant::ROTATE3D;    result.rotate_3d    = this->operator()(primitive.rotate_z);     break;
+		case PrimitiveVariant::ROTATE2D:    result.type = PrimitiveVariant::ROTATE3D;    result.rotate_3d    = this->operator()(primitive.rotate_2d);    break;
+		case PrimitiveVariant::ROTATE3D:    break;
 		default:
 			RMLUI_ASSERT(false);
 			break;
@@ -627,20 +634,42 @@ struct ConvertToGenericTypeVisitor
 	}
 };
 
+static bool CanInterpolateRotate3D(const Rotate3D& p0, const Rotate3D& p1)
+{
+	// Rotate3D can only be interpolated if and only if their rotation axes point in the same direction.
+	// Assumes each rotation axis has already been normalized.
+	auto& v0 = p0.values;
+	auto& v1 = p1.values;
+	return v0[0] == v1[0] && v0[1] == v1[1] && v0[2] == v1[2];
+}
 
 
 bool Primitive::TryConvertToMatchingGenericType(Primitive & p0, Primitive & p1) noexcept
 {
 	if (p0.primitive.type == p1.primitive.type)
+	{
+		if(p0.primitive.type == PrimitiveVariant::ROTATE3D && !CanInterpolateRotate3D(p0.primitive.rotate_3d, p1.primitive.rotate_3d))
+			return false;
+
 		return true;
+	}
 
 	GenericType c0 = GetGenericTypeVisitor{}.run(p0.primitive);
 	GenericType c1 = GetGenericTypeVisitor{}.run(p1.primitive);
 
 	if (c0 == c1 && c0 != GenericType::None)
 	{
-		p0.primitive = ConvertToGenericTypeVisitor{}.run(p0.primitive);
-		p1.primitive = ConvertToGenericTypeVisitor{}.run(p1.primitive);
+		PrimitiveVariant new_p0 = ConvertToGenericTypeVisitor{}.run(p0.primitive);
+		PrimitiveVariant new_p1 = ConvertToGenericTypeVisitor{}.run(p1.primitive);
+		
+		RMLUI_ASSERT(new_p0.type == new_p1.type);
+		
+		if (new_p0.type == PrimitiveVariant::ROTATE3D && !CanInterpolateRotate3D(new_p0.rotate_3d, new_p1.rotate_3d))
+			return false;
+
+		p0.primitive = new_p0;
+		p1.primitive = new_p1;
+
 		return true;
 	}
 
@@ -671,10 +700,12 @@ struct InterpolateVisitor
 	}
 	bool Interpolate(Rotate3D& p0, const Rotate3D& p1)
 	{
-		// Currently, we promote Rotate3D to decomposed matrices in PrepareForInterpolation(), thus, it is an error if we get here. Make sure primitives are prepared and decomposed as necessary.
-		// We may change this later by assuming that the underlying direction vectors are equivalent (else, need to do full matrix interpolation)
-		// If we change this later: p0.values[3] = p0.values[3] * (1.0f - alpha) + p1.values[3] * alpha;
-		return false;
+		RMLUI_ASSERT(CanInterpolateRotate3D(p0, p1));
+		// We can only interpolate rotate3d if their rotation axes align. That should be the case if we get here, 
+		// otherwise the generic type matching should decompose them. Thus, we only need to interpolate
+		// the angle value here.
+		p0.values[3] = p0.values[3] * (1.0f - alpha) + p1.values[3] * alpha;
+		return true;
 	}
 	bool Interpolate(Matrix2D& p0, const Matrix2D& p1) { return false; /* Error if we get here, see PrepareForInterpolation() */ }
 	bool Interpolate(Matrix3D& p0, const Matrix3D& p1) { return false; /* Error if we get here, see PrepareForInterpolation() */ }