From b85fd31d03920e91a75ece1bb004e39977e30876 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Tue, 31 Mar 2020 19:33:45 +0300 Subject: [PATCH 01/11] Immediate rendering support for X11 platform via render loop --- src/Avalonia.X11/X11ImmediateRendererProxy.cs | 107 ++++++++++++++++++ src/Avalonia.X11/X11Platform.cs | 1 + src/Avalonia.X11/X11Window.cs | 20 ++-- 3 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 src/Avalonia.X11/X11ImmediateRendererProxy.cs diff --git a/src/Avalonia.X11/X11ImmediateRendererProxy.cs b/src/Avalonia.X11/X11ImmediateRendererProxy.cs new file mode 100644 index 0000000000..f7bc7f9711 --- /dev/null +++ b/src/Avalonia.X11/X11ImmediateRendererProxy.cs @@ -0,0 +1,107 @@ +using System; +using System.Collections.Generic; +using Avalonia.Rendering; +using Avalonia.Threading; +using Avalonia.VisualTree; + +namespace Avalonia.X11 +{ + public class X11ImmediateRendererProxy : IRenderer, IRenderLoopTask + { + private readonly IRenderLoop _loop; + private ImmediateRenderer _renderer; + private bool _invalidated; + private object _lock = new object(); + + public X11ImmediateRendererProxy(IVisual root, IRenderLoop loop) + { + _loop = loop; + _renderer = new ImmediateRenderer(root); + + } + + public void Dispose() + { + _renderer.Dispose(); + } + + public bool DrawFps + { + get => _renderer.DrawFps; + set => _renderer.DrawFps = value; + } + + public bool DrawDirtyRects + { + get => _renderer.DrawDirtyRects; + set => _renderer.DrawDirtyRects = value; + } + + public event EventHandler SceneInvalidated + { + add => _renderer.SceneInvalidated += value; + remove => _renderer.SceneInvalidated -= value; + } + + public void AddDirty(IVisual visual) + { + lock (_lock) + _invalidated = true; + _renderer.AddDirty(visual); + } + + public IEnumerable HitTest(Point p, IVisual root, Func filter) + { + return _renderer.HitTest(p, root, filter); + } + + public IVisual HitTestFirst(Point p, IVisual root, Func filter) + { + return _renderer.HitTestFirst(p, root, filter); + } + + public void RecalculateChildren(IVisual visual) + { + _renderer.RecalculateChildren(visual); + } + + public void Resized(Size size) + { + _renderer.Resized(size); + } + + public void Paint(Rect rect) + { + _invalidated = false; + _renderer.Paint(rect); + } + + public void Start() + { + _loop.Add(this); + _renderer.Start(); + } + + public void Stop() + { + _loop.Remove(this); + _renderer.Stop(); + } + + public bool NeedsUpdate => false; + public void Update(TimeSpan time) + { + + } + + public void Render() + { + if (_invalidated) + { + lock (_lock) + _invalidated = false; + Dispatcher.UIThread.Post(() => Paint(new Rect(0, 0, 100000, 100000))); + } + } + } +} diff --git a/src/Avalonia.X11/X11Platform.cs b/src/Avalonia.X11/X11Platform.cs index 8b531bd9c5..028c47c978 100644 --- a/src/Avalonia.X11/X11Platform.cs +++ b/src/Avalonia.X11/X11Platform.cs @@ -96,6 +96,7 @@ namespace Avalonia public bool UseGpu { get; set; } = true; public bool OverlayPopups { get; set; } public bool UseDBusMenu { get; set; } + public bool UseDeferredRendering { get; set; } = true; public List GlxRendererBlacklist { get; set; } = new List { diff --git a/src/Avalonia.X11/X11Window.cs b/src/Avalonia.X11/X11Window.cs index 8419e7dab2..74f2606dc8 100644 --- a/src/Avalonia.X11/X11Window.cs +++ b/src/Avalonia.X11/X11Window.cs @@ -27,7 +27,6 @@ namespace Avalonia.X11 private readonly IWindowImpl _popupParent; private readonly bool _popup; private readonly X11Info _x11; - private bool _invalidated; private XConfigureEvent? _configure; private PixelPoint? _configurePoint; private bool _triggeredExpose; @@ -308,8 +307,13 @@ namespace Avalonia.X11 public Action Closed { get; set; } public Action PositionChanged { get; set; } - public IRenderer CreateRenderer(IRenderRoot root) => - new DeferredRenderer(root, AvaloniaLocator.Current.GetService()); + public IRenderer CreateRenderer(IRenderRoot root) + { + var loop = AvaloniaLocator.Current.GetService(); + return _platform.Options.UseDeferredRendering ? + new DeferredRenderer(root, loop) : + (IRenderer)new X11ImmediateRendererProxy(root, loop); + } void OnEvent(XEvent ev) { @@ -683,20 +687,12 @@ namespace Avalonia.X11 void DoPaint() { - _invalidated = false; Paint?.Invoke(new Rect()); } public void Invalidate(Rect rect) { - if(_invalidated) - return; - _invalidated = true; - Dispatcher.UIThread.InvokeAsync(() => - { - if (_mapped) - DoPaint(); - }); + } public IInputRoot InputRoot => _inputRoot; From 58a20b610940dd559581dedf794578b185484098 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Fri, 3 Apr 2020 17:23:06 +0300 Subject: [PATCH 02/11] [X11] Emulate synchronous resize/move events until window is mapped for the first time --- src/Avalonia.X11/X11Window.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.X11/X11Window.cs b/src/Avalonia.X11/X11Window.cs index 8419e7dab2..a24042184c 100644 --- a/src/Avalonia.X11/X11Window.cs +++ b/src/Avalonia.X11/X11Window.cs @@ -41,6 +41,7 @@ namespace Avalonia.X11 private IntPtr _xic; private IntPtr _renderHandle; private bool _mapped; + private bool _wasMappedAtLeastOnce = false; private HashSet _transientChildren = new HashSet(); private X11Window _transientParent; private double? _scalingOverride; @@ -769,6 +770,7 @@ namespace Avalonia.X11 void ShowCore() { + _wasMappedAtLeastOnce = true; XMapWindow(_x11.Display, _handle); XFlush(_x11.Display); } @@ -816,7 +818,7 @@ namespace Avalonia.X11 XConfigureResizeWindow(_x11.Display, _renderHandle, pixelSize); XFlush(_x11.Display); - if (force || (_popup && needImmediatePopupResize)) + if (force || !_wasMappedAtLeastOnce || (_popup && needImmediatePopupResize)) { _realSize = pixelSize; Resized?.Invoke(ClientSize); @@ -857,6 +859,11 @@ namespace Avalonia.X11 XConfigureWindow(_x11.Display, _handle, ChangeWindowFlags.CWX | ChangeWindowFlags.CWY, ref changes); XFlush(_x11.Display); + if (!_wasMappedAtLeastOnce) + { + _position = value; + PositionChanged?.Invoke(value); + } } } From b0c79e30948db9020ed9c71e1d8dc6a2440c5ed4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 3 Apr 2020 16:12:16 +0200 Subject: [PATCH 03/11] Added failing leak test for #3738. --- tests/Avalonia.LeakTests/ControlTests.cs | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 5cec5c134d..de3f0f8189 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.Remoting.Contexts; using Avalonia.Controls; using Avalonia.Controls.Templates; using Avalonia.Diagnostics; @@ -419,6 +420,37 @@ namespace Avalonia.LeakTests } } + [Fact] + public void Context_MenuItems_Are_Freed() + { + using (Start()) + { + void BuildAndShowContextMenu(Control control) + { + var contextMenu = new ContextMenu + { + Items = new[] + { + new MenuItem { Header = "Foo" }, + new MenuItem { Header = "Foo" }, + } + }; + + contextMenu.Open(control); + contextMenu.Close(); + } + + var window = new Window(); + BuildAndShowContextMenu(window); + BuildAndShowContextMenu(window); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + private IDisposable Start() { return UnitTestApplication.Start(TestServices.StyledWindow); From b42458be76891a08968f0c9fc516889a4e0dd24f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 3 Apr 2020 23:19:42 +0200 Subject: [PATCH 04/11] Input manager subscription may be null. As we accept a null `InputManager` for unit testing. --- src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index bf811ad008..3715bc52a4 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -96,7 +96,7 @@ namespace Avalonia.Controls.Platform root.Deactivated -= WindowDeactivated; } - _inputManagerSubscription!.Dispose(); + _inputManagerSubscription?.Dispose(); Menu = null; _root = null; From 76b4d17abf0de768fc9504fc1b8ef9967466429c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 3 Apr 2020 23:21:06 +0200 Subject: [PATCH 05/11] Remove ContextMenu from logical tree. When the `ContextMenu` is not attached to a control, i.e. it is shown using `Show(control)` then we need to detach it from the logical tree once hidden otherwise it will leak, causing #3738. --- src/Avalonia.Controls/ContextMenu.cs | 41 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index fc56e26add..0ee906fb55 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -20,6 +20,7 @@ namespace Avalonia.Controls private static readonly ITemplate DefaultPanel = new FuncTemplate(() => new StackPanel { Orientation = Orientation.Vertical }); private Popup _popup; + private bool _attachedToControl; /// /// Initializes a new instance of the class. @@ -69,13 +70,15 @@ namespace Avalonia.Controls { var control = (Control)e.Sender; - if (e.OldValue != null) + if (e.OldValue is ContextMenu oldMenu) { control.PointerReleased -= ControlPointerReleased; + oldMenu._attachedToControl = false; } - if (e.NewValue != null) + if (e.NewValue is ContextMenu newMenu) { + newMenu._attachedToControl = true; control.PointerReleased += ControlPointerReleased; } } @@ -145,18 +148,6 @@ namespace Avalonia.Controls return new MenuItemContainerGenerator(this); } - private void CloseCore() - { - SelectedIndex = -1; - IsOpen = false; - - RaiseEvent(new RoutedEventArgs - { - RoutedEvent = MenuClosedEvent, - Source = this, - }); - } - private void PopupOpened(object sender, EventArgs e) { Focus(); @@ -164,17 +155,27 @@ namespace Avalonia.Controls private void PopupClosed(object sender, EventArgs e) { - var contextMenu = (sender as Popup)?.Child as ContextMenu; - - if (contextMenu != null) + foreach (var i in LogicalChildren) { - foreach (var i in contextMenu.GetLogicalChildren().OfType()) + if (i is MenuItem menuItem) { - i.IsSubMenuOpen = false; + menuItem.IsSubMenuOpen = false; } + } + + SelectedIndex = -1; + IsOpen = false; - contextMenu.CloseCore(); + if (!_attachedToControl) + { + ((ISetLogicalParent)_popup).SetParent(null); } + + RaiseEvent(new RoutedEventArgs + { + RoutedEvent = MenuClosedEvent, + Source = this, + }); } private static void ControlPointerReleased(object sender, PointerReleasedEventArgs e) From 323cfdca3a9014aa8ef2ded007c6031350477a46 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 4 Apr 2020 00:23:45 +0200 Subject: [PATCH 06/11] Make ContextMenu leak tests fail again. By adding focus/input manager. --- src/Avalonia.Controls/WindowBase.cs | 2 +- tests/Avalonia.LeakTests/ControlTests.cs | 10 +++++++++- tests/Avalonia.UnitTests/MockWindowingPlatform.cs | 4 ++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index 196110edf7..63eabb32f4 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -255,7 +255,7 @@ namespace Avalonia.Controls if (scope != null) { - FocusManager.Instance.SetFocusScope(scope); + FocusManager.Instance?.SetFocusScope(scope); } IsActive = true; diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index de3f0f8189..f93b763738 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -5,6 +5,7 @@ using System.Runtime.Remoting.Contexts; using Avalonia.Controls; using Avalonia.Controls.Templates; using Avalonia.Diagnostics; +using Avalonia.Input; using Avalonia.Layout; using Avalonia.Media; using Avalonia.Platform; @@ -441,6 +442,10 @@ namespace Avalonia.LeakTests } var window = new Window(); + window.Show(); + + Assert.Same(window, FocusManager.Instance.Current); + BuildAndShowContextMenu(window); BuildAndShowContextMenu(window); @@ -453,7 +458,10 @@ namespace Avalonia.LeakTests private IDisposable Start() { - return UnitTestApplication.Start(TestServices.StyledWindow); + return UnitTestApplication.Start(TestServices.StyledWindow.With( + focusManager: new FocusManager(), + keyboardDevice: () => new KeyboardDevice(), + inputManager: new InputManager())); } private class Node diff --git a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs index a6701ef655..782e4a0974 100644 --- a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs +++ b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs @@ -21,6 +21,10 @@ namespace Avalonia.UnitTests { var win = Mock.Of(x => x.Scaling == 1); var mock = Mock.Get(win); + mock.Setup(x => x.Show()).Callback(() => + { + mock.Object.Activated?.Invoke(); + }); mock.Setup(x => x.CreatePopup()).Returns(() => { if (popupImpl != null) From 1cb7d830c78c50385cdc4130cdec4d68b91bb99d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 4 Apr 2020 00:32:08 +0200 Subject: [PATCH 07/11] Hack to reset focus when context menu closed. Including a fix to `FocusManager` to ensure that it doesn't try to set focus to an invisble focus scope. Fixes #3738. --- src/Avalonia.Controls/ContextMenu.cs | 5 +++++ src/Avalonia.Input/FocusManager.cs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 0ee906fb55..61e4b69c9b 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -21,6 +21,7 @@ namespace Avalonia.Controls new FuncTemplate(() => new StackPanel { Orientation = Orientation.Vertical }); private Popup _popup; private bool _attachedToControl; + private IInputElement _previousFocus; /// /// Initializes a new instance of the class. @@ -150,6 +151,7 @@ namespace Avalonia.Controls private void PopupOpened(object sender, EventArgs e) { + _previousFocus = FocusManager.Instance?.Current; Focus(); } @@ -171,6 +173,9 @@ namespace Avalonia.Controls ((ISetLogicalParent)_popup).SetParent(null); } + // HACK: Reset the focus when the popup is closed. We need to fix this so it's automatic. + FocusManager.Instance?.Focus(_previousFocus); + RaiseEvent(new RoutedEventArgs { RoutedEvent = MenuClosedEvent, diff --git a/src/Avalonia.Input/FocusManager.cs b/src/Avalonia.Input/FocusManager.cs index 500677c545..bcae8a3c53 100644 --- a/src/Avalonia.Input/FocusManager.cs +++ b/src/Avalonia.Input/FocusManager.cs @@ -168,7 +168,7 @@ namespace Avalonia.Input { var scope = control as IFocusScope; - if (scope != null) + if (scope != null && control.VisualRoot?.IsVisible == true) { yield return scope; } From c4da0f75b805a3c9aa6c7dfc78c2c7ea95077678 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 4 Apr 2020 19:41:54 +0200 Subject: [PATCH 08/11] Added another failing ContextMenu leak test. --- tests/Avalonia.LeakTests/ControlTests.cs | 38 +++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index f93b763738..0afb2465ee 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -422,7 +422,43 @@ namespace Avalonia.LeakTests } [Fact] - public void Context_MenuItems_Are_Freed() + public void Attached_ContextMenu_Is_Freed() + { + using (Start()) + { + void AttachShowAndDetachContextMenu(Control control) + { + var contextMenu = new ContextMenu + { + Items = new[] + { + new MenuItem { Header = "Foo" }, + new MenuItem { Header = "Foo" }, + } + }; + + control.ContextMenu = contextMenu; + contextMenu.Open(control); + contextMenu.Close(); + control.ContextMenu = null; + } + + var window = new Window(); + window.Show(); + + Assert.Same(window, FocusManager.Instance.Current); + + AttachShowAndDetachContextMenu(window); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + + [Fact] + public void Standalone_ContextMenu_Is_Freed() { using (Start()) { From c4433369281c6aaed7d2b7278dc07f5fb76764bc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 4 Apr 2020 19:48:06 +0200 Subject: [PATCH 09/11] Fix leaks in attached context menu, --- src/Avalonia.Controls/ContextMenu.cs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 61e4b69c9b..bb9b853c28 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -20,7 +20,7 @@ namespace Avalonia.Controls private static readonly ITemplate DefaultPanel = new FuncTemplate(() => new StackPanel { Orientation = Orientation.Vertical }); private Popup _popup; - private bool _attachedToControl; + private Control _attachedControl; private IInputElement _previousFocus; /// @@ -74,12 +74,13 @@ namespace Avalonia.Controls if (e.OldValue is ContextMenu oldMenu) { control.PointerReleased -= ControlPointerReleased; - oldMenu._attachedToControl = false; + oldMenu._attachedControl = null; + ((ISetLogicalParent)oldMenu._popup).SetParent(null); } if (e.NewValue is ContextMenu newMenu) { - newMenu._attachedToControl = true; + newMenu._attachedControl = control; control.PointerReleased += ControlPointerReleased; } } @@ -95,8 +96,18 @@ namespace Avalonia.Controls /// The control. public void Open(Control control) { - if (control == null) + if (control is null && _attachedControl is null) + { throw new ArgumentNullException(nameof(control)); + } + + if (control is object && _attachedControl is object && control != _attachedControl) + { + throw new ArgumentException( + "Cannot show ContentMenu on a different control to the one it is attached to.", + nameof(control)); + } + if (IsOpen) { return; @@ -168,7 +179,7 @@ namespace Avalonia.Controls SelectedIndex = -1; IsOpen = false; - if (!_attachedToControl) + if (_attachedControl is null) { ((ISetLogicalParent)_popup).SetParent(null); } From bc36330bc01615bcfc99736ee6d5a2ed1affaef3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 5 Apr 2020 17:19:37 +0200 Subject: [PATCH 10/11] Added failing ContextMenu test. Due to NRE introduced in #3745. --- .../ContextMenuTests.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs b/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs index f44f89e91f..5a47a86e51 100644 --- a/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs @@ -1,9 +1,5 @@ using System; -using System.Windows.Input; -using Avalonia.Controls.Primitives; -using Avalonia.Data; using Avalonia.Input; -using Avalonia.Markup.Data; using Avalonia.Platform; using Avalonia.UnitTests; using Moq; @@ -159,6 +155,19 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Can_Set_Clear_ContextMenu_Property() + { + using (Application()) + { + var target = new ContextMenu(); + var control = new Panel(); + + control.ContextMenu = target; + control.ContextMenu = null; + } + } + [Fact(Skip = "The only reason this test was 'passing' before was that the author forgot to call Window.ApplyTemplate()")] public void Cancelling_Closing_Leaves_ContextMenuOpen() { From 5ad8c41f37a593f0d3861e5308fb45201f96fa4f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 5 Apr 2020 17:19:58 +0200 Subject: [PATCH 11/11] Fix NRE in ContextMenu. --- src/Avalonia.Controls/ContextMenu.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index bb9b853c28..1735599988 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -75,7 +75,7 @@ namespace Avalonia.Controls { control.PointerReleased -= ControlPointerReleased; oldMenu._attachedControl = null; - ((ISetLogicalParent)oldMenu._popup).SetParent(null); + ((ISetLogicalParent)oldMenu._popup)?.SetParent(null); } if (e.NewValue is ContextMenu newMenu)