From 0f891dc0b71b0d8df4eb823c17dd739a58a4cedb Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 8 Jun 2020 15:07:52 +0200 Subject: [PATCH] Fix scrolling to non-uniform size items. - Change the hack we're using to find the effective viewport in `ViewportManager` - Unregister anchor candidates when bringing an item into view - Adjust `ScrollContentPresenter` extent when tracking an anchor, if the adjustment causes the offset to go out of bounds of the extent - Simplify coercing `Offset` in `ScrollViewer` --- .../Presenters/ScrollContentPresenter.cs | 56 +++++++--- .../Repeater/ViewportManager.cs | 101 ++++++------------ src/Avalonia.Controls/ScrollViewer.cs | 21 +--- 3 files changed, 74 insertions(+), 104 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs b/src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs index de3b5fb35d..401867dc5d 100644 --- a/src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs +++ b/src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs @@ -90,6 +90,8 @@ namespace Avalonia.Controls.Presenters this.GetObservable(ChildProperty).Subscribe(UpdateScrollableSubscription); } + internal event EventHandler PreArrange; + /// /// Gets or sets a value indicating whether the content can be scrolled horizontally. /// @@ -123,7 +125,7 @@ namespace Avalonia.Controls.Presenters public Vector Offset { get { return _offset; } - set { SetAndRaise(OffsetProperty, ref _offset, value); } + set { SetAndRaise(OffsetProperty, ref _offset, ScrollViewer.CoerceOffset(Extent, Viewport, value)); } } /// @@ -214,7 +216,7 @@ namespace Avalonia.Controls.Presenters { _anchorCandidates?.Remove(element); - if (_anchor.Item1 == element) + if (_anchor.control == element) { _anchor = default; } @@ -239,6 +241,8 @@ namespace Avalonia.Controls.Presenters /// protected override Size ArrangeOverride(Size finalSize) { + PreArrange?.Invoke(this, EventArgs.Empty); + if (_logicalScrollSubscription != null || Child == null) { return base.ArrangeOverride(finalSize); @@ -270,16 +274,13 @@ namespace Avalonia.Controls.Presenters TranslateBounds(_anchor.control, Child, out var updatedBounds) && updatedBounds.Position != _anchor.bounds.Position) { - return updatedBounds.Position - _anchor.bounds.Position; + var offset = updatedBounds.Position - _anchor.bounds.Position; + return offset; } return default; } - // Check whether our previous anchor (if any) has moved relative to Child since the last - // arrange, and if so adjust the offset to bring it back into place. - Offset += TrackAnchor(); - // Calculate the new anchor element. _anchor = CalculateCurrentAnchor(); @@ -287,11 +288,27 @@ namespace Avalonia.Controls.Presenters ArrangeOverrideImpl(size, -Offset); // If the anchor moved during the arrange, we need to adjust the offset and do another arrange. - var postOffset = TrackAnchor(); + var anchorShift = TrackAnchor(); - if (postOffset != default) + if (anchorShift != default) { - Offset += postOffset; + var newOffset = Offset + anchorShift; + var newExtent = Extent; + var maxOffset = new Vector(Extent.Width - Viewport.Width, Extent.Height - Viewport.Height); + + if (newOffset.X > maxOffset.X) + { + newExtent = newExtent.WithWidth(newOffset.X + Viewport.Width); + } + + if (newOffset.Y > maxOffset.Y) + { + newExtent = newExtent.WithHeight(newOffset.Y + Viewport.Height); + } + + Extent = newExtent; + Offset = newOffset; + System.Diagnostics.Debug.WriteLine($"Shifting due to anchor by {anchorShift} to {Offset + anchorShift} with max {Extent - Viewport}"); ArrangeOverrideImpl(size, -Offset); } @@ -471,14 +488,13 @@ namespace Avalonia.Controls.Presenters } } - (IControl, Rect) CalculateCurrentAnchor() + private (IControl, Rect) CalculateCurrentAnchor() { if (_anchorCandidates == null) { return default; } - var thisBounds = new Rect(Bounds.Size); var bestCandidate = default(IControl); var bestCandidateDistance = double.MaxValue; @@ -486,9 +502,7 @@ namespace Avalonia.Controls.Presenters // ScrollContentPresenter. foreach (var element in _anchorCandidates) { - if (element.IsVisible && - TranslateBounds(element, this, out var bounds) && - bounds.Intersects(thisBounds)) + if (element.IsVisible && GetViewportBounds(element, out var bounds)) { var distance = (Vector)bounds.Position; var candidateDistance = Math.Abs(distance.Length); @@ -513,6 +527,18 @@ namespace Avalonia.Controls.Presenters return default; } + private bool GetViewportBounds(IControl element, out Rect bounds) + { + // We want the bounds relative to the new Offset, regardless of whether the child + // control has actually been arranged to this offset yet, so translate first to the + // child control and then apply Offset rather than translating directly to this + // control. + var thisBounds = new Rect(Bounds.Size); + var childBounds = TranslateBounds(element, Child); + bounds = new Rect(childBounds.Position - Offset, childBounds.Size); + return bounds.Intersects(thisBounds); + } + private Rect TranslateBounds(IControl control, IControl to) { if (TranslateBounds(control, to, out var bounds)) diff --git a/src/Avalonia.Controls/Repeater/ViewportManager.cs b/src/Avalonia.Controls/Repeater/ViewportManager.cs index 7f7b56909e..6677b8fea7 100644 --- a/src/Avalonia.Controls/Repeater/ViewportManager.cs +++ b/src/Avalonia.Controls/Repeater/ViewportManager.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Reactive.Disposables; using System.Reactive.Linq; using System.Threading.Tasks; +using Avalonia.Controls.Presenters; using Avalonia.Layout; using Avalonia.Logging; using Avalonia.Media; @@ -340,10 +341,10 @@ namespace Avalonia.Controls // Make sure that only the target child can be the anchor during the bring into view operation. foreach (var child in _owner.Children) { - ////if (child.CanBeScrollAnchor && child != targetChild) - ////{ - //// child.CanBeScrollAnchor = false; - ////} + if (child != targetChild) + { + _scroller.UnregisterAnchorCandidate(child); + } } // Register action to go back to how things were before where any child can be the anchor. Here, @@ -381,6 +382,16 @@ namespace Avalonia.Controls _isBringIntoViewInProgress = false; _makeAnchorElement = null; + foreach (var child in _owner.Children) + { + var info = ItemsRepeater.GetVirtualizationInfo(child); + + if (info.IsRealized && info.IsHeldByLayout) + { + _scroller.RegisterAnchorCandidate(child); + } + } + // HACK: Invalidate measure now that the anchor has been removed so that a layout can be // done with a proper realization rect. This is a hack not present upstream to try to fix // https://github.com/microsoft/microsoft-ui-xaml/issues/1422 @@ -395,13 +406,8 @@ namespace Avalonia.Controls _ensuredScroller = false; } - private void OnEffectiveViewportChanged(TransformedBounds tb) + private void OnEffectiveViewportChanged(Rect effectiveViewport) { - var globalClip = tb.Clip; - var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; - var clip = globalClip.TransformToAABB(transform); - var effectiveViewport = clip.Intersect(tb.Bounds); - Logger.TryGet(LogEventLevel.Verbose)?.Log("Repeater", this, "{LayoutId}: EffectiveViewportChanged event callback", _owner.Layout.LayoutId); UpdateViewport(effectiveViewport); @@ -512,72 +518,27 @@ namespace Avalonia.Controls { // HACK: This is a bit of a hack. We need the effective viewport of the ItemsRepeater - // we can get this from TransformedBounds, but this property is updated after layout has - // run, resulting in the UI being updated too late when scrolling quickly. We can - // partially remedey this by triggering also on Bounds changes, but this won't work so - // well for nested ItemsRepeaters. + // run, which is too late. Instead, for now lets just hook into an internal event on + // ScrollContentPresenter to find out what the offset and viewport will be after arrange + // and use those values. Note that this doesn't handle nested ScrollViewers at all, but + // it's enough to get scrolling to non-uniformly sized items working for now. // // UWP uses the EffectiveViewportChanged event (which I think was implemented specially - // for this case): we need to implement that in Avalonia. - return new EffectiveViewportChangedObservable(control) - .Subscribe(OnEffectiveViewportChanged); - } - - private class EffectiveViewportChangedObservable : SingleSubscriberObservableBase, - IObserver, - IObserver - { - private readonly IControl _control; - private IDisposable _subscription; - private TransformedBounds? _lastBounds; - private bool _skipBounds; - - public EffectiveViewportChangedObservable(IControl control) => _control = control; - - void IObserver.OnNext(TransformedBounds? value) - { - _lastBounds = value; - - if (value != null) - { - PublishNext(value.Value); - } - } - - void IObserver.OnNext(Rect value) - { - if (_lastBounds.HasValue) - { - if (!_skipBounds) - { - PublishNext(new TransformedBounds( - new Rect(value.Size), - _lastBounds.Value.Clip, - _lastBounds.Value.Transform)); - } - - _skipBounds = false; - } - } - - protected override void Subscribed() + // for this case): we need to implement that in Avalonia, but the semantics of it aren't + // clear to me. Hopefully the source for this event will be released with WinUI 3. + if (control.VisualParent is ScrollContentPresenter scp) { - _subscription = new CompositeDisposable( - _control.GetObservable(Visual.TransformedBoundsProperty).Subscribe(this), - _control.GetObservable(Visual.BoundsProperty).Subscribe(this)); - _lastBounds = null; - _skipBounds = true; + scp.PreArrange += ScrollContentPresenterPreArrange; + return Disposable.Create(() => scp.PreArrange -= ScrollContentPresenterPreArrange); } - protected override void Unsubscribed() - { - _subscription.Dispose(); - _subscription = null; - } + return Disposable.Empty; + } - void IObserver.OnCompleted() { } - void IObserver.OnError(Exception error) { } - void IObserver.OnCompleted() { } - void IObserver.OnError(Exception error) { } + private void ScrollContentPresenterPreArrange(object sender, EventArgs e) + { + var scp = (ScrollContentPresenter)sender; + OnEffectiveViewportChanged(new Rect((Point)scp.Offset, scp.Viewport)); } private class ScrollerInfo diff --git a/src/Avalonia.Controls/ScrollViewer.cs b/src/Avalonia.Controls/ScrollViewer.cs index f2c6f85996..cd0e7a1601 100644 --- a/src/Avalonia.Controls/ScrollViewer.cs +++ b/src/Avalonia.Controls/ScrollViewer.cs @@ -1,4 +1,5 @@ using System; +using System.Security.Authentication.ExtendedProtection; using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives; using Avalonia.Input; @@ -244,9 +245,7 @@ namespace Avalonia.Controls set { - value = ValidateOffset(this, value); - - if (SetAndRaise(OffsetProperty, ref _offset, value)) + if (SetAndRaise(OffsetProperty, ref _offset, CoerceOffset(Extent, Viewport, value))) { CalculatedPropertiesChanged(); } @@ -487,22 +486,6 @@ namespace Avalonia.Controls return double.IsNaN(result) ? 0 : result; } - private static Vector ValidateOffset(AvaloniaObject o, Vector value) - { - ScrollViewer scrollViewer = o as ScrollViewer; - - if (scrollViewer != null) - { - var extent = scrollViewer.Extent; - var viewport = scrollViewer.Viewport; - return CoerceOffset(extent, viewport, value); - } - else - { - return value; - } - } - private void ChildChanged(IControl child) { if (_logicalScrollable is object)