feat: Improve Skia TextBoxView overlay positioning and sizing

- Previously we were using LayoutChanged and SizeChanged, but that was not completely reliable. Now we do the invalidation before render, which is much more accurate - To prevent unnecessary operations we short-circuit as soon as possible - only computing when a TextBoxView is actually focused
unoplatform · Nov 28, 2021 · b8240f8 · b8240f8
1 parent ba37217
commit b8240f8
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 77 deletions.
diff --git a/src/Uno.UI.Runtime.Skia.Gtk/UI/Xaml/Controls/TextBoxViewExtension.cs b/src/Uno.UI.Runtime.Skia.Gtk/UI/Xaml/Controls/TextBoxViewExtension.cs
@@ -12,10 +12,14 @@
 using Uno.UI.Runtime.Skia.GTK.UI.Text;
 using GtkWindow = Gtk.Window;
 using Object = GLib.Object;
-using Point = Windows.Foundation.Point;
 using Scale = Pango.Scale;
 using System.Diagnostics;
 using Windows.UI.Xaml.Media;
+using Gdk;
+using Point = Windows.Foundation.Point;
+using GdkPoint = Gdk.Point;
+using Size = Windows.Foundation.Size;
+using GdkSize = Gdk.Size;
 
 namespace Uno.UI.Runtime.Skia.GTK.Extensions.UI.Xaml.Controls
 {
@@ -28,16 +32,19 @@ internal class TextBoxViewExtension : ITextBoxViewExtension
 		private ContentControl? _contentElement;
 		private Widget? _currentInputWidget;
 		private bool _handlingTextChanged;
+		private GdkPoint _lastPosition = new GdkPoint(-1, -1);
+		private GdkSize _lastSize = new GdkSize(-1, -1);
 
 		private readonly SerialDisposable _textChangedDisposable = new SerialDisposable();
-		private readonly SerialDisposable _textBoxEventSubscriptions = new SerialDisposable();
 
 		public TextBoxViewExtension(TextBoxView owner, GtkWindow window)
 		{
 			_owner = owner ?? throw new ArgumentNullException(nameof(owner));
 			_window = window ?? throw new ArgumentNullException(nameof(window));
 		}
 
+		public static TextBoxViewExtension? ActiveTextBoxView { get; private set; }
+
 		private Fixed GetWindowTextInputLayer()
 		{
 			// now we have the GtkEventBox
@@ -58,20 +65,12 @@ public void StartEntry()
 			EnsureWidget(textBox);
 			var textInputLayer = GetWindowTextInputLayer();
 			textInputLayer.Put(_currentInputWidget!, 0, 0);
-
-			textBox.SizeChanged += ContentElementSizeChanged;
-			textBox.LayoutUpdated += ContentElementLayoutUpdated;
-			_textBoxEventSubscriptions.Disposable = Disposable.Create(() =>
-			{
-				textBox.SizeChanged -= ContentElementSizeChanged;
-				textBox.LayoutUpdated -= ContentElementLayoutUpdated;
-			});
-
+			_lastSize = new GdkSize(-1, -1);
+			_lastPosition = new GdkPoint(-1, -1);
 			UpdateNativeView();
 			SetWidgetText(textBox.Text);
 
-			UpdateSize();
-			UpdatePosition();
+			InvalidateLayout();
 
 			textInputLayer.ShowAll();
 			_currentInputWidget!.HasFocus = true;
@@ -85,7 +84,6 @@ public void EndEntry()
 			}
 
 			_contentElement = null;
-			_textBoxEventSubscriptions.Disposable = null;
 
 			if (_currentInputWidget != null)
 			{
@@ -131,6 +129,12 @@ public void UpdateNativeView()
 			}
 		}
 
+		public void InvalidateLayout()
+		{
+			UpdateSize();
+			UpdatePosition();
+		}
+
 		public void UpdateSize()
 		{
 			if (_contentElement == null || _currentInputWidget == null)
@@ -139,9 +143,18 @@ public void UpdateSize()
 			}
 
 			var textInputLayer = GetWindowTextInputLayer();
