Browse Source

Corrected review comments + fixed some issues

Wojciech Krysiak 7 years ago
parent
commit
184967a62b

+ 54 - 36
src/Avalonia.Controls/Grid.cs

@@ -56,31 +56,12 @@ namespace Avalonia.Controls
             _sharedSizeHost?.InvalidateMeasure(this);
         }
 
-        protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
-        {
-            base.OnAttachedToVisualTree(e);
-            var scope = this.GetVisualAncestors().OfType<Control>()
-                .FirstOrDefault(c => c.GetValue(IsSharedSizeScopeProperty));
-
-            Debug.Assert(_sharedSizeHost == null);
-
-            if (scope != null)
-            {
-                _sharedSizeHost = scope.GetValue(s_sharedSizeScopeHostProperty);
-                _sharedSizeHost.RegisterGrid(this);
-            }
-        }
-
-        protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
-        {
-            base.OnDetachedFromVisualTree(e);
-
-            _sharedSizeHost?.UnegisterGrid(this);
-            _sharedSizeHost = null;
-        }
-
         private SharedSizeScopeHost _sharedSizeHost;
 
+        /// <summary>
+        /// Defines the SharedSizeScopeHost private property. 
+        /// The ampersands are used to make accessing the property via xaml inconvenient.
+        /// </summary>
         private static readonly AttachedProperty<SharedSizeScopeHost> s_sharedSizeScopeHostProperty =
             AvaloniaProperty.RegisterAttached<Grid, Control, SharedSizeScopeHost>("&&SharedSizeScopeHost", null);
 
@@ -94,20 +75,53 @@ namespace Avalonia.Controls
             IsSharedSizeScopeProperty.Changed.AddClassHandler<Control>(IsSharedSizeScopeChanged);
         }
 
-        private static void IsSharedSizeScopeChanged(Control source, AvaloniaPropertyChangedEventArgs arg2) 
+        public Grid()
+        {
+            this.AttachedToVisualTree += Grid_AttachedToVisualTree;
+            this.DetachedFromVisualTree += Grid_DetachedFromVisualTree;
+        }
+
+        private void Grid_AttachedToVisualTree(object sender, VisualTreeAttachmentEventArgs e)
         {
-            if ((bool)arg2.NewValue)
+            var scope = 
+                new Control[] { this }.Concat(this.GetVisualAncestors().OfType<Control>())
+                    .FirstOrDefault(c => c.GetValue(IsSharedSizeScopeProperty));
+
+            if (_sharedSizeHost != null)
+                throw new AvaloniaInternalException("Shared size scope already present when attaching to visual tree!");
+
+            if (scope != null)
             {
-                Debug.Assert(source.GetValue(s_sharedSizeScopeHostProperty) == null);
-                source.SetValue(s_sharedSizeScopeHostProperty, new SharedSizeScopeHost(source));
+                _sharedSizeHost = scope.GetValue(s_sharedSizeScopeHostProperty);
+                _sharedSizeHost.RegisterGrid(this);
             }
-            else
+        }
+
+        private void Grid_DetachedFromVisualTree(object sender, VisualTreeAttachmentEventArgs e)
+        {
+            _sharedSizeHost?.UnegisterGrid(this);
+            _sharedSizeHost = null;
+        }
+
+        private static void IsSharedSizeScopeChanged(Control source, AvaloniaPropertyChangedEventArgs arg2) 
+        {
+            var shouldDispose = (arg2.OldValue is bool d) && d;
+            if (shouldDispose)
             {
                 var host = source.GetValue(s_sharedSizeScopeHostProperty) as SharedSizeScopeHost;
-                Debug.Assert(host != null);
+                if (host == null)
+                    throw new AvaloniaInternalException("SharedScopeHost wasn't set when IsSharedSizeScope was true!");
                 host.Dispose();
                 source.SetValue(s_sharedSizeScopeHostProperty, null);
             }
+
+            var shouldAssign = (arg2.NewValue is bool a) && a;
+            if (shouldAssign)
+            {
+                if (source.GetValue(s_sharedSizeScopeHostProperty) != null)
+                    throw new AvaloniaInternalException("SharedScopeHost was already set when IsSharedSizeScope is only now being set to true!");
+                source.SetValue(s_sharedSizeScopeHostProperty, new SharedSizeScopeHost(source));
+            }
         }
 
         /// <summary>
@@ -328,7 +342,10 @@ namespace Avalonia.Controls
             _rowLayoutCache = rowLayout;
             _columnLayoutCache = columnLayout;
 
