From 1c258e768091c83e60c61fae520b218d551c7aa1 Mon Sep 17 00:00:00 2001 From: Denis Zubritskiy Date: Mon, 5 Jun 2023 12:18:56 +0300 Subject: [PATCH 01/11] Fix expression in CreateGetter --- .../Data/Core/ClrPropertyInfo.cs | 3 ++- .../Data/ReflectionClrPropertyInfoTests.cs | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 tests/Avalonia.Base.UnitTests/Data/ReflectionClrPropertyInfoTests.cs diff --git a/src/Avalonia.Base/Data/Core/ClrPropertyInfo.cs b/src/Avalonia.Base/Data/Core/ClrPropertyInfo.cs index 24149c17e0..6027676501 100644 --- a/src/Avalonia.Base/Data/Core/ClrPropertyInfo.cs +++ b/src/Avalonia.Base/Data/Core/ClrPropertyInfo.cs @@ -60,7 +60,8 @@ namespace Avalonia.Data.Core var target = Expression.Parameter(typeof(object), "target"); return Expression.Lambda>( Expression.Convert(Expression.Call(Expression.Convert(target, info.DeclaringType!), info.GetMethod), - typeof(object))) + typeof(object)), + target) .Compile(); } diff --git a/tests/Avalonia.Base.UnitTests/Data/ReflectionClrPropertyInfoTests.cs b/tests/Avalonia.Base.UnitTests/Data/ReflectionClrPropertyInfoTests.cs new file mode 100644 index 0000000000..5c51dd0c06 --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Data/ReflectionClrPropertyInfoTests.cs @@ -0,0 +1,24 @@ +using Avalonia.Data.Core; +using Xunit; + +namespace Avalonia.Base.UnitTests.Data; + +public class ReflectionClrPropertyInfoTests +{ + public class TestClass + { + public string Test { get; set; } + } + + [Fact] + public void Can_Compile() + { + var propertyInfo = new ReflectionClrPropertyInfo( + typeof(TestClass).GetProperty(nameof(TestClass.Test))!); + var target = new TestClass(); + const string result = "qwerty"; + propertyInfo.Set(target, result); + Assert.Equal(result, target.Test); + Assert.Equal(result, (string)propertyInfo.Get(target)); + } +} From c5d8715e1e049738754a040b74450279a0b7f4bc Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Fri, 9 Jun 2023 08:38:37 +0200 Subject: [PATCH 02/11] Rework Imm32InputMethod WM handling --- .../Avalonia.Win32/Input/Imm32InputMethod.cs | 83 +++++++++++++++++++ .../Avalonia.Win32/WindowImpl.AppWndProc.cs | 56 ++----------- src/Windows/Avalonia.Win32/WindowImpl.cs | 2 +- 3 files changed, 93 insertions(+), 48 deletions(-) diff --git a/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs b/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs index aabf361844..4c8299bf8f 100644 --- a/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs +++ b/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs @@ -1,6 +1,8 @@ using System; using System.Diagnostics.CodeAnalysis; using System.Text; +using Avalonia.Input; +using Avalonia.Input.Raw; using Avalonia.Input.TextInput; using Avalonia.Threading; @@ -297,6 +299,87 @@ namespace Avalonia.Win32.Input return ImmGetCompositionString(himc, flag); } + public void HandleCompositionStart() + { + Composition = null; + + if (IsActive) + { + Client.SetPreeditText(null); + } + + IsComposing = true; + } + + public void HandleCompositionEnd(WindowImpl windowImpl, uint timestamp) + { + var currentComposition = Composition; + + //In case composition has not been comitted yet we need to do that here. + if (!string.IsNullOrEmpty(currentComposition)) + { + var e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, windowImpl.Owner, currentComposition); + + if(windowImpl.Input != null) + { + windowImpl.Input(e); + } + } + + //Cleanup composition state. + IsComposing = false; + Composition = null; + + if (IsActive) + { + Client.SetPreeditText(null); + } + } + + public void HandleComposition(WindowImpl windowImpl, IntPtr wParam, IntPtr lParam, uint timestamp, ref bool ignoreWmChar) + { + var flags = (GCS)ToInt32(lParam); + + if ((flags & GCS.GCS_RESULTSTR) != 0) + { + var resultString = GetCompositionString(GCS.GCS_RESULTSTR); + + if (!string.IsNullOrEmpty(resultString)) + { + Composition = null; + + if (IsActive) + { + Client.SetPreeditText(null); + } + + var e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, windowImpl.Owner, resultString); + + if(windowImpl.Input != null) + { + windowImpl.Input(e); + + ignoreWmChar = true; + } + } + } + + if ((flags & GCS.GCS_COMPSTR) != 0) + { + var compositionString = GetCompositionString(GCS.GCS_COMPSTR); + + CompositionChanged(compositionString); + } + } + + private static int ToInt32(IntPtr ptr) + { + if (IntPtr.Size == 4) + return ptr.ToInt32(); + + return (int)(ptr.ToInt64() & 0xffffffff); + } + ~Imm32InputMethod() { _caretManager.TryDestroy(); diff --git a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs index b256a9433d..252410264e 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs @@ -87,7 +87,7 @@ namespace Avalonia.Win32 { // The first and foremost thing to do - notify the TopLevel Closed?.Invoke(); - + if (UiaCoreTypesApi.IsNetComInteropAvailable) { UiaCoreProviderApi.UiaReturnRawElementProvider(_hwnd, IntPtr.Zero, IntPtr.Zero, null); @@ -98,7 +98,7 @@ namespace Avalonia.Win32 { Imm32InputMethod.Current.ClearLanguageAndWindow(); } - + // Cleanup render targets (_gl as IDisposable)?.Dispose(); @@ -724,26 +724,7 @@ namespace Avalonia.Win32 } case WindowsMessage.WM_IME_COMPOSITION: { - var flags = (GCS)ToInt32(lParam); - - if ((flags & GCS.GCS_COMPSTR) != 0) - { - var currentComposition = Imm32InputMethod.Current.GetCompositionString(GCS.GCS_COMPSTR); - - Imm32InputMethod.Current.CompositionChanged(currentComposition); - } - - if ((flags & GCS.GCS_RESULTSTR) != 0) - { - var result = Imm32InputMethod.Current.GetCompositionString(GCS.GCS_RESULTSTR); - - if (!string.IsNullOrEmpty(result)) - { - Imm32InputMethod.Current.Composition = result; - - _ignoreWmChar = true; - } - } + Imm32InputMethod.Current.HandleComposition(this, wParam, lParam, timestamp, ref _ignoreWmChar); break; } @@ -757,35 +738,16 @@ namespace Avalonia.Win32 case WindowsMessage.WM_IME_NOTIFY: break; case WindowsMessage.WM_IME_STARTCOMPOSITION: - Imm32InputMethod.Current.Composition = null; - - if (Imm32InputMethod.Current.IsActive) { - Imm32InputMethod.Current.Client.SetPreeditText(null); - } + Imm32InputMethod.Current.HandleCompositionStart(); - Imm32InputMethod.Current.IsComposing = true; - return IntPtr.Zero; + return IntPtr.Zero; + } case WindowsMessage.WM_IME_ENDCOMPOSITION: { - var currentComposition = Imm32InputMethod.Current.Composition; - - //In case composition has not been comitted yet we need to do that here. - if (!string.IsNullOrEmpty(currentComposition)) - { - e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, Owner, currentComposition); - } - - //Cleanup composition state. - Imm32InputMethod.Current.IsComposing = false; - Imm32InputMethod.Current.Composition = null; + Imm32InputMethod.Current.HandleCompositionEnd(this, timestamp); - if (Imm32InputMethod.Current.IsActive) - { - Imm32InputMethod.Current.Client.SetPreeditText(null); - } - - break; + return IntPtr.Zero; } case WindowsMessage.WM_GETOBJECT: if ((long)lParam == uiaRootObjectId && UiaCoreTypesApi.IsNetComInteropAvailable && _owner is Control control) @@ -830,7 +792,7 @@ namespace Avalonia.Win32 return IntPtr.Zero; } } - + return DefWindowProc(hWnd, msg, wParam, lParam); } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index 4260f90e9f..81257667af 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -186,7 +186,7 @@ namespace Avalonia.Win32 s_instances.Add(this); } - private IInputRoot Owner + internal IInputRoot Owner => _owner ?? throw new InvalidOperationException($"{nameof(SetInputRoot)} must have been called"); public Action? Activated { get; set; } From 281979d80ddcf7d731843f328f1e48ecad049a39 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Fri, 9 Jun 2023 09:32:28 +0200 Subject: [PATCH 03/11] Properly reset IMM32 state --- .../Input/TextInput/InputMethodManager.cs | 10 ++- .../Avalonia.Win32/Input/Imm32InputMethod.cs | 61 +++++++++++-------- .../Avalonia.Win32/WindowImpl.AppWndProc.cs | 4 +- src/Windows/Avalonia.Win32/WindowImpl.cs | 2 +- 4 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/Avalonia.Base/Input/TextInput/InputMethodManager.cs b/src/Avalonia.Base/Input/TextInput/InputMethodManager.cs index 1c61334888..c7fca04ea9 100644 --- a/src/Avalonia.Base/Input/TextInput/InputMethodManager.cs +++ b/src/Avalonia.Base/Input/TextInput/InputMethodManager.cs @@ -22,11 +22,18 @@ namespace Avalonia.Input.TextInput set { if(_client == value) + { return; + } + if (_client != null) { _client.CursorRectangleChanged -= OnCursorRectangleChanged; _client.TextViewVisualChanged -= OnTextViewVisualChanged; + + _client = null; + + _im?.Reset(); } _client = value; @@ -35,8 +42,6 @@ namespace Avalonia.Input.TextInput { _client.CursorRectangleChanged += OnCursorRectangleChanged; _client.TextViewVisualChanged += OnTextViewVisualChanged; - - _im?.Reset(); if (_focusedElement is StyledElement target) { @@ -50,6 +55,7 @@ namespace Avalonia.Input.TextInput _transformTracker.SetVisual(_client?.TextViewVisual); _im?.SetClient(_client); + UpdateCursorRect(); } else diff --git a/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs b/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs index 4c8299bf8f..05074cc82e 100644 --- a/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs +++ b/src/Windows/Avalonia.Win32/Input/Imm32InputMethod.cs @@ -24,6 +24,8 @@ namespace Avalonia.Win32.Input private ushort _langId; private const int CaretMargin = 1; + private bool _ignoreComposition; + public ITextInputMethodClient? Client { get; private set; } [MemberNotNullWhen(true, nameof(Client))] @@ -123,19 +125,35 @@ namespace Avalonia.Win32.Input { var himc = ImmGetContext(Hwnd); - if (IsComposing) + if (himc != IntPtr.Zero) { + _ignoreComposition = true; + + if (_parent != null) + { + _parent._ignoreWmChar = true; + } + ImmNotifyIME(himc, NI_COMPOSITIONSTR, CPS_COMPLETE, 0); - + + ImmReleaseContext(Hwnd, himc); + IsComposing = false; - } - ImmReleaseContext(Hwnd, himc); + Composition = null; + } }); } public void SetClient(ITextInputMethodClient? client) { + if(Client != null) + { + Composition = null; + + Client.SetPreeditText(null); + } + Client = client; Dispatcher.UIThread.Post(() => @@ -311,23 +329,11 @@ namespace Avalonia.Win32.Input IsComposing = true; } - public void HandleCompositionEnd(WindowImpl windowImpl, uint timestamp) + public void HandleCompositionEnd() { - var currentComposition = Composition; - - //In case composition has not been comitted yet we need to do that here. - if (!string.IsNullOrEmpty(currentComposition)) - { - var e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, windowImpl.Owner, currentComposition); - - if(windowImpl.Input != null) - { - windowImpl.Input(e); - } - } - //Cleanup composition state. IsComposing = false; + Composition = null; if (IsActive) @@ -336,15 +342,22 @@ namespace Avalonia.Win32.Input } } - public void HandleComposition(WindowImpl windowImpl, IntPtr wParam, IntPtr lParam, uint timestamp, ref bool ignoreWmChar) + public void HandleComposition(IntPtr wParam, IntPtr lParam, uint timestamp) { + if (_ignoreComposition) + { + _ignoreComposition = false; + + return; + } + var flags = (GCS)ToInt32(lParam); if ((flags & GCS.GCS_RESULTSTR) != 0) { var resultString = GetCompositionString(GCS.GCS_RESULTSTR); - if (!string.IsNullOrEmpty(resultString)) + if (_parent != null && !string.IsNullOrEmpty(resultString)) { Composition = null; @@ -353,13 +366,13 @@ namespace Avalonia.Win32.Input Client.SetPreeditText(null); } - var e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, windowImpl.Owner, resultString); + var e = new RawTextInputEventArgs(WindowsKeyboardDevice.Instance, timestamp, _parent.Owner, resultString); - if(windowImpl.Input != null) + if (_parent.Input != null) { - windowImpl.Input(e); + _parent.Input(e); - ignoreWmChar = true; + _parent._ignoreWmChar = true; } } } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs index 252410264e..9a7f4b62e3 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs @@ -724,7 +724,7 @@ namespace Avalonia.Win32 } case WindowsMessage.WM_IME_COMPOSITION: { - Imm32InputMethod.Current.HandleComposition(this, wParam, lParam, timestamp, ref _ignoreWmChar); + Imm32InputMethod.Current.HandleComposition(wParam, lParam, timestamp); break; } @@ -745,7 +745,7 @@ namespace Avalonia.Win32 } case WindowsMessage.WM_IME_ENDCOMPOSITION: { - Imm32InputMethod.Current.HandleCompositionEnd(this, timestamp); + Imm32InputMethod.Current.HandleCompositionEnd(); return IntPtr.Zero; } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index 81257667af..057cdb2db0 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -97,7 +97,7 @@ namespace Avalonia.Win32 private bool _shown; private bool _hiddenWindowIsParent; private uint _langid; - private bool _ignoreWmChar; + internal bool _ignoreWmChar; private WindowTransparencyLevel _transparencyLevel; private const int MaxPointerHistorySize = 512; From 26d47460967501bebb0790dd68e6576919b5e439 Mon Sep 17 00:00:00 2001 From: jankrib Date: Tue, 13 Jun 2023 13:35:13 +0200 Subject: [PATCH 04/11] Break layout flip flop in headless --- .../Avalonia.Headless/HeadlessWindowImpl.cs | 13 ++++++---- .../RenderingTests.cs | 24 ++++++++++++++++++- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/Headless/Avalonia.Headless/HeadlessWindowImpl.cs b/src/Headless/Avalonia.Headless/HeadlessWindowImpl.cs index 84aca6e94f..c7eb07ba10 100644 --- a/src/Headless/Avalonia.Headless/HeadlessWindowImpl.cs +++ b/src/Headless/Avalonia.Headless/HeadlessWindowImpl.cs @@ -114,23 +114,26 @@ namespace Avalonia.Headless public Size MaxClientSize { get; } = new Size(1920, 1280); public void Resize(Size clientSize, WindowResizeReason reason) { + if (ClientSize == clientSize) + return; + // Emulate X11 behavior here if (IsPopup) - DoResize(clientSize); + DoResize(clientSize, reason); else Dispatcher.UIThread.Post(() => { - DoResize(clientSize); - }); + DoResize(clientSize, reason); + }, DispatcherPriority.Send); } - private void DoResize(Size clientSize) + private void DoResize(Size clientSize, WindowResizeReason reason) { // Uncomment this check and experience a weird bug in layout engine if (ClientSize != clientSize) { ClientSize = clientSize; - Resized?.Invoke(clientSize, WindowResizeReason.Unspecified); + Resized?.Invoke(clientSize, reason); } } diff --git a/tests/Avalonia.Headless.UnitTests/RenderingTests.cs b/tests/Avalonia.Headless.UnitTests/RenderingTests.cs index 3f45bf97e4..cda459e7f9 100644 --- a/tests/Avalonia.Headless.UnitTests/RenderingTests.cs +++ b/tests/Avalonia.Headless.UnitTests/RenderingTests.cs @@ -1,4 +1,5 @@ -using Avalonia.Controls; +using System.Collections.ObjectModel; +using Avalonia.Controls; using Avalonia.Layout; using Avalonia.Media; using Avalonia.Threading; @@ -24,6 +25,27 @@ public class RenderingTests Content = new PathIcon { Data = StreamGeometry.Parse("M0,9 L10,0 20,9 19,10 10,2 1,10 z") +#if NUNIT + [AvaloniaTest, Timeout(10000)] +#elif XUNIT + [AvaloniaFact(Timeout = 10000)] +#endif + public void Should_Not_Hang_With_Non_Trivial_Layout() + { + var window = new Window + { + Content = new ContentControl + { + HorizontalAlignment = HorizontalAlignment.Stretch, + VerticalAlignment = VerticalAlignment.Stretch, + Padding = new Thickness(1), + Content = new ListBox + { + ItemsSource = new ObservableCollection() + { + "Test 1", + "Test 2" + } } }, SizeToContent = SizeToContent.WidthAndHeight From 682b6746258ded4b13bcb46560b0e2b4f6078890 Mon Sep 17 00:00:00 2001 From: jankrib Date: Tue, 13 Jun 2023 13:47:22 +0200 Subject: [PATCH 05/11] Fix merge issue in test --- tests/Avalonia.Headless.UnitTests/RenderingTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Avalonia.Headless.UnitTests/RenderingTests.cs b/tests/Avalonia.Headless.UnitTests/RenderingTests.cs index cda459e7f9..591863bcdb 100644 --- a/tests/Avalonia.Headless.UnitTests/RenderingTests.cs +++ b/tests/Avalonia.Headless.UnitTests/RenderingTests.cs @@ -25,6 +25,18 @@ public class RenderingTests Content = new PathIcon { Data = StreamGeometry.Parse("M0,9 L10,0 20,9 19,10 10,2 1,10 z") + } + }, + SizeToContent = SizeToContent.WidthAndHeight + }; + + window.Show(); + + var frame = window.CaptureRenderedFrame(); + + Assert.NotNull(frame); + } + #if NUNIT [AvaloniaTest, Timeout(10000)] #elif XUNIT @@ -50,6 +62,7 @@ public class RenderingTests }, SizeToContent = SizeToContent.WidthAndHeight }; + window.Show(); var frame = window.CaptureRenderedFrame(); From dd764e8c438c38c561200a9dee97c74c00add1a6 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Wed, 14 Jun 2023 06:19:31 +0200 Subject: [PATCH 06/11] Prevent infinite loop if the default FontFamily lookup fails --- src/Avalonia.Base/Media/FontManager.cs | 7 ++++++- .../Media/FontManagerTests.cs | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Media/FontManager.cs b/src/Avalonia.Base/Media/FontManager.cs index 4425147098..17d1984286 100644 --- a/src/Avalonia.Base/Media/FontManager.cs +++ b/src/Avalonia.Base/Media/FontManager.cs @@ -151,8 +151,13 @@ namespace Avalonia.Media } } + if(typeface.FontFamily == DefaultFontFamily) + { + return false; + } + //Nothing was found so use the default - return TryGetGlyphTypeface(new Typeface(DefaultFontFamily, typeface.Style, typeface.Weight, typeface.Stretch), out glyphTypeface); + return TryGetGlyphTypeface(new Typeface(FontFamily.DefaultFontFamilyName, typeface.Style, typeface.Weight, typeface.Stretch), out glyphTypeface); } /// diff --git a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs index 62f1cd2032..f4add27a55 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs @@ -123,5 +123,21 @@ namespace Avalonia.Skia.UnitTests.Media } } } + + [Fact] + public void Should_Return_False_For_Invalid_DefaultFontFamily() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface.With(fontManagerImpl: new FontManagerImpl()))) + { + using (AvaloniaLocator.EnterScope()) + { + AvaloniaLocator.CurrentMutable.BindToSelf(new FontManagerOptions { DefaultFamilyName = "ABC" }); + + var result = FontManager.Current.TryGetGlyphTypeface(Typeface.Default, out _); + + Assert.False(result); + } + } + } } } From 0ff67a4bb83ca4f19f3d8390d72d00c65d1f3e67 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Wed, 14 Jun 2023 07:30:38 +0200 Subject: [PATCH 07/11] Don't use system font lookup for the test --- tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs index f4add27a55..0818510bc2 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs @@ -131,7 +131,7 @@ namespace Avalonia.Skia.UnitTests.Media { using (AvaloniaLocator.EnterScope()) { - AvaloniaLocator.CurrentMutable.BindToSelf(new FontManagerOptions { DefaultFamilyName = "ABC" }); + AvaloniaLocator.CurrentMutable.BindToSelf(new FontManagerOptions { DefaultFamilyName = "avares://resm:Avalonia.Skia.UnitTests.Assets?assembly=Avalonia.Skia.UnitTests#Unknown" }); var result = FontManager.Current.TryGetGlyphTypeface(Typeface.Default, out _); From e9f655212fb1d0fd828c9b6ac62e233cdebd4bba Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Wed, 14 Jun 2023 10:05:13 +0200 Subject: [PATCH 08/11] Use a blocking collection for SKTextBlob caching --- src/Skia/Avalonia.Skia/GlyphRunImpl.cs | 35 ++++++++++++-------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/Skia/Avalonia.Skia/GlyphRunImpl.cs b/src/Skia/Avalonia.Skia/GlyphRunImpl.cs index f6d84f0b12..fdb9d0b031 100644 --- a/src/Skia/Avalonia.Skia/GlyphRunImpl.cs +++ b/src/Skia/Avalonia.Skia/GlyphRunImpl.cs @@ -1,5 +1,6 @@ using System; using System.Buffers; +using System.Collections.Concurrent; using System.Collections.Generic; using Avalonia.Media; using Avalonia.Media.TextFormatting; @@ -14,7 +15,7 @@ namespace Avalonia.Skia private readonly ushort[] _glyphIndices; private readonly SKPoint[] _glyphPositions; - private readonly Dictionary _textBlobCache = new(1); + private readonly ConcurrentDictionary _textBlobCache = new(); public GlyphRunImpl(IGlyphTypeface glyphTypeface, double fontRenderingEmSize, IReadOnlyList glyphInfos, Point baselineOrigin) @@ -100,32 +101,28 @@ namespace Avalonia.Skia break; } - if (_textBlobCache.TryGetValue(edging, out var textBlob)) + return _textBlobCache.GetOrAdd(edging, (_) => { - return textBlob; - } - - var font = _glyphTypefaceImpl.SKFont; + var font = _glyphTypefaceImpl.SKFont; - font.Hinting = SKFontHinting.Full; - font.Subpixel = edging == SKFontEdging.SubpixelAntialias; - font.Edging = edging; - font.Size = (float)FontRenderingEmSize; + font.Hinting = SKFontHinting.Full; + font.Subpixel = edging == SKFontEdging.SubpixelAntialias; + font.Edging = edging; + font.Size = (float)FontRenderingEmSize; - var builder = SKTextBlobBuilderCache.Shared.Get(); + var builder = SKTextBlobBuilderCache.Shared.Get(); - var runBuffer = builder.AllocatePositionedRun(font, _glyphIndices.Length); + var runBuffer = builder.AllocatePositionedRun(font, _glyphIndices.Length); - runBuffer.SetPositions(_glyphPositions); - runBuffer.SetGlyphs(_glyphIndices); + runBuffer.SetPositions(_glyphPositions); + runBuffer.SetGlyphs(_glyphIndices); - textBlob = builder.Build(); + var textBlob = builder.Build(); - SKTextBlobBuilderCache.Shared.Return(builder); + SKTextBlobBuilderCache.Shared.Return(builder); - _textBlobCache.Add(edging, textBlob); - - return textBlob; + return textBlob; + }); } public void Dispose() From 8e9532a580b41bfc71cec7a7dd422a0eba1b71f8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 15 Jun 2023 11:28:21 +0200 Subject: [PATCH 09/11] Added tests for AccessKeyHandler. Some failing: tests are chosen to match UWP behavior, not WPF (which hides key press events for access keys). --- .../Input/AccessKeyHandlerTests.cs | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 tests/Avalonia.Base.UnitTests/Input/AccessKeyHandlerTests.cs diff --git a/tests/Avalonia.Base.UnitTests/Input/AccessKeyHandlerTests.cs b/tests/Avalonia.Base.UnitTests/Input/AccessKeyHandlerTests.cs new file mode 100644 index 0000000000..4d8ece4dda --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Input/AccessKeyHandlerTests.cs @@ -0,0 +1,205 @@ +using System.Collections.Generic; +using Avalonia.Controls; +using Avalonia.Input; +using Avalonia.UnitTests; +using Moq; +using Xunit; + +namespace Avalonia.Base.UnitTests.Input +{ + public class AccessKeyHandlerTests + { + [Fact] + public void Should_Raise_Key_Events_For_Unregistered_Access_Key() + { + var root = new TestRoot(); + var target = new AccessKeyHandler(); + var events = new List(); + + target.SetOwner(root); + root.KeyDown += (s, e) => events.Add($"KeyDown {e.Key}"); + root.KeyUp += (s, e) => events.Add($"KeyUp {e.Key}"); + + KeyDown(root, Key.LeftAlt); + KeyDown(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.LeftAlt); + + Assert.Equal(new[] + { + "KeyDown LeftAlt", + "KeyDown A", + "KeyUp A", + "KeyUp LeftAlt", + }, events); + } + + [Fact] + public void Should_Raise_Key_Events_For_Unregistered_Access_Key_With_MainMenu() + { + var root = new TestRoot(); + var target = new AccessKeyHandler(); + var menu = Mock.Of(); + var events = new List(); + + target.SetOwner(root); + target.MainMenu = menu; + root.KeyDown += (s, e) => events.Add($"KeyDown {e.Key}"); + root.KeyUp += (s, e) => events.Add($"KeyUp {e.Key}"); + + KeyDown(root, Key.LeftAlt); + KeyDown(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.LeftAlt); + + Assert.Equal(new[] + { + "KeyDown LeftAlt", + "KeyDown A", + "KeyUp A", + "KeyUp LeftAlt", + }, events); + } + + [Fact] + public void Should_Raise_Key_Events_For_Alt_Key() + { + var root = new TestRoot(); + var target = new AccessKeyHandler(); + var events = new List(); + + target.SetOwner(root); + root.KeyDown += (s, e) => events.Add($"KeyDown {e.Key}"); + root.KeyUp += (s, e) => events.Add($"KeyUp {e.Key}"); + + KeyDown(root, Key.LeftAlt); + KeyUp(root, Key.LeftAlt); + + Assert.Equal(new[] + { + "KeyDown LeftAlt", + "KeyUp LeftAlt", + }, events); + } + + [Fact] + public void Should_Raise_Key_Events_For_Alt_Key_With_MainMenu() + { + var root = new TestRoot(); + var target = new AccessKeyHandler(); + var menu = new Mock(); + var events = new List(); + + menu.SetupAllProperties(); + menu.Setup(x => x.Open()).Callback(() => menu.Setup(x => x.IsOpen).Returns(true)); + + target.SetOwner(root); + target.MainMenu = menu.Object; + + root.KeyDown += (s, e) => events.Add($"KeyDown {e.Key}"); + root.KeyUp += (s, e) => events.Add($"KeyUp {e.Key}"); + + KeyDown(root, Key.LeftAlt); + KeyUp(root, Key.LeftAlt); + KeyDown(root, Key.LeftAlt); + KeyUp(root, Key.LeftAlt); + + Assert.Equal(new[] + { + "KeyDown LeftAlt", + "KeyUp LeftAlt", + "KeyDown LeftAlt", + "KeyUp LeftAlt", + }, events); + } + + [Fact] + public void Should_Raise_Key_Events_For_Registered_Access_Key() + { + var button = new Button(); + var root = new TestRoot(button); + var target = new AccessKeyHandler(); + var events = new List(); + + target.SetOwner(root); + target.Register('A', button); + root.KeyDown += (s, e) => events.Add($"KeyDown {e.Key}"); + root.KeyUp += (s, e) => events.Add($"KeyUp {e.Key}"); + + KeyDown(root, Key.LeftAlt); + KeyDown(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.LeftAlt); + + // This differs from WPF which doesn't raise the `A` key event, but matches UWP. + Assert.Equal(new[] + { + "KeyDown LeftAlt", + "KeyDown A", + "KeyUp A", + "KeyUp LeftAlt", + }, events); + } + + [Fact] + public void Should_Raise_AccessKeyPressed_For_Registered_Access_Key() + { + var button = new Button(); + var root = new TestRoot(button); + var target = new AccessKeyHandler(); + var raised = 0; + + target.SetOwner(root); + target.Register('A', button); + button.AddHandler(AccessKeyHandler.AccessKeyPressedEvent, (s, e) => ++raised); + + KeyDown(root, Key.LeftAlt); + Assert.Equal(0, raised); + + KeyDown(root, Key.A, KeyModifiers.Alt); + Assert.Equal(1, raised); + + KeyUp(root, Key.A, KeyModifiers.Alt); + KeyUp(root, Key.LeftAlt); + + Assert.Equal(1, raised); + } + + [Fact] + public void Should_Open_MainMenu_On_Alt_KeyUp() + { + var root = new TestRoot(); + var target = new AccessKeyHandler(); + var menu = new Mock(); + + target.SetOwner(root); + target.MainMenu = menu.Object; + + KeyDown(root, Key.LeftAlt); + menu.Verify(x => x.Open(), Times.Never); + + KeyUp(root, Key.LeftAlt); + menu.Verify(x => x.Open(), Times.Once); + } + + private static void KeyDown(IInputElement target, Key key, KeyModifiers modifiers = KeyModifiers.None) + { + target.RaiseEvent(new KeyEventArgs + { + RoutedEvent = InputElement.KeyDownEvent, + Key = key, + KeyModifiers = modifiers, + }); + } + + private static void KeyUp(IInputElement target, Key key, KeyModifiers modifiers = KeyModifiers.None) + { + target.RaiseEvent(new KeyEventArgs + { + RoutedEvent = InputElement.KeyUpEvent, + Key = key, + KeyModifiers = modifiers, + }); + } + } +} From ca22dd8783f253fd489fd7c48f485e265922c5f6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 15 Jun 2023 11:50:54 +0200 Subject: [PATCH 10/11] Get platform settings from TopLevel. Allows easier unit testing. --- src/Avalonia.Controls/Control.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Control.cs b/src/Avalonia.Controls/Control.cs index 6e0063c9ec..ae7d4f3fb4 100644 --- a/src/Avalonia.Controls/Control.cs +++ b/src/Avalonia.Controls/Control.cs @@ -480,7 +480,7 @@ namespace Avalonia.Controls if (e.Source == this && !e.Handled) { - var keymap = Application.Current!.PlatformSettings?.HotkeyConfiguration.OpenContextMenu; + var keymap = TopLevel.GetTopLevel(this)?.PlatformSettings?.HotkeyConfiguration.OpenContextMenu; if (keymap is null) { From 001c81d1afb6eb7f74706d50cc073d7dd615acc1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 15 Jun 2023 11:55:41 +0200 Subject: [PATCH 11/11] Don't mark events as handled in AccessKeyHandler. Fixes #11633 --- src/Avalonia.Base/Input/AccessKeyHandler.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Avalonia.Base/Input/AccessKeyHandler.cs b/src/Avalonia.Base/Input/AccessKeyHandler.cs index 2e0268a644..38692d8b77 100644 --- a/src/Avalonia.Base/Input/AccessKeyHandler.cs +++ b/src/Avalonia.Base/Input/AccessKeyHandler.cs @@ -159,8 +159,6 @@ namespace Avalonia.Input _restoreFocusElement?.Focus(); _restoreFocusElement = null; - - e.Handled = true; } } else if (_altIsDown) @@ -200,7 +198,6 @@ namespace Avalonia.Input if (match is not null) { match.RaiseEvent(new RoutedEventArgs(AccessKeyPressedEvent)); - e.Handled = true; } } } @@ -225,7 +222,6 @@ namespace Avalonia.Input else if (_showingAccessKeys && MainMenu != null) { MainMenu.Open(); - e.Handled = true; } break;