-			if (textInputLayer.Children.Contains(_currentInputWidget))
+			if (!textInputLayer.Children.Contains(_currentInputWidget))
+			{
+				return;
+			}
+
+			var width = (int)_contentElement.ActualWidth;
+			var height = (int)_contentElement.ActualHeight;
+
+			if (_lastSize.Width != width && _lastSize.Height != height)
 			{
-				_currentInputWidget?.SetSizeRequest((int)_contentElement.ActualWidth, (int)_contentElement.ActualHeight);
+				_lastSize = new GdkSize(width, height);
+				_currentInputWidget?.SetSizeRequest(_lastSize.Width, _lastSize.Height);
 			}
 		}
 
@@ -152,12 +165,21 @@ public void UpdatePosition()
 				return;
 			}
 
+			var textInputLayer = GetWindowTextInputLayer();
+			if (!textInputLayer.Children.Contains(_currentInputWidget))
+			{
+				return;
+			}
+
 			var transformToRoot = _contentElement.TransformToVisual(Windows.UI.Xaml.Window.Current.Content);
 			var point = transformToRoot.TransformPoint(new Point(0, 0));
-			var textInputLayer = GetWindowTextInputLayer();
-			if (textInputLayer.Children.Contains(_currentInputWidget))
+			var pointX = point.X;
+			var pointY = point.Y;
+
+			if (_lastPosition.X != pointX && _lastPosition.Y != pointY)
 			{
-				textInputLayer.Move(_currentInputWidget, (int)point.X, (int)point.Y);
+				_lastPosition = new GdkPoint((int)pointX, (int)pointY);
+				textInputLayer.Move(_currentInputWidget, _lastPosition.X, _lastPosition.Y);
 			}
 		}
 
@@ -271,18 +293,6 @@ private void SetWidgetText(string text)
 			};
 		}
 
-		private void ContentElementLayoutUpdated(object? sender, object e)
-		{
-			UpdateSize();
-			UpdatePosition();
-		}
-
-		private void ContentElementSizeChanged(object sender, Windows.UI.Xaml.SizeChangedEventArgs args)
-		{
-			UpdateSize();
-			UpdatePosition();
-		}
-
 		public void SetIsPassword(bool isPassword)
 		{
 			if (_currentInputWidget is Entry entry)

diff --git a/src/Uno.UI.Runtime.Skia.Gtk/UnoDrawingArea.cs b/src/Uno.UI.Runtime.Skia.Gtk/UnoDrawingArea.cs
@@ -6,6 +6,7 @@
 using Windows.UI.Xaml.Input;
 using WUX = Windows.UI.Xaml;
 using Uno.Foundation.Logging;
+using Windows.UI.Xaml.Controls;
 
 namespace Uno.UI.Runtime.Skia
 {
@@ -21,18 +22,24 @@ public UnoDrawingArea()
 				+= () =>
 				{
 					// TODO Uno: Make this invalidation less often if possible.
-					InvalidateFocusVisual();
+					InvalidateOverlays();
 					Invalidate();
 				};
 		}
 
-		private void InvalidateFocusVisual()
+		private void InvalidateOverlays()
 		{
-			if (_focusManager == null)
+			_focusManager ??= VisualTree.GetFocusManagerForElement(Windows.UI.Xaml.Window.Current?.RootElement);
+			_focusManager?.FocusRectManager?.RedrawFocusVisual();
+			if (_focusManager?.FocusedElement is TextBox textBox)
 			{
-				_focusManager = VisualTree.GetFocusManagerForElement(Windows.UI.Xaml.Window.Current?.RootElement);
+				textBox.TextBoxView?.Extension?.InvalidateLayout();
 			}
-			_focusManager?.FocusRectManager?.RedrawFocusVisual();
+		}
+
+		private void InvalidateInputField()
+		{
+
 		}
 
 		private void Invalidate()

diff --git a/src/Uno.UI.Runtime.Skia.Wpf/Extensions/UI/Xaml/Controls/TextBoxViewExtension.cs b/src/Uno.UI.Runtime.Skia.Wpf/Extensions/UI/Xaml/Controls/TextBoxViewExtension.cs
@@ -18,8 +18,6 @@ internal class TextBoxViewExtension : ITextBoxViewExtension
 		private ContentControl? _contentElement;
 		private WpfTextViewTextBox? _currentInputWidget;
 
-		private readonly SerialDisposable _textBoxEventSubscriptions = new SerialDisposable();
-
 		public TextBoxViewExtension(TextBoxView owner)
 		{
 			_owner = owner ?? throw new ArgumentNullException(nameof(owner));
@@ -42,20 +40,10 @@ public void StartEntry()
 			EnsureWidgetForAcceptsReturn();
 			textInputLayer.Children.Add(_currentInputWidget!);
 
-			textBox.SizeChanged += ContentElementSizeChanged;
-			textBox.LayoutUpdated += ContentElementLayoutUpdated;
-
-			_textBoxEventSubscriptions.Disposable = Disposable.Create(() =>
-			{
-				textBox.SizeChanged -= ContentElementSizeChanged;
-				textBox.LayoutUpdated -= ContentElementLayoutUpdated;
-			});
-
 			UpdateNativeView();
 			SetTextNative(textBox.Text);
 
-			UpdateSize();
-			UpdatePosition();
+			InvalidateLayout();
 
 			_currentInputWidget!.Focus();
 		}
@@ -68,7 +56,6 @@ public void EndEntry()
 			}
 
 			_contentElement = null;
