diff --git a/src/Avalonia.Base/Media/PolylineGeometry.cs b/src/Avalonia.Base/Media/PolylineGeometry.cs index 47cf2f48a4..13b11d8444 100644 --- a/src/Avalonia.Base/Media/PolylineGeometry.cs +++ b/src/Avalonia.Base/Media/PolylineGeometry.cs @@ -1,8 +1,10 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using Avalonia.Collections; using Avalonia.Metadata; using Avalonia.Platform; +using Avalonia.Reactive; namespace Avalonia.Media { @@ -100,10 +102,8 @@ namespace Avalonia.Media private void OnPointsChanged(IList? newValue) { _pointsObserver?.Dispose(); - _pointsObserver = (newValue as IAvaloniaList)?.ForEachItem( - _ => InvalidateGeometry(), - _ => InvalidateGeometry(), - InvalidateGeometry); + _pointsObserver = (newValue as INotifyCollectionChanged)?.GetWeakCollectionChangedObservable() + .Subscribe(_ => InvalidateGeometry()); } } } diff --git a/src/Avalonia.Controls/Shapes/Path.cs b/src/Avalonia.Controls/Shapes/Path.cs index 9cbe6f943f..e743527a2c 100644 --- a/src/Avalonia.Controls/Shapes/Path.cs +++ b/src/Avalonia.Controls/Shapes/Path.cs @@ -8,12 +8,9 @@ namespace Avalonia.Controls.Shapes public static readonly StyledProperty DataProperty = AvaloniaProperty.Register(nameof(Data)); - private EventHandler? _geometryChangedHandler; - static Path() { AffectsGeometry(DataProperty); - DataProperty.Changed.AddClassHandler((o, e) => o.DataChanged(e)); } public Geometry? Data @@ -22,54 +19,6 @@ namespace Avalonia.Controls.Shapes set => SetValue(DataProperty, value); } - private EventHandler GeometryChangedHandler => _geometryChangedHandler ??= GeometryChanged; - protected override Geometry? CreateDefiningGeometry() => Data; - - protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) - { - base.OnAttachedToVisualTree(e); - - if (Data is object) - { - Data.Changed += GeometryChangedHandler; - } - } - - protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) - { - base.OnDetachedFromVisualTree(e); - - if (Data is object) - { - Data.Changed -= GeometryChangedHandler; - } - } - - private void DataChanged(AvaloniaPropertyChangedEventArgs e) - { - if (VisualRoot is null) - { - return; - } - - var oldGeometry = (Geometry?)e.OldValue; - var newGeometry = (Geometry?)e.NewValue; - - if (oldGeometry is object) - { - oldGeometry.Changed -= GeometryChangedHandler; - } - - if (newGeometry is object) - { - newGeometry.Changed += GeometryChangedHandler; - } - } - - private void GeometryChanged(object? sender, EventArgs e) - { - InvalidateGeometry(); - } } } diff --git a/src/Avalonia.Controls/Shapes/Polygon.cs b/src/Avalonia.Controls/Shapes/Polygon.cs index e51e117e33..48530432f0 100644 --- a/src/Avalonia.Controls/Shapes/Polygon.cs +++ b/src/Avalonia.Controls/Shapes/Polygon.cs @@ -27,7 +27,7 @@ namespace Avalonia.Controls.Shapes protected override Geometry CreateDefiningGeometry() { - return new PolylineGeometry(Points, true); + return new PolylineGeometry { Points = Points, IsFilled = true }; } } } diff --git a/src/Avalonia.Controls/Shapes/Polyline.cs b/src/Avalonia.Controls/Shapes/Polyline.cs index 84fccb66f1..628fc70e1d 100644 --- a/src/Avalonia.Controls/Shapes/Polyline.cs +++ b/src/Avalonia.Controls/Shapes/Polyline.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using Avalonia.Media; using Avalonia.Data; @@ -28,7 +29,7 @@ namespace Avalonia.Controls.Shapes protected override Geometry CreateDefiningGeometry() { - return new PolylineGeometry(Points, false); + return new PolylineGeometry { Points = Points, IsFilled = false }; } } } diff --git a/src/Avalonia.Controls/Shapes/Shape.cs b/src/Avalonia.Controls/Shapes/Shape.cs index e358656fae..8457f4789c 100644 --- a/src/Avalonia.Controls/Shapes/Shape.cs +++ b/src/Avalonia.Controls/Shapes/Shape.cs @@ -64,6 +64,7 @@ namespace Avalonia.Controls.Shapes private Geometry? _definingGeometry; private Geometry? _renderedGeometry; private IPen? _strokePen; + private EventHandler? _geometryChangedHandler; /// /// Gets a value that represents the of the shape. @@ -75,6 +76,10 @@ namespace Avalonia.Controls.Shapes if (_definingGeometry == null) { _definingGeometry = CreateDefiningGeometry(); + if (_definingGeometry is not null && VisualRoot is not null) + { + _definingGeometry.Changed += GeometryChangedHandler; + } } return _definingGeometry; @@ -186,6 +191,8 @@ namespace Avalonia.Controls.Shapes get => GetValue(StrokeJoinProperty); set => SetValue(StrokeJoinProperty, value); } + + private EventHandler GeometryChangedHandler => _geometryChangedHandler ??= OnGeometryChanged; public sealed override void Render(DrawingContext context) { @@ -225,12 +232,29 @@ namespace Avalonia.Controls.Shapes /// /// Defining of the shape. protected abstract Geometry? CreateDefiningGeometry(); + + /// + /// Called when the underlying changed + /// + /// + /// + protected virtual void OnGeometryChanged(object? sender, EventArgs e) + { + _renderedGeometry = null; + + InvalidateMeasure(); + } /// /// Invalidates the geometry of this shape. /// protected void InvalidateGeometry() { + if (_definingGeometry is not null) + { + _definingGeometry.Changed -= GeometryChangedHandler; + } + _renderedGeometry = null; _definingGeometry = null; @@ -294,6 +318,26 @@ namespace Avalonia.Controls.Shapes return default; } + + protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) + { + base.OnAttachedToVisualTree(e); + + if (_definingGeometry is not null) + { + _definingGeometry.Changed += GeometryChangedHandler; + } + } + + protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) + { + base.OnDetachedFromVisualTree(e); + + if (_definingGeometry is not null) + { + _definingGeometry.Changed -= GeometryChangedHandler; + } + } internal static (Size size, Matrix transform) CalculateSizeAndTransform(Size availableSize, Rect shapeBounds, Stretch Stretch) { diff --git a/tests/Avalonia.Controls.UnitTests/Shapes/PolygonTests.cs b/tests/Avalonia.Controls.UnitTests/Shapes/PolygonTests.cs new file mode 100644 index 0000000000..1584e836e8 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/Shapes/PolygonTests.cs @@ -0,0 +1,28 @@ +using System.Collections.ObjectModel; +using Avalonia.Controls.Shapes; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Controls.UnitTests.Shapes; + +public class PolygonTests +{ + [Fact] + public void Polygon_Will_Update_Geometry_On_Shapes_Collection_Content_Change() + { + using var app = UnitTestApplication.Start(TestServices.MockPlatformRenderInterface); + var points = new ObservableCollection(); + + var target = new Polygon() { Points = points }; + target.Measure(new Size()); + Assert.True(target.IsMeasureValid); + + var root = new TestRoot(target); + + points.Add(new Point()); + + Assert.False(target.IsMeasureValid); + + root.Child = null; + } +} diff --git a/tests/Avalonia.Controls.UnitTests/Shapes/PolylineTests.cs b/tests/Avalonia.Controls.UnitTests/Shapes/PolylineTests.cs new file mode 100644 index 0000000000..a68dc9d4b4 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/Shapes/PolylineTests.cs @@ -0,0 +1,28 @@ +using System.Collections.ObjectModel; +using Avalonia.Controls.Shapes; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Controls.UnitTests.Shapes; + +public class PolylineTests +{ + [Fact] + public void Polyline_Will_Update_Geometry_On_Shapes_Collection_Content_Change() + { + using var app = UnitTestApplication.Start(TestServices.MockPlatformRenderInterface); + var points = new ObservableCollection(); + + var target = new Polyline { Points = points }; + target.Measure(new Size()); + Assert.True(target.IsMeasureValid); + + var root = new TestRoot(target); + + points.Add(new Point()); + + Assert.False(target.IsMeasureValid); + + root.Child = null; + } +} diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 8c39334820..36f0818638 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -713,6 +713,48 @@ namespace Avalonia.LeakTests GC.KeepAlive(geometry); } } + + [Fact] + public void Polyline_WithObservableCollectionPointsBinding_Is_Freed() + { + using (Start()) + { + var observableCollection = new ObservableCollection(){new()}; + + Func run = () => + { + var window = new Window + { + Content = new Polyline() + { + Points = observableCollection + } + }; + + window.Show(); + + window.LayoutManager.ExecuteInitialLayoutPass(); + Assert.IsType(window.Presenter.Child); + + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + // Process all Loaded events to free control reference(s) + Dispatcher.UIThread.RunJobs(DispatcherPriority.Loaded); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + + // We are keeping collection alive to simulate a resource that outlives the control. + GC.KeepAlive(observableCollection); + } + } [Fact] public void ElementName_Binding_In_DataTemplate_Is_Freed()