CompositionAnimation fixes#20936
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses broken composition expression animations by simplifying the expression numeric type system (removing the Scalar variant and standardizing on Double), fixing property lookup for server objects during animation evaluation, and centralizing expression keyword strings to reduce duplication and inconsistencies.
Changes:
- Removed
VariantType.Scalarusage paths and updated tests/operators/FFI mappings to useVariantType.Doublefor scalar math. - Reworked server-side composition property lookup to rely on
ServerObject.GetCompositionProperty(...)instead of a global registry/dictionary path. - Introduced
ExpressionKeywordsconstants and updated parsing/reference tracking, including special handling forthis.target.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationTests.cs | Updates assertions to validate scalar results via .Double instead of .Scalar. |
| tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationParserTests.cs | Removes Scalar-type branching and expects expression results as Double. |
| src/tools/DevGenerators/CompositionGenerator/Generator.cs | Expands variant-capable property types to include double and Vector. |
| src/Avalonia.Base/Rendering/Composition/Server/ServerObjectAnimations.cs | Switches animation property lookup to GetCompositionProperty on the owner. |
| src/Avalonia.Base/Rendering/Composition/Server/ServerObject.cs | Uses GetCompositionProperty for expression property access when not animated. |
| src/Avalonia.Base/Rendering/Composition/Server/CompositionProperty.cs | Removes the dynamic registry implementation and leaves a simplified registration path. |
| src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionVariant.cs | Removes Scalar representation and refactors operators/casting accordingly. |
| src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionParser.cs | Uses centralized ExpressionKeywords for keyword parsing. |
| src/Avalonia.Base/Rendering/Composition/Expressions/Expression.cs | Adds ExpressionKeywords and updates reference collection for this.target. |
| src/Avalonia.Base/Rendering/Composition/Expressions/DelegateExpressionFfi.cs | Maps float to VariantType.Double and removes Scalar-related casts. |
| src/Avalonia.Base/Rendering/Composition/Animations/AnimationInstanceBase.cs | Tracks this.target references by mapping them to the animation’s TargetObject. |
Comments suppressed due to low confidence (2)
src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionVariant.cs:441
- Now that scalar literals are
VariantType.Double,operator *no longer handles scalar scaling for most vector/matrix/quaternion types. For example,Vector2 * Double/Vector3 * Double/Vector4 * Doublewill currently returndefault(invalid) because onlyVector * Doubleis implemented. Please add the missing* Doublebranches (casting tofloatwhere required) for the types that previously supported* Scalar.
if (left.Type == VariantType.Vector2 && right.Type == VariantType.Vector2)
return left.Vector2 * right.Vector2;
if (left.Type == VariantType.Vector && right.Type == VariantType.Vector)
return Vector.Multiply(left.Vector, right.Vector);
if (left.Type == VariantType.Vector && right.Type == VariantType.Double)
return left.Vector * right.Double;
src/Avalonia.Base/Rendering/Composition/Expressions/DelegateExpressionFfi.cs:118
- Mapping
floattoVariantType.DoublemakesfloatanddoubleFFI overloads indistinguishable at runtime. The registry will then pick the first matching record based on insertion order, and can also force double values to be cast down tofloatinside delegates. This is observable with existing built-ins likeVector2(float,float)andVector2(double,double)/Vector3(float,float,float)andVector3(double,double,double)which will now share the same signature. Consider normalizing built-in scalar signatures todouble(or rejecting duplicate registrations) to avoid order-dependent behavior and precision loss.
static readonly Dictionary<Type, VariantType> TypeMap = new Dictionary<Type, VariantType>
{
[typeof(bool)] = VariantType.Boolean,
[typeof(float)] = VariantType.Double,
[typeof(double)] = VariantType.Double,
[typeof(Vector2)] = VariantType.Vector2,
[typeof(Vector)] = VariantType.Vector,
[typeof(Vector3)] = VariantType.Vector3,
[typeof(Vector3D)] = VariantType.Vector3D,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionVariant.cs
Show resolved
Hide resolved
src/Avalonia.Base/Rendering/Composition/Server/CompositionProperty.cs
Outdated
Show resolved
Hide resolved
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
There was a problem hiding this comment.
Pull request overview
This PR fixes broken Composition/Expression animations by simplifying the expression numeric type system (removing the Scalar variant and standardizing on Double), centralizing expression keyword strings, and adjusting composition property lookup/tracking so expression dependencies and property resolution work correctly.
Changes:
- Remove
VariantType.Scalarand migrate scalar parsing/math/casting toVariantType.Double. - Centralize expression keyword literals via
ExpressionKeywordsand update parsing/reference collection/tracking (includingthis.Target). - Switch composition property lookup to
ServerObject.GetCompositionProperty, update generator type support (double,Vector), and add/adjust unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationTests.cs | Updates scalar assertions to Double and adds new coverage for property lookup + expression tracking. |
| tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationParserTests.cs | Updates parser test to expect only Double results for scalars. |
| src/tools/DevGenerators/CompositionGenerator/Generator.cs | Extends variant get-property generation to include double and Vector. |
| src/Avalonia.Base/Rendering/Composition/Server/ServerObjectAnimations.cs | Uses GetCompositionProperty for animation property resolution; improves subscription invalidation behavior. |
| src/Avalonia.Base/Rendering/Composition/Server/ServerObject.cs | Routes expression property access through GetCompositionProperty. |
| src/Avalonia.Base/Rendering/Composition/Server/CompositionProperty.cs | Removes the old global per-type registry mechanics, keeping property creation only. |
| src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionVariant.cs | Removes Scalar, updates arithmetic/comparison/casting, and standardizes scalar behavior on Double. |
| src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionParser.cs | Replaces keyword literals with ExpressionKeywords. |
| src/Avalonia.Base/Rendering/Composition/Expressions/Expression.cs | Introduces ExpressionKeywords and updates reference collection for this.Target. |
| src/Avalonia.Base/Rendering/Composition/Expressions/DelegateExpressionFfi.cs | Updates type mapping (float → Double) and removes Scalar cast logic. |
| src/Avalonia.Base/Rendering/Composition/Animations/AnimationInstanceBase.cs | Special-cases reference tracking for this.Target so target object dependencies are subscribed correctly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Avalonia.Base/Rendering/Composition/Expressions/ExpressionVariant.cs
Outdated
Show resolved
Hide resolved
tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationTests.cs
Outdated
Show resolved
Hide resolved
tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationTests.cs
Outdated
Show resolved
Hide resolved
tests/Avalonia.Base.UnitTests/Composition/CompositionAnimationTests.cs
Outdated
Show resolved
Hide resolved
src/Avalonia.Base/Rendering/Composition/Expressions/Expression.cs
Outdated
Show resolved
Hide resolved
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
What does the pull request do?
This pull request removes the
Scalarvariant type from the composition expression system and standardizes all scalar numeric values to use theDoubletype. It also introduces a centralizedExpressionKeywordsclass for managing expression keywords, which improves maintainability and consistency across the codebase. The changes simplify arithmetic and comparison operations, update type mappings, and ensure correct handling of tracked objects and references.Type System Simplification
Scalarvariant fromExpressionVariant, consolidating all scalar numeric values under theDoubletype. This change affects implicit conversions, arithmetic, and comparison operations, which now exclusively useDoublefor scalar math.DelegateExpressionFfito mapfloattoVariantType.Doubleinstead of the removedScalartype.Arithmetic and Comparison Operation Updates
Refactored arithmetic and comparison operators in
ExpressionVariantto remove all logic related to theScalartype and ensure operations involving scalars useDouble. This includes addition, subtraction, multiplication, division, modulus, and comparison operators.Fixed a bug in vector division where previously
right.Scalarwas used; now correctly usesright.Double.Expression Keyword Centralization
Introduced the
ExpressionKeywordsclass to centralize keyword constants, replacing scattered string literals throughout the expression parsing and evaluation code.Updated
ExpressionParserand reference collection logic to use the newExpressionKeywordsclass for improved maintainability and consistency. [1] [2]Object Tracking and Parameter Handling
AnimationInstanceBaseto correctly handle thethis.targetkeyword using the new centralized keyword class, ensuring the correct object is tracked for target expressions.Type Casting Logic
DelegateExpressionFfito remove support for casting betweenDoubleand the removedScalartype, ensuring only valid type casts are allowed.What is the current behavior?
First, nothing is written to s_dynamicRegistry(CompositionProperty.cs), thus all composition properties seem not correctly registered. Thus, GetPropertyAnimation method(ServerObjectAnimation.cs) always return default, making ExpressionAnimation completely broken.
Second, VariantPropertyTypes (Generator.cs) does not contain Vector type, so it always generate null for GetVariant parameter .
The last and trickiest, constants like '0.5' are parsed as Scalar, and when we do operations with double values(e.g. Vector3D.X), it will return Invalid value as they're different ExpressionVarient types.
These problems made CompositionAnimation (especially ExpressionAnimation) hard to work properly.
What is the updated/expected behavior with this PR?
20260319-1307-42.2557921.mp4
How was the solution implemented (if it's not obvious)?
Checklist
Breaking changes
Obsoletions / Deprecations
Fixed issues
#16774
#18784