-            _sharedSizeHost?.UpdateMeasureStatus(this, rowResult, columnResult);
+            if (_sharedSizeHost?.ParticipatesInScope(this) ?? false)
+            {
+                _sharedSizeHost.UpdateMeasureStatus(this, rowResult, columnResult);
+            }
 
             return new Size(columnResult.DesiredLength, rowResult.DesiredLength);
 
@@ -379,13 +396,14 @@ namespace Avalonia.Controls
             var columnLayout = _columnLayoutCache;
             var rowLayout = _rowLayoutCache;
 
-            var (rowCache, columnCache) =
-                _sharedSizeHost?.HandleArrange(this, _rowMeasureCache, _columnMeasureCache) ??
-                (_rowMeasureCache, _columnMeasureCache);
+            var rowCache = _rowMeasureCache;
+            var columnCache = _columnMeasureCache;
 
-            if (_sharedSizeHost != null)
-            {
-                rowCache = rowLayout.Measure(finalSize.Width, rowCache.LeanLengthList);
+            if (_sharedSizeHost?.ParticipatesInScope(this) ?? false)
+                {
+                (rowCache, columnCache) = _sharedSizeHost.HandleArrange(this, _rowMeasureCache, _columnMeasureCache);
+            
+                rowCache = rowLayout.Measure(finalSize.Height, rowCache.LeanLengthList);
                 columnCache = columnLayout.Measure(finalSize.Width, columnCache.LeanLengthList);
             }
 

+ 11 - 6
src/Avalonia.Controls/Utils/GridLayout.cs

@@ -143,6 +143,9 @@ namespace Avalonia.Controls.Utils
         /// <param name="containerLength">
         /// The container length. Usually, it is the constraint of the <see cref="Layoutable.MeasureOverride"/> method.
         /// </param>
+        /// <param name="conventions">
+        /// Overriding conventions that allows the algorithm to handle external inputa 
+        /// </param>
         /// <returns>
         /// The measured result that containing the desired size and all the column/row lengths.
         /// </returns>
@@ -248,7 +251,7 @@ namespace Avalonia.Controls.Utils
             // | min | max |     |           | min |     |  min max  | max |
             // |#des#| fix |#des#| fix | fix | fix | fix |   #des#   |#des#|
 
-            var desiredStarMin = AggregateAdditionalConventionsForStars(conventions);
+            var (minLengths, desiredStarMin) = AggregateAdditionalConventionsForStars(conventions);
             aggregatedLength += desiredStarMin;
 
             // M6/7. Determine the desired length of the grid for current container length. Its value is stored in desiredLength.
@@ -282,7 +285,7 @@ namespace Avalonia.Controls.Utils
 
             // Returns the measuring result.
             return new MeasureResult(containerLength, desiredLength, greedyDesiredLength,
-                conventions, dynamicConvention);
+                conventions, dynamicConvention, minLengths);
         }
 
         /// <summary>
@@ -313,7 +316,7 @@ namespace Avalonia.Controls.Utils
                 // If the final length is smaller, we measure the M6/6 procedure only.
                 var dynamicConvention = ExpandStars(measure.LeanLengthList, finalLength);
                 measure = new MeasureResult(finalLength, measure.DesiredLength, measure.GreedyDesiredLength,
-                    measure.LeanLengthList, dynamicConvention);
+                    measure.LeanLengthList, dynamicConvention, measure.MinLengths);
             }
 
             return new ArrangeResult(measure.LengthList);
@@ -370,7 +373,7 @@ namespace Avalonia.Controls.Utils
         /// <param name="conventions">All the conventions that have almost been fixed except the rest *.</param>
         /// <returns>The total desired length of all the * length.</returns>
         [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private double AggregateAdditionalConventionsForStars(
+        private (List<double>, double) AggregateAdditionalConventionsForStars(
             IReadOnlyList<LengthConvention> conventions)
         {
             // 1. Determine all one-span column's desired widths or row's desired heights.
@@ -403,7 +406,7 @@ namespace Avalonia.Controls.Utils
                 lengthList[group.Key] = Math.Max(lengthList[group.Key], length > 0 ? length : 0);
             }
 
-            return lengthList.Sum() - fixedLength;
+            return (lengthList, lengthList.Sum() - fixedLength);
         }
 
         /// <summary>
@@ -638,13 +641,14 @@ namespace Avalonia.Controls.Utils
             /// Initialize a new instance of <see cref="MeasureResult"/>.
             /// </summary>
             internal MeasureResult(double containerLength, double desiredLength, double greedyDesiredLength,
-                IReadOnlyList<LengthConvention> leanConventions, IReadOnlyList<double> expandedConventions)
+                IReadOnlyList<LengthConvention> leanConventions, IReadOnlyList<double> expandedConventions, IReadOnlyList<double> minLengths)
             {
                 ContainerLength = containerLength;
                 DesiredLength = desiredLength;
                 GreedyDesiredLength = greedyDesiredLength;
                 LeanLengthList = leanConventions;
                 LengthList = expandedConventions;
+                MinLengths = minLengths;
             }
 
             /// <summary>
