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`

src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs

@@ -90,6 +90,8 @@ namespace Avalonia.Controls.Presenters
             this.GetObservable(ChildProperty).Subscribe(UpdateScrollableSubscription);
         }
 
+        internal event EventHandler<EventArgs> PreArrange;
+
         /// <summary>
         /// Gets or sets a value indicating whether the content can be scrolled horizontally.
         /// </summary>
@@ -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)); }
         }
 
         /// <summary>
@@ -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
         /// <inheritdoc/>
         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))

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<TransformedBounds>,
-            IObserver<TransformedBounds?>,
-            IObserver<Rect>
-        {
-            private readonly IControl _control;
-            private IDisposable _subscription;
-            private TransformedBounds? _lastBounds;
-            private bool _skipBounds;
-
-            public EffectiveViewportChangedObservable(IControl control) => _control = control;
-
-            void IObserver<TransformedBounds?>.OnNext(TransformedBounds? value)
-            {
-                _lastBounds = value;
-
-                if (value != null)
-                {
-                    PublishNext(value.Value);
-                }
-            }
-
-            void IObserver<Rect>.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<TransformedBounds?>.OnCompleted() { }
-            void IObserver<TransformedBounds?>.OnError(Exception error) { }
-            void IObserver<Rect>.OnCompleted() { }
-            void IObserver<Rect>.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

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)