CompositionBrush corrections (#21873)

* Add failing tests for composition gradient brush change tracking

Three defects in the unreleased CompositionBrush surface:

- Replacing GradientStops after the first commit never reaches the
  server: the hand-written property registers nothing for
  serialization, so the change only ships once an unrelated tracked
  property dirties the brush.
- SpreadMethod has the same gap.
- A mutable Media.GradientStop crosses the batch by reference and the
  render thread then reads a UI-thread object at replay time.
- A CompositionBrush resolves its server object for any compositor,
  so a foreign compositor ends up sharing a resource across render
  loops.

* Track composition gradient brush changes and respect compositor affinity

- GradientStops and SpreadMethod now register for serialization when
  assigned, so changes made after the first commit reach the server
  without waiting for an unrelated tracked property to dirty the
  brush. In-place list mutation stays untracked and is documented as
  requiring re-assignment.
- Non-composition gradient stops are snapshotted to
  ImmutableGradientStop at serialization time instead of crossing the
  batch by reference, since the render thread reads the server list at
  replay time.
- Resolving a CompositionBrush for a different compositor now throws
  instead of silently wiring one server resource into two render
  loops. Transient contexts without a compositor keep receiving the
  client brush unchanged.

Author: Gillibald

src/Avalonia.Base/Rendering/Composition/Brushes/CompositionBrush.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using Avalonia.Media;
+using Avalonia.Media.Immutable;
 using Avalonia.Rendering.Composition;
 using Avalonia.Rendering.Composition.Server;
 using Avalonia.Rendering.Composition.Transport;
@@ -42,10 +43,47 @@ partial class CompositionConicGradientBrush : IConicGradientBrush
 
 public abstract partial class CompositionGradientBrush : CompositionBrush, IGradientBrush
 {
+    private List<IGradientStop> _gradientStops = [];
+    private GradientSpreadMethod _spreadMethod;
+
     internal new ServerCompositionGradientBrush Server { get; }
-    public List<IGradientStop> GradientStops { get; set; } = [];
+
+    /// <summary>
+    /// The gradient stops. Mutations of the list itself are not tracked - assign
+    /// the property to ship in-place edits made after a commit. Stops created via
+    /// <see cref="Compositor.CreateGradientStop(double, Media.Color)"/> stay live
+    /// on the server and animate individually; other stops are snapshotted at
+    /// serialization time.
+    /// </summary>
+    public List<IGradientStop> GradientStops
+    {
+        get => _gradientStops;
+        set
+        {
+            if (ReferenceEquals(_gradientStops, value))
+                return;
+            _gradientStops = value;
+            RegisterForSerialization();
+        }
+    }
+
     IReadOnlyList<IGradientStop> IGradientBrush.GradientStops => GradientStops;
-    public GradientSpreadMethod SpreadMethod { get; set; }
+
+    /// <summary>
+    /// How the gradient repeats outside the stop range.
+    /// </summary>
+    public GradientSpreadMethod SpreadMethod
+    {
+        get => _spreadMethod;
+        set
+        {
+            if (_spreadMethod == value)
+                return;
+            _spreadMethod = value;
+            RegisterForSerialization();
+        }
+    }
+
     partial void OnRootChanged();
     partial void OnRootChanging();
 
@@ -63,7 +101,10 @@ private protected override void SerializeChangesCore(BatchStreamWriter writer)
             if (stop is CompositionGradientStop comp)
                 writer.WriteObject(comp.Server);
             else
-                writer.WriteObject(stop);
+                // A mutable UI-thread stop must not cross to the render thread
+                // by reference; ship its current values instead.
+                writer.WriteObject(stop as ImmutableGradientStop
+                    ?? new ImmutableGradientStop(stop.Offset, stop.Color));
         }
     }
 }

src/Avalonia.Base/Rendering/Composition/Drawing/ServerResourceHelperExtensions.cs

@@ -20,7 +20,14 @@ static class ServerResourceHelperExtensions
         if (brush is ICompositionRenderResource<IBrush> resource)
             return resource.GetForCompositor(compositor);
         if (brush is CompositionBrush compositionBrush)
+        {
+            // The server object belongs to its own compositor's render loop;
+            // handing it to another compositor would share one resource across
+            // two render threads.
+            if (compositionBrush.Compositor != compositor)
+                ThrowForeignCompositor(compositionBrush);
             return compositionBrush.Server;
+        }
         ThrowNotCompatible(brush);
         return null;
     }