@@ -674,6 +678,7 @@ namespace Avalonia.Controls.Utils
             /// Gets the length list for each column/row.
             /// </summary>
             public IReadOnlyList<double> LengthList { get; }
+            public IReadOnlyList<double> MinLengths { get; }
         }
 
         /// <summary>

+ 65 - 44
src/Avalonia.Controls/Utils/SharedSizeScopeHost.cs

@@ -135,11 +135,13 @@ namespace Avalonia.Controls
                 for (int i = 0; i < Grid.RowDefinitions.Count; i++)
                 {
                     Results[i].MeasuredResult = rowResult.LengthList[i];
+                    Results[i].MinLength = rowResult.MinLengths[i];
                 }
 
                 for (int i = 0; i < Grid.ColumnDefinitions.Count; i++)
                 {
                     Results[i + Grid.RowDefinitions.Count].MeasuredResult = columnResult.LengthList[i];
+                    Results[i + Grid.RowDefinitions.Count].MinLength = columnResult.MinLengths[i];
                 }
             }
 
@@ -176,8 +178,47 @@ namespace Avalonia.Controls
 
             public DefinitionBase Definition { get; }
             public double MeasuredResult { get; set; }
+            public double MinLength { get; set; }
             public Group SizeGroup { get; set; }
             public Grid OwningGrid { get; }
+
+            public (double length, int priority) GetPriorityLength()
+            {
+                var length = (Definition as ColumnDefinition)?.Width ?? ((RowDefinition)Definition).Height;
+
+                if (length.IsAbsolute)
+                    return (MeasuredResult, 1);
+                if (length.IsAuto)
+                    return (MeasuredResult, 2);
+                if (MinLength > 0)
+                    return (MinLength, 3);
+                return (MeasuredResult, 4);
+            }
+        }
+
+
+        private class LentgthGatherer
+        {
+            public double Length { get; private set; }
+            private int gatheredPriority = 6;
+
+            public void Visit(MeasurementResult result)
+            {
+                var (length, priority) = result.GetPriorityLength();
+
+                if (gatheredPriority < priority)
+                    return;
+
+                gatheredPriority = priority;
+                if (gatheredPriority == priority)
+                {
+                    Length = Math.Max(length,Length);
+                }
+                else
+                {
+                    Length = length;
+                }
+            }
         }
 
 
@@ -208,7 +249,7 @@ namespace Avalonia.Controls
             {
                 if (_results.Contains(result))
                     throw new AvaloniaInternalException(
-                        $"Invalid call to Group.Add - The SharedSizeGroup {Name} already contains the passed result");
+                        $"SharedSizeScopeHost: Invalid call to Group.Add - The SharedSizeGroup {Name} already contains the passed result");
 
                 result.SizeGroup = this;
                 _results.Add(result);
@@ -218,7 +259,7 @@ namespace Avalonia.Controls
             {
                 if (!_results.Contains(result))
                     throw new AvaloniaInternalException(
-                        $"Invalid call to Group.Remove - The SharedSizeGroup {Name} does not contain the passed result");
+                        $"SharedSizeScopeHost: Invalid call to Group.Remove - The SharedSizeGroup {Name} does not contain the passed result");
                 result.SizeGroup = null;
                 _results.Remove(result);
             }