-			_textBoxEventSubscriptions.Disposable = null;
 
 			var textInputLayer = GetWindowTextInputLayer();
 			textInputLayer?.Children.Remove(_currentInputWidget);
@@ -100,6 +87,12 @@ public void UpdateNativeView()
 			_currentInputWidget.Foreground = textBox.Foreground.ToWpfBrush();
 		}
 
+		public void InvalidateLayout()
+		{
+			UpdateSize();
+			UpdatePosition();
+		}
+
 		public void UpdateSize()
 		{
 			var textInputLayer = GetWindowTextInputLayer();
@@ -162,18 +155,6 @@ private void WpfTextViewTextChanged(object sender, System.Windows.Controls.TextC
 
 		private string? GetInputText() => _currentInputWidget?.Text;
 
-		private void ContentElementLayoutUpdated(object? sender, object e)
-		{
-			UpdateSize();
-			UpdatePosition();
-		}
-
-		private void ContentElementSizeChanged(object sender, Windows.UI.Xaml.SizeChangedEventArgs args)
-		{
-			UpdateSize();
-			UpdatePosition();
-		}
-
 		public void SetIsPassword(bool isPassword)
 		{
 			// No support for now.

diff --git a/src/Uno.UI.Runtime.Skia.Wpf/WpfHost.cs b/src/Uno.UI.Runtime.Skia.Wpf/WpfHost.cs
@@ -125,7 +125,7 @@ bool EnqueueNative(DispatcherQueuePriority priority, DispatcherQueueHandler call
 
 			WinUI.Window.InvalidateRender += () =>
 			{
-				InvalidateFocusVisual();
+				InvalidateOverlays();
 				InvalidateVisual();
 			};
 
@@ -266,13 +266,14 @@ protected override void OnRender(DrawingContext drawingContext)
 			drawingContext.DrawImage(bitmap, new Rect(0, 0, ActualWidth, ActualHeight));
 		}
 
-		private void InvalidateFocusVisual()
+		private void InvalidateOverlays()
 		{
-			if (_focusManager == null)
+			_focusManager ??= VisualTree.GetFocusManagerForElement(Windows.UI.Xaml.Window.Current?.RootElement);
+			_focusManager?.FocusRectManager?.RedrawFocusVisual();
+			if (_focusManager?.FocusedElement is TextBox textBox)
 			{
-				_focusManager = VisualTree.GetFocusManagerForElement(Windows.UI.Xaml.Window.Current?.RootElement);
+				textBox.TextBoxView?.Extension?.InvalidateLayout();
 			}
-			_focusManager?.FocusRectManager?.RedrawFocusVisual();
 		}
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/ITextBoxExtension.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/ITextBoxExtension.skia.cs
@@ -11,6 +11,8 @@ internal interface ITextBoxViewExtension
 
 		void UpdateNativeView();
 
+		void InvalidateLayout();
+
 		void UpdateSize();
 
 		void UpdatePosition();

diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.skia.cs
@@ -5,44 +5,46 @@ namespace Windows.UI.Xaml.Controls
 	public partial class TextBox
 	{
 		private TextBoxView _textBoxView;
+
+		internal TextBoxView TextBoxView => _textBoxView;
 
 		internal ContentControl ContentElement => _contentElement;
 
-		partial void OnForegroundColorChangedPartial(Brush newValue) => _textBoxView?.OnForegroundChanged(newValue);
+		partial void OnForegroundColorChangedPartial(Brush newValue) => TextBoxView?.OnForegroundChanged(newValue);
 
-		partial void OnMaxLengthChangedPartial(DependencyPropertyChangedEventArgs e) => _textBoxView?.UpdateMaxLength();
+		partial void OnMaxLengthChangedPartial(DependencyPropertyChangedEventArgs e) => TextBoxView?.UpdateMaxLength();
 
 		private void UpdateTextBoxView()
 		{
 			_textBoxView ??= new TextBoxView(this);
-			if (ContentElement != null && ContentElement.Content != _textBoxView.DisplayBlock)
+			if (ContentElement != null && ContentElement.Content != TextBoxView.DisplayBlock)
 			{
-				ContentElement.Content = _textBoxView.DisplayBlock;
+				ContentElement.Content = TextBoxView.DisplayBlock;
 			}
 		}
 
-		partial void OnFocusStateChangedPartial(FocusState focusState) => _textBoxView?.OnFocusStateChanged(focusState);
+		partial void OnFocusStateChangedPartial(FocusState focusState) => TextBoxView?.OnFocusStateChanged(focusState);
 
 		partial void SelectPartial(int start, int length)
 		{
-			_textBoxView?.Select(start, length);
+			TextBoxView?.Select(start, length);
 		}
 
 		partial void SelectAllPartial() => Select(0, Text.Length);
 
 		public int SelectionStart
 		{
-			get => _textBoxView?.GetSelectionStart() ?? 0;
+			get => TextBoxView?.GetSelectionStart() ?? 0;
 			set => Select(start: value, length: SelectionLength);
 		}
 
 		public int SelectionLength
 		{
-			get => _textBoxView?.GetSelectionLength() ?? 0;
+			get => TextBoxView?.GetSelectionLength() ?? 0;
 			set => Select(SelectionStart, value);
 		}
 
 
-		protected void SetIsPassword(bool isPassword) => _textBoxView?.SetIsPassword(isPassword);
+		protected void SetIsPassword(bool isPassword) => TextBoxView?.SetIsPassword(isPassword);
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.skia.cs
@@ -33,6 +33,8 @@ public TextBoxView(TextBox textBox)
 			}
 		}
 
+		internal ITextBoxViewExtension Extension => _textBoxExtension;
+
 		public TextBox? TextBox
 		{
 			get
@@ -51,7 +53,6 @@ public TextBoxView(TextBox textBox)
 
 		public TextBlock DisplayBlock { get; } = new TextBlock();
 
-
 		internal void SetTextNative(string text)
 		{
 			// TODO: Inheritance hierarchy is wrong in Uno. PasswordBox shouldn't inherit TextBox.