From 249e9940da2aaf70e5fdda553dd670a9a6030fd7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 17 Sep 2019 15:44:35 +0200 Subject: [PATCH 1/2] Added failing tests for #2982. Crash in renderer when reparenting a control. --- .../Rendering/DeferredRendererTests.cs | 50 ++++++++++++++++ .../Rendering/SceneGraph/SceneBuilderTests.cs | 58 +++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs index 4c302a24a2..568ccb81d8 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs @@ -270,6 +270,56 @@ namespace Avalonia.Visuals.UnitTests.Rendering 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(); + + 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 index 3d084b81e1..13bcd27240 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs @@ -475,6 +475,64 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph } } + [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.SortChildren(scene); + moveToNode.SortChildren(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() { From db8751d71164f866d4b3acb59cd3e13900096239 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 17 Sep 2019 15:47:41 +0200 Subject: [PATCH 2/2] Handle reparenting controls in SceneBuilder. Renamed `VisualNode.SortChildren` -> `UpdateChildren` and make it remove nodes for controls that are no longer children. --- .../Rendering/DeferredRenderer.cs | 2 +- .../Rendering/SceneGraph/VisualNode.cs | 15 ++++++++++++--- .../Rendering/SceneGraph/SceneBuilderTests.cs | 4 ++-- .../Rendering/SceneGraph/VisualNodeTests.cs | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index efcc555159..0ff0285a04 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -540,7 +540,7 @@ namespace Avalonia.Rendering 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 index f579bf0a62..f079023c6d 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -174,12 +174,12 @@ namespace Avalonia.Rendering.SceneGraph /// /// Sorts the 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. /// /// The scene that the node is a part of. - 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 @@ namespace Avalonia.Rendering.SceneGraph 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 @@ namespace Avalonia.Rendering.SceneGraph if (node != null) { _children.Add(node); + toRemove.Remove(node); } } + + foreach (var node in toRemove) + { + scene.Remove(node); + } } /// diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs index 13bcd27240..dcf23e94e2 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs @@ -521,8 +521,8 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph moveFromNode = (VisualNode)scene.FindNode(moveFrom); moveToNode = (VisualNode)scene.FindNode(moveTo); - moveFromNode.SortChildren(scene); - moveToNode.SortChildren(scene); + moveFromNode.UpdateChildren(scene); + moveToNode.UpdateChildren(scene); sceneBuilder.Update(scene, moveFrom); sceneBuilder.Update(scene, moveTo); sceneBuilder.Update(scene, moveMe); diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs index 24ba2d1c48..d4f7a6a142 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs @@ -99,7 +99,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph var node = new VisualNode(Mock.Of(), null); var scene = new Scene(Mock.Of()); - node.SortChildren(scene); + node.UpdateChildren(scene); } } }