@@ -42,6 +49,10 @@ static class ServerResourceHelperExtensions
     [MethodImpl(MethodImplOptions.NoInlining), DoesNotReturn]
     static void ThrowNotCompatible(object o) =>
         throw new InvalidOperationException(o.GetType() + " is not compatible with composition");
+
+    [MethodImpl(MethodImplOptions.NoInlining), DoesNotReturn]
+    static void ThrowForeignCompositor(CompositionObject o) =>
+        throw new InvalidOperationException(o.GetType() + " belongs to a different compositor");
 
     public static ITransform? GetServer(this ITransform? transform, Compositor? compositor)
     {

tests/Avalonia.Base.UnitTests/Composition/CompositionBrushTests.cs

@@ -0,0 +1,94 @@
+using System;
+using System.Collections.Generic;
+using Avalonia.Media;
+using Avalonia.Media.Immutable;
+using Avalonia.Rendering;
+using Avalonia.Rendering.Composition;
+using Avalonia.Rendering.Composition.Drawing;
+using Avalonia.Threading;
+using Avalonia.UnitTests;
+using Xunit;
+
+namespace Avalonia.Base.UnitTests.Composition;
+
+public class CompositionBrushTests : ScopedTestBase
+{
+    [Fact]
+    public void Replacing_The_Gradient_Stop_List_After_A_Commit_Should_Reach_The_Server()
+    {
+        using var services = new CompositorTestServices();
+        var compositor = services.Compositor;
+
+        var brush = compositor.CreateLinearGradientBrush();
+        brush.GradientStops.Add(compositor.CreateGradientStop(0, Colors.Red));
+        services.RunJobs();
+
+        Assert.Single(brush.Server.GradientStops);
+
+        brush.GradientStops = new List<IGradientStop>
+        {
+            compositor.CreateGradientStop(0, Colors.Red),
+            compositor.CreateGradientStop(1, Colors.Blue),
+        };
+        services.RunJobs();
+
+        Assert.Equal(2, brush.Server.GradientStops.Count);
+    }
+
+    [Fact]
+    public void Changing_SpreadMethod_After_A_Commit_Should_Reach_The_Server()
+    {
+        using var services = new CompositorTestServices();
+        var compositor = services.Compositor;
+
+        var brush = compositor.CreateLinearGradientBrush();
+        brush.GradientStops.Add(compositor.CreateGradientStop(0, Colors.Red));
+        services.RunJobs();
+
+        Assert.Equal(GradientSpreadMethod.Pad, brush.Server.SpreadMethod);
+
+        brush.SpreadMethod = GradientSpreadMethod.Repeat;
+        services.RunJobs();
+
+        Assert.Equal(GradientSpreadMethod.Repeat, brush.Server.SpreadMethod);
+    }
+
+    [Fact]
+    public void Mutable_Gradient_Stops_Should_Be_Snapshotted_For_The_Server()
+    {
+        using var services = new CompositorTestServices();
+        var compositor = services.Compositor;
+
+        var mutableStop = new GradientStop(Colors.Red, 0);
+        var brush = compositor.CreateLinearGradientBrush();
+        brush.GradientStops.Add(mutableStop);
+        services.RunJobs();
+
+        // The render thread reads the server list at replay time, so a mutable
+        // UI-thread stop must not cross the batch by reference.
+        var serverStop = Assert.Single(brush.Server.GradientStops);
+        Assert.NotSame(mutableStop, serverStop);
+        Assert.Equal(Colors.Red, serverStop.Color);
+        Assert.Equal(0, serverStop.Offset);
+    }
+
+    [Fact]
+    public void Using_A_Composition_Brush_With_A_Foreign_Compositor_Should_Throw()
+    {
+        using var services = new CompositorTestServices();
+
+        var brush = services.Compositor.CreateSolidColorBrush(Colors.Red);
+
+        var foreign = new Compositor(RenderLoop.FromTimer(services.Timer), null,
+            true, new DispatcherCompositorScheduler(), true, Dispatcher.UIThread);
+
+        // A composition brush's server object belongs to its own compositor's
+        // render loop; handing it to another compositor's stream would let two
+        // render threads race over one resource.
+        Assert.Throws<InvalidOperationException>(() => brush.GetServer(foreign));
+
+        // A transient context without a compositor keeps the client brush and
+        // draws its static values, so no affinity applies there.
+        Assert.Same(brush, brush.GetServer(null));
+    }
+}