@@ -226,44 +267,12 @@ namespace Avalonia.Controls
 
             private double Gather()
             {
-                var result = 0.0d;
+                var visitor = new LentgthGatherer();
 
-                bool onlyFixed = false;
+                _results.ForEach(visitor.Visit);
 
-                foreach (var measurement in Results)
-                {
-                    if (Double.IsInfinity(measurement.MeasuredResult))
-                        continue;
-
-                    if (measurement.Definition is ColumnDefinition column)
-                    {
-                        if (!onlyFixed && column.Width.IsAbsolute)
-                        {
-                            onlyFixed = true;
-                            result = measurement.MeasuredResult;
-                        }
-                        else if (onlyFixed == column.Width.IsAbsolute)
-                            result = Math.Max(result, measurement.MeasuredResult);
-
-                        result = Math.Max(result, column.MinWidth);
-                    }
-                    if (measurement.Definition is RowDefinition row)
-                    {
-                        if (!onlyFixed && row.Height.IsAbsolute)
-                        {
-                            onlyFixed = true;
-                            result = measurement.MeasuredResult;
-                        }
-                        else if (onlyFixed == row.Height.IsAbsolute)
-                            result = Math.Max(result, measurement.MeasuredResult);
-
-                        result = Math.Max(result, row.MinHeight);
-                    }
-                }
-
-                return result;
+                return visitor.Length;
             }
-
         }
 
         private readonly AvaloniaList<MeasurementCache> _measurementCaches;
@@ -308,7 +317,7 @@ namespace Avalonia.Controls
 
             if (cache == null)
                 throw new AvaloniaInternalException(
-                    $"InvalidateMeasureImpl - called with a grid not present in the internal cache");
+                    $"SharedSizeScopeHost: InvalidateMeasureImpl - called with a grid not present in the internal cache");
 
             // already invalidated the cache, early out.
             if (cache.MeasurementState == MeasurementState.Invalidated)
@@ -340,7 +349,8 @@ namespace Avalonia.Controls
         internal void UpdateMeasureStatus(Grid grid, GridLayout.MeasureResult rowResult, GridLayout.MeasureResult columnResult)
         {
             var cache = _measurementCaches.FirstOrDefault(mc => ReferenceEquals(mc.Grid, grid));
-            Debug.Assert(cache != null);
+            if (cache == null)
+                throw new AvaloniaInternalException("SharedSizeScopeHost: Attempted to update measurement status for a grid that wasn't registered!");
 
             cache.UpdateMeasureResult(rowResult, columnResult);
         }
@@ -386,13 +396,15 @@ namespace Avalonia.Controls
                     rowDesiredLength,
                     rowResult.GreedyDesiredLength,//??
                     rowConventions,
-                    rowLengths),
+                    rowLengths,
+                    rowResult.MinLengths),
                 new GridLayout.MeasureResult(
                     columnResult.ContainerLength,
                     columnDesiredLength,
                     columnResult.GreedyDesiredLength, //??
                     columnConventions,
-                    columnLengths)
+                    columnLengths,
+                    columnResult.MinLengths)
             );
         }
 
@@ -440,7 +452,8 @@ namespace Avalonia.Controls
             if (string.IsNullOrEmpty(scopeName))
                 return;
 
-            Debug.Assert(_groups.TryGetValue(scopeName, out var group));
+            if (!_groups.TryGetValue(scopeName, out var group))
+                throw new AvaloniaInternalException($"SharedSizeScopeHost: The scope {scopeName} wasn't found in the shared size scope");
 
             group.Remove(result);
             if (!group.Results.Any())
@@ -471,7 +484,9 @@ namespace Avalonia.Controls
 
         internal void RegisterGrid(Grid toAdd)
         {
-            Debug.Assert(!_measurementCaches.Any(mc => ReferenceEquals(mc.Grid, toAdd)));
+            if (_measurementCaches.Any(mc => ReferenceEquals(mc.Grid, toAdd)))
+                throw new AvaloniaInternalException("SharedSizeScopeHost: tried to register a grid twice!");
+
             var cache = new MeasurementCache(toAdd);
             _measurementCaches.Add(cache);
             AddGridToScopes(cache);
@@ -480,11 +495,17 @@ namespace Avalonia.Controls
         internal void UnegisterGrid(Grid toRemove)
         {
             var cache = _measurementCaches.FirstOrDefault(mc => ReferenceEquals(mc.Grid, toRemove));
+            if (cache == null)
+                throw new AvaloniaInternalException("SharedSizeScopeHost: tried to unregister a grid that wasn't registered before!");
 
-            Debug.Assert(cache != null);
             _measurementCaches.Remove(cache);
             RemoveGridFromScopes(cache);
             cache.Dispose();
         }
+
+        internal bool ParticipatesInScope(Grid toCheck)
+        {
+            return _measurementCaches.FirstOrDefault(mc => ReferenceEquals(mc.Grid, toCheck))?.Results.Any() ?? false;
+        }
     }
 }