Merge pull request #3004 from AvaloniaUI/fixes/2983-reparent-controls…

…-crash Correctly handle reparenting of controls
AvaloniaUI · Sep 19, 2019 · 734df11 · 734df11
2 parents 0671005 + 9da4dfd
commit 734df11
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 5 deletions.
diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs
@@ -540,7 +540,7 @@ private void UpdateScene()
                     foreach (var visual in _recalculateChildren)
                     {
                         var node = scene.FindNode(visual);
-                        ((VisualNode)node)?.SortChildren(scene);
+                        ((VisualNode)node)?.UpdateChildren(scene);
                     }
 
                     _recalculateChildren.Clear();

diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs
@@ -174,12 +174,12 @@ public void ReplaceDrawOperation(int index, IRef<IDrawOperation> operation)
 
         /// <summary>
         /// Sorts the <see cref="Children"/> collection according to the order of the visual's
-        /// children and their z-index.
+        /// children and their z-index and removes controls that are no longer children.
         /// </summary>
         /// <param name="scene">The scene that the node is a part of.</param>
-        public void SortChildren(Scene scene)
+        public void UpdateChildren(Scene scene)
         {
-            if (_children == null || _children.Count <= 1)
+            if (_children == null || _children.Count == 0)
             {
                 return;
             }
@@ -193,9 +193,12 @@ public void SortChildren(Scene scene)
                 keys.Add(((long)zIndex << 32) + i);
             }
 
+            var toRemove = _children.ToList();
+
             keys.Sort();
             _children.Clear();
 
+
             foreach (var i in keys)
             {
                 var child = Visual.VisualChildren[(int)(i & 0xffffffff)];
@@ -204,8 +207,14 @@ public void SortChildren(Scene scene)
                 if (node != null)
                 {
                     _children.Add(node);
+                    toRemove.Remove(node);
                 }
             }
+
+            foreach (var node in toRemove)
+            {
+                scene.Remove(node);
+            }
         }
 
         /// <summary>

diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs
@@ -270,6 +270,56 @@ public void Should_Update_VisualNode_Order_On_ZIndex_Change_With_Dirty_Ancestor(
             Assert.Same(stackNode.Children[1].Visual, canvas1);
         }
 
+        [Fact]
+        public void Should_Update_VisualNodes_When_Child_Moved_To_New_Parent()
+        {
+            var dispatcher = new ImmediateDispatcher();
+            var loop = new Mock<IRenderLoop>();
+
+            Decorator moveFrom;
+            Decorator moveTo;
+            Canvas moveMe;
+            var root = new TestRoot
+            {
+                Child = new StackPanel
+                {
+                    Children =
+                    {
+                        (moveFrom = new Decorator
+                        {
+                            Child = moveMe = new Canvas(),
+                        }),
+                        (moveTo = new Decorator()),
+                    }
+                }
+            };
+
+            var sceneBuilder = new SceneBuilder();
+            var target = new DeferredRenderer(
+                root,
+                loop.Object,
+                sceneBuilder: sceneBuilder,
+                dispatcher: dispatcher);
+
+            root.Renderer = target;
+            target.Start();
+            RunFrame(target);
+
+            moveFrom.Child = null;
+            moveTo.Child = moveMe;
+
+            RunFrame(target);
+
+            var scene = target.UnitTestScene();
+            var moveFromNode = (VisualNode)scene.FindNode(moveFrom);
+            var moveToNode = (VisualNode)scene.FindNode(moveTo);
+
+            Assert.Empty(moveFromNode.Children);
+            Assert.Equal(1, moveToNode.Children.Count);
+            Assert.Same(moveMe, moveToNode.Children[0].Visual);
+
+        }
+
         [Fact]
         public void Should_Push_Opacity_For_Controls_With_Less_Than_1_Opacity()
         {

diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs
@@ -475,6 +475,64 @@ public void Should_Update_When_Control_Removed()
             }
         }
 
+        [Fact]
+        public void Should_Update_When_Control_Moved()
+        {
+            using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface))
+            {
+                Decorator moveFrom;
+                Decorator moveTo;
+                Canvas moveMe;
+                var tree = new TestRoot
+                {
+                    Width = 100,
+                    Height = 100,
+                    Child = new StackPanel
+                    {
+                        Children =
+                        {
+                            (moveFrom = new Decorator
+                            {
+                                Child = moveMe = new Canvas(),
+                            }),
+                            (moveTo = new Decorator()),
+                        }
+                    }
+                };
+
+                tree.Measure(Size.Infinity);
+                tree.Arrange(new Rect(tree.DesiredSize));
+
+                var scene = new Scene(tree);
+                var sceneBuilder = new SceneBuilder();
+                sceneBuilder.UpdateAll(scene);
+
+                var moveFromNode = (VisualNode)scene.FindNode(moveFrom);
+                var moveToNode = (VisualNode)scene.FindNode(moveTo);
+
+                Assert.Equal(1, moveFromNode.Children.Count);
+                Assert.Same(moveMe, moveFromNode.Children[0].Visual);
+                Assert.Empty(moveToNode.Children);
+
+                moveFrom.Child = null;
+                moveTo.Child = moveMe;
+
+                scene = scene.CloneScene();
+                moveFromNode = (VisualNode)scene.FindNode(moveFrom);
+                moveToNode = (VisualNode)scene.FindNode(moveTo);
+
+                moveFromNode.UpdateChildren(scene);
+                moveToNode.UpdateChildren(scene);
+                sceneBuilder.Update(scene, moveFrom);
+                sceneBuilder.Update(scene, moveTo);
+                sceneBuilder.Update(scene, moveMe);
+
+                Assert.Empty(moveFromNode.Children);
+                Assert.Equal(1, moveToNode.Children.Count);
+                Assert.Same(moveMe, moveToNode.Children[0].Visual);
+            }
+        }
+
         [Fact]
         public void Should_Update_When_Control_Made_Invisible()
         {

diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs
@@ -99,7 +99,7 @@ public void SortChildren_Does_Not_Throw_On_Null_Children()
             var node = new VisualNode(Mock.Of<IVisual>(), null);
             var scene = new Scene(Mock.Of<IVisual>());
 
-            node.SortChildren(scene);
+            node.UpdateChildren(scene);
         }
     }
 }