From fe0b91d5417632721b8733c17a0db709f687734f Mon Sep 17 00:00:00 2001
From: Julien Lebosquain <julien@lebosquain.net>
Date: Wed, 23 Oct 2024 10:26:05 +0200
Subject: [PATCH] Fix PopupRoot.ConfigurePosition being called unnecessary
 (#17322)

---
 src/Avalonia.Controls/Primitives/Popup.cs     | 49 ++++++++++++-------
 .../Primitives/PopupTests.cs                  |  4 +-
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs
index 87962846aa..8f377f44c5 100644
--- a/src/Avalonia.Controls/Primitives/Popup.cs
+++ b/src/Avalonia.Controls/Primitives/Popup.cs
@@ -1,5 +1,6 @@
 using System;
 using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;
 using Avalonia.Reactive;
 using Avalonia.Automation.Peers;
 using Avalonia.Controls.Diagnostics;
@@ -152,9 +153,7 @@ namespace Avalonia.Controls.Primitives
         {
             IsHitTestVisibleProperty.OverrideDefaultValue<Popup>(false);
             ChildProperty.Changed.AddClassHandler<Popup>((x, e) => x.ChildChanged(e));
-            IsOpenProperty.Changed.AddClassHandler<Popup>((x, e) => x.IsOpenChanged((AvaloniaPropertyChangedEventArgs<bool>)e));    
-            VerticalOffsetProperty.Changed.AddClassHandler<Popup>((x, _) => x.HandlePositionChange());    
-            HorizontalOffsetProperty.Changed.AddClassHandler<Popup>((x, _) => x.HandlePositionChange());
+            IsOpenProperty.Changed.AddClassHandler<Popup>((x, e) => x.IsOpenChanged((AvaloniaPropertyChangedEventArgs<bool>)e));
         }
 
         /// <summary>
@@ -720,18 +719,7 @@ namespace Avalonia.Controls.Primitives
         {
             if (_openState != null)
             {
-                var placementTarget = PlacementTarget ?? this.FindLogicalAncestorOfType<Control>();
-                if (placementTarget == null)
-                    return;
-                _openState.PopupHost.ConfigurePosition(new PopupPositionRequest(
-                    placementTarget,
-                    Placement,
-                    new Point(HorizontalOffset, VerticalOffset),
-                    PlacementAnchor,
-                    PlacementGravity,
-                    PlacementConstraintAdjustment,
-                    PlacementRect,
-                    CustomPopupPlacementCallback));
+                UpdateHostPosition(_openState.PopupHost, _openState.PlacementTarget);
             }
         }
 
@@ -977,7 +965,20 @@ namespace Avalonia.Controls.Primitives
 
         private void WindowPositionChanged(PixelPoint pp) => HandlePositionChange();
 
-        private void PlacementTargetLayoutUpdated(object? src, EventArgs e) => HandlePositionChange();
+        private void PlacementTargetLayoutUpdated(object? src, EventArgs e)
+        {
+            if (_openState is null)
+                return;
+
+            // A LayoutUpdated event is raised for the whole visual tree:
+            // the bounds of the PlacementTarget might not have effectively changed.
+            var newBounds = _openState.PlacementTarget.Bounds;
+            if (newBounds == _openState.LastPlacementTargetBounds)
+                return;
+
+            _openState.LastPlacementTargetBounds = newBounds;
+            UpdateHostPosition(_openState.PopupHost, _openState.PlacementTarget);
+        }
 
         private void ParentPopupPositionChanged(object? src, PixelPointEventArgs e) => HandlePositionChange();
 
@@ -1006,6 +1007,7 @@ namespace Avalonia.Controls.Primitives
         {
             private readonly IDisposable _cleanup;
             private IDisposable? _presenterCleanup;
+            private Control _placementTarget;
 
             public PopupOpenState(Control placementTarget, TopLevel topLevel, IPopupHost popupHost, IDisposable cleanup)
             {
@@ -1016,7 +1018,20 @@ namespace Avalonia.Controls.Primitives
             }
 
             public TopLevel TopLevel { get; }
-            public Control PlacementTarget { get; set; }
+
+            public Control PlacementTarget
+            {
+                get => _placementTarget;
+                [MemberNotNull(nameof(_placementTarget))]
+                set
+                {
+                    _placementTarget = value;
+                    LastPlacementTargetBounds = value.Bounds;
+                }
+            }
+
+            public Rect LastPlacementTargetBounds { get; set; }
+
             public IPopupHost PopupHost { get; }
 
             public void SetPresenterSubscription(IDisposable? presenterCleanup)
diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs
index 1cad1304af..da37c9ad48 100644
--- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs
+++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs
@@ -1244,9 +1244,7 @@ namespace Avalonia.Controls.UnitTests.Primitives
                 popup.Open();
                 root.LayoutManager.ExecuteLayoutPass();
 
-                // Ideally, callback should be executed only once for this test.
-                // But currently PlacementTargetLayoutUpdated triggers second update either way.
-                Assert.Equal(2, callbackExecuted);
+                Assert.Equal(1, callbackExecuted);
             }
         }