From 98369bdd502718eb4c9630c6b24a0b338cde4134 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:32:22 -0700 Subject: [PATCH] Introduce methods for computing the baseline location of a RenderBox without affecting the current layout (#144655) Extracted from https://github.com/flutter/flutter/pull/138369 Introduces `RenderBox.{compute,get}DryBaseline` for computing the baseline location in `RenderBox.computeDryLayout`. --- .../lib/src/material/input_decorator.dart | 5 +- .../flutter/lib/src/material/list_tile.dart | 6 +- .../lib/src/material/toggle_buttons.dart | 10 +- packages/flutter/lib/src/rendering/box.dart | 615 +++++++++++++----- .../flutter/lib/src/rendering/object.dart | 14 +- packages/flutter/lib/src/rendering/stack.dart | 51 +- .../material/dropdown_form_field_test.dart | 23 +- .../test/material/toggle_buttons_test.dart | 2 +- packages/flutter/test/rendering/box_test.dart | 17 + .../rendering/cached_intrinsics_test.dart | 123 ++++ .../flutter/test/rendering/stack_test.dart | 1 + .../flutter/test/widgets/baseline_test.dart | 9 + 12 files changed, 651 insertions(+), 225 deletions(-) diff --git a/packages/flutter/lib/src/material/input_decorator.dart b/packages/flutter/lib/src/material/input_decorator.dart index 136826b0e65963..e44dbf6516902d 100644 --- a/packages/flutter/lib/src/material/input_decorator.dart +++ b/packages/flutter/lib/src/material/input_decorator.dart @@ -1316,7 +1316,10 @@ class _RenderDecoration extends RenderBox with SlottedContainerRenderObjectMixin @override double computeDistanceToActualBaseline(TextBaseline baseline) { - return _boxParentData(input!).offset.dy + (input?.computeDistanceToActualBaseline(baseline) ?? 0.0); + final RenderBox? input = this.input; + return input == null + ? 0.0 + : _boxParentData(input).offset.dy + (input.computeDistanceToActualBaseline(baseline) ?? 0.0); } // Records where the label was painted. diff --git a/packages/flutter/lib/src/material/list_tile.dart b/packages/flutter/lib/src/material/list_tile.dart index 64d9f66e2cd559..1a846079b6ff83 100644 --- a/packages/flutter/lib/src/material/list_tile.dart +++ b/packages/flutter/lib/src/material/list_tile.dart @@ -1249,10 +1249,12 @@ class _RenderListTile extends RenderBox with SlottedContainerRenderObjectMixin<_ } @override - double computeDistanceToActualBaseline(TextBaseline baseline) { + double? computeDistanceToActualBaseline(TextBaseline baseline) { assert(title != null); final BoxParentData parentData = title!.parentData! as BoxParentData; - return parentData.offset.dy + title!.getDistanceToActualBaseline(baseline)!; + final BaselineOffset offset = BaselineOffset(title!.getDistanceToActualBaseline(baseline)) + + parentData.offset.dy; + return offset.offset; } static double? _boxBaseline(RenderBox box, TextBaseline baseline) { diff --git a/packages/flutter/lib/src/material/toggle_buttons.dart b/packages/flutter/lib/src/material/toggle_buttons.dart index 06727bcee7b65b..f74aa93c0c5934 100644 --- a/packages/flutter/lib/src/material/toggle_buttons.dart +++ b/packages/flutter/lib/src/material/toggle_buttons.dart @@ -1175,11 +1175,13 @@ class _SelectToggleButtonRenderObject extends RenderShiftedBox { } @override - double computeDistanceToActualBaseline(TextBaseline baseline) { + double? computeDistanceToActualBaseline(TextBaseline baseline) { // The baseline of this widget is the baseline of its child - return direction == Axis.horizontal - ? child!.computeDistanceToActualBaseline(baseline)! + borderSide.width - : child!.computeDistanceToActualBaseline(baseline)! + leadingBorderSide.width; + final BaselineOffset childOffset = BaselineOffset(child?.computeDistanceToActualBaseline(baseline)); + return switch (direction) { + Axis.horizontal => childOffset + borderSide.width, + Axis.vertical => childOffset + leadingBorderSide.width, + }.offset; } @override diff --git a/packages/flutter/lib/src/rendering/box.dart b/packages/flutter/lib/src/rendering/box.dart index 3ac00d990feb20..449b39417afe98 100644 --- a/packages/flutter/lib/src/rendering/box.dart +++ b/packages/flutter/lib/src/rendering/box.dart @@ -192,7 +192,7 @@ class BoxConstraints extends Constraints { } /// Returns new box constraints that are smaller by the given edge dimensions. - BoxConstraints deflate(EdgeInsets edges) { + BoxConstraints deflate(EdgeInsetsGeometry edges) { assert(debugAssertIsValid()); final double horizontal = edges.horizontal; final double vertical = edges.vertical; @@ -940,24 +940,154 @@ class BoxParentData extends ParentData { /// the relevant type arguments. abstract class ContainerBoxParentData extends BoxParentData with ContainerParentDataMixin { } -enum _IntrinsicDimension { minWidth, maxWidth, minHeight, maxHeight } +/// A wrapper that represents the baseline location of a `RenderBox`. +extension type const BaselineOffset(double? offset) { + /// A value that indicates that the associated `RenderBox` does not have any + /// baselines. + /// + /// [BaselineOffset.noBaseline] is an identity element in most binary + /// operations involving two [BaselineOffset]s (such as [minOf]), for render + /// objects with no baselines typically do not contribute to the baseline + /// offset of their parents. + static const BaselineOffset noBaseline = BaselineOffset(null); + + /// Returns a new baseline location that is `offset` pixels further away from + /// the origin than `this`, or unchanged if `this` is [noBaseline]. + BaselineOffset operator +(double offset) { + final double? value = this.offset; + return BaselineOffset(value == null ? null : value + offset); + } + + /// Compares this [BaselineOffset] and `other`, and returns whichever is closer + /// to the origin. + /// + /// When both `this` and `other` are [noBaseline], this method returns + /// [noBaseline]. When one of them is [noBaseline], this method returns the + /// other oprand that's not [noBaseline]. + BaselineOffset minOf(BaselineOffset other) { + return switch ((this, other)) { + (final double lhs?, final double rhs?) => lhs >= rhs ? other : this, + (final double lhs?, null) => BaselineOffset(lhs), + (null, final BaselineOffset rhs) => rhs, + }; + } +} -@immutable -class _IntrinsicDimensionsCacheEntry { - const _IntrinsicDimensionsCacheEntry(this.dimension, this.argument); +/// An interface that represents a memoized layout computation run by a [RenderBox]. +/// +/// Each subclass is inhabited by a single object. Each object represents the +/// signature of a memoized layout computation run by [RenderBox]. For instance, +/// the [dryLayout] object of the [_DryLayout] subclass represents the signature +/// of the [RenderBox.computeDryLayout] method: it takes a [BoxConstraints] (the +/// subclass's `Input` type parameter) and returns a [Size] (the subclass's +/// `Output` type parameter). +/// +/// Subclasses do not own their own cache storage. Rather, their [memoize] +/// implementation takes a `cacheStorage`. If a prior computation with the same +/// input valus has already been memoized in `cacheStorage`, it returns the +/// memoized value without running `computer`. Otherwise the method runs the +/// `computer` to compute the return value, and caches the result to +/// `cacheStorage`. +/// +/// The layout cache storage is typically cleared in `markNeedsLayout`, but is +/// usually kept across [RenderObject.layout] calls because the incoming +/// [BoxConstraints] is always an input of every layout computation. +abstract class _CachedLayoutCalculation { + static const _DryLayout dryLayout = _DryLayout(); + static const _Baseline baseline = _Baseline(); + + Output memoize(_LayoutCacheStorage cacheStorage, Input input, Output Function(Input) computer); + + // Debug information that will be used to generate the Timeline event for this type of calculation. + Map debugFillTimelineArguments(Map timelineArguments, Input input); + String eventLabel(RenderBox renderBox); +} - final _IntrinsicDimension dimension; - final double argument; +final class _DryLayout implements _CachedLayoutCalculation { + const _DryLayout(); @override - bool operator ==(Object other) { - return other is _IntrinsicDimensionsCacheEntry - && other.dimension == dimension - && other.argument == argument; + Size memoize(_LayoutCacheStorage cacheStorage, BoxConstraints input, Size Function(BoxConstraints) computer) { + return (cacheStorage._cachedDryLayoutSizes ??= {}).putIfAbsent(input, () => computer(input)); + } + + @override + Map debugFillTimelineArguments(Map timelineArguments, BoxConstraints input) { + return timelineArguments..['getDryLayout constraints'] = '$input'; + } + + @override + String eventLabel(RenderBox renderBox) => '${renderBox.runtimeType}.getDryLayout'; +} + +final class _Baseline implements _CachedLayoutCalculation<(BoxConstraints, TextBaseline), BaselineOffset> { + const _Baseline(); + + @override + BaselineOffset memoize(_LayoutCacheStorage cacheStorage, (BoxConstraints, TextBaseline) input, BaselineOffset Function((BoxConstraints, TextBaseline)) computer) { + final Map cache = switch (input.$2) { + TextBaseline.alphabetic => cacheStorage._cachedAlphabeticBaseline ??= {}, + TextBaseline.ideographic => cacheStorage._cachedIdeoBaseline ??= {}, + }; + BaselineOffset ifAbsent() => computer(input); + return cache.putIfAbsent(input.$1, ifAbsent); + } + + @override + Map debugFillTimelineArguments(Map timelineArguments, (BoxConstraints, TextBaseline) input) { + return timelineArguments + ..['baseline type'] = '${input.$2}' + ..['constraints'] = '${input.$1}'; + } + + @override + String eventLabel(RenderBox renderBox) => '${renderBox.runtimeType}.getDryBaseline'; +} + +// Intrinsic dimension calculation that computes the intrinsic width given the +// max height, or the intrinsic height given the max width. +enum _IntrinsicDimension implements _CachedLayoutCalculation { + minWidth, maxWidth, minHeight, maxHeight; + + @override + double memoize(_LayoutCacheStorage cacheStorage, double input, double Function(double) computer) { + return (cacheStorage._cachedIntrinsicDimensions ??= <(_IntrinsicDimension, double), double>{}) + .putIfAbsent((this, input), () => computer(input)); } @override - int get hashCode => Object.hash(dimension, argument); + Map debugFillTimelineArguments(Map timelineArguments, double input) { + return timelineArguments + ..['intrinsics dimension'] = name + ..['intrinsics argument'] = '$input'; + } + + @override + String eventLabel(RenderBox renderBox) => '${renderBox.runtimeType} intrinsics'; +} + +final class _LayoutCacheStorage { + Map<(_IntrinsicDimension, double), double>? _cachedIntrinsicDimensions; + Map? _cachedDryLayoutSizes; + Map? _cachedAlphabeticBaseline; + Map? _cachedIdeoBaseline; + + // Returns a boolean indicating whether the cache storage has cached + // intrinsics / dry layout data in it. + bool clear() { + final bool hasCache = (_cachedDryLayoutSizes?.isNotEmpty ?? false) + || (_cachedIntrinsicDimensions?.isNotEmpty ?? false) + || (_cachedAlphabeticBaseline?.isNotEmpty ?? false) + || (_cachedIdeoBaseline?.isNotEmpty ?? false); + + if (hasCache) { + _cachedDryLayoutSizes?.clear(); + _cachedIntrinsicDimensions?.clear(); + _cachedAlphabeticBaseline?.clear(); + _cachedIdeoBaseline?.clear(); + } + return hasCache; + } } /// A render object in a 2D Cartesian coordinate system. @@ -1381,55 +1511,52 @@ abstract class RenderBox extends RenderObject { } } - Map<_IntrinsicDimensionsCacheEntry, double>? _cachedIntrinsicDimensions; - static int _debugIntrinsicsDepth = 0; + final _LayoutCacheStorage _layoutCacheStorage = _LayoutCacheStorage(); - double _computeIntrinsicDimension(_IntrinsicDimension dimension, double argument, double Function(double argument) computer) { + static int _debugIntrinsicsDepth = 0; + Output _computeIntrinsics( + _CachedLayoutCalculation type, + Input input, + Output Function(Input) computer, + ) { assert(RenderObject.debugCheckingIntrinsics || !debugDoingThisResize); // performResize should not depend on anything except the incoming constraints bool shouldCache = true; assert(() { // we don't want the checked-mode intrinsic tests to affect // who gets marked dirty, etc. - if (RenderObject.debugCheckingIntrinsics) { - shouldCache = false; - } + shouldCache = !RenderObject.debugCheckingIntrinsics; return true; }()); - if (shouldCache) { - Map? debugTimelineArguments; - assert(() { - if (debugEnhanceLayoutTimelineArguments) { - debugTimelineArguments = toDiagnosticsNode().toTimelineArguments(); - } else { - debugTimelineArguments = {}; - } - debugTimelineArguments!['intrinsics dimension'] = dimension.name; - debugTimelineArguments!['intrinsics argument'] = '$argument'; - return true; - }()); - if (!kReleaseMode) { - if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { - FlutterTimeline.startSync( - '$runtimeType intrinsics', - arguments: debugTimelineArguments, - ); - } - _debugIntrinsicsDepth += 1; + return shouldCache ? _computeWithTimeline(type, input, computer) : computer(input); + } + + Output _computeWithTimeline( + _CachedLayoutCalculation type, + Input input, + Output Function(Input) computer, + ) { + Map? debugTimelineArguments; + assert(() { + final Map arguments = debugEnhanceLayoutTimelineArguments + ? toDiagnosticsNode().toTimelineArguments()! + : {}; + debugTimelineArguments = type.debugFillTimelineArguments(arguments, input); + return true; + }()); + if (!kReleaseMode) { + if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { + FlutterTimeline.startSync(type.eventLabel(this), arguments: debugTimelineArguments); } - _cachedIntrinsicDimensions ??= <_IntrinsicDimensionsCacheEntry, double>{}; - final double result = _cachedIntrinsicDimensions!.putIfAbsent( - _IntrinsicDimensionsCacheEntry(dimension, argument), - () => computer(argument), - ); - if (!kReleaseMode) { - _debugIntrinsicsDepth -= 1; - if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { - FlutterTimeline.finishSync(); - } + _debugIntrinsicsDepth += 1; + } + final Output result = type.memoize(_layoutCacheStorage, input, computer); + if (!kReleaseMode) { + _debugIntrinsicsDepth -= 1; + if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { + FlutterTimeline.finishSync(); } - return result; } - return computer(argument); + return result; } /// Returns the minimum width that this box could be without failing to @@ -1463,7 +1590,7 @@ abstract class RenderBox extends RenderObject { } return true; }()); - return _computeIntrinsicDimension(_IntrinsicDimension.minWidth, height, computeMinIntrinsicWidth); + return _computeIntrinsics(_IntrinsicDimension.minWidth, height, computeMinIntrinsicWidth); } /// Computes the value returned by [getMinIntrinsicWidth]. Do not call this @@ -1605,7 +1732,7 @@ abstract class RenderBox extends RenderObject { } return true; }()); - return _computeIntrinsicDimension(_IntrinsicDimension.maxWidth, height, computeMaxIntrinsicWidth); + return _computeIntrinsics(_IntrinsicDimension.maxWidth, height, computeMaxIntrinsicWidth); } /// Computes the value returned by [getMaxIntrinsicWidth]. Do not call this @@ -1681,7 +1808,7 @@ abstract class RenderBox extends RenderObject { } return true; }()); - return _computeIntrinsicDimension(_IntrinsicDimension.minHeight, width, computeMinIntrinsicHeight); + return _computeIntrinsics(_IntrinsicDimension.minHeight, width, computeMinIntrinsicHeight); } /// Computes the value returned by [getMinIntrinsicHeight]. Do not call this @@ -1756,7 +1883,7 @@ abstract class RenderBox extends RenderObject { } return true; }()); - return _computeIntrinsicDimension(_IntrinsicDimension.maxHeight, width, computeMaxIntrinsicHeight); + return _computeIntrinsics(_IntrinsicDimension.maxHeight, width, computeMaxIntrinsicHeight); } /// Computes the value returned by [getMaxIntrinsicHeight]. Do not call this @@ -1800,9 +1927,6 @@ abstract class RenderBox extends RenderObject { return 0.0; } - Map? _cachedDryLayoutSizes; - bool _computingThisDryLayout = false; - /// Returns the [Size] that this [RenderBox] would like to be given the /// provided [BoxConstraints]. /// @@ -1822,49 +1946,11 @@ abstract class RenderBox extends RenderObject { /// /// Do not override this method. Instead, implement [computeDryLayout]. @mustCallSuper - Size getDryLayout(BoxConstraints constraints) { - bool shouldCache = true; - assert(() { - // we don't want the checked-mode intrinsic tests to affect - // who gets marked dirty, etc. - if (RenderObject.debugCheckingIntrinsics) { - shouldCache = false; - } - return true; - }()); - if (shouldCache) { - Map? debugTimelineArguments; - assert(() { - if (debugEnhanceLayoutTimelineArguments) { - debugTimelineArguments = toDiagnosticsNode().toTimelineArguments(); - } else { - debugTimelineArguments = {}; - } - debugTimelineArguments!['getDryLayout constraints'] = '$constraints'; - return true; - }()); - if (!kReleaseMode) { - if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { - FlutterTimeline.startSync( - '$runtimeType.getDryLayout', - arguments: debugTimelineArguments, - ); - } - _debugIntrinsicsDepth += 1; - } - _cachedDryLayoutSizes ??= {}; - final Size result = _cachedDryLayoutSizes!.putIfAbsent(constraints, () => _computeDryLayout(constraints)); - if (!kReleaseMode) { - _debugIntrinsicsDepth -= 1; - if (debugProfileLayoutsEnabled || _debugIntrinsicsDepth == 0) { - FlutterTimeline.finishSync(); - } - } - return result; - } - return _computeDryLayout(constraints); + Size getDryLayout(covariant BoxConstraints constraints) { + return _computeIntrinsics(_CachedLayoutCalculation.dryLayout, constraints, _computeDryLayout); } + bool _computingThisDryLayout = false; Size _computeDryLayout(BoxConstraints constraints) { assert(() { assert(!_computingThisDryLayout); @@ -1902,11 +1988,8 @@ abstract class RenderBox extends RenderObject { /// ### When the size cannot be known /// /// There are cases where render objects do not have an efficient way to - /// compute their size without doing a full layout. For example, the size - /// may depend on the baseline of a child (which is not available without - /// doing a full layout), it may be computed by a callback about which the - /// render object cannot reason, or the layout is so complex that it - /// is impractical to calculate the size in an efficient way. + /// compute their size. For example, the size may computed by a callback about + /// which the render object cannot reason. /// /// In such cases, it may be impossible (or at least impractical) to actually /// return a valid answer. In such cases, the function should call @@ -1926,10 +2009,108 @@ abstract class RenderBox extends RenderObject { return Size.zero; } - static bool _dryLayoutCalculationValid = true; + /// Returns the distance from the top of the box to the first baseline of the + /// box's contents for the given `constraints`, or `null` if this [RenderBox] + /// does not have any baselines. + /// + /// This method calls [computeDryBaseline] under the hood and caches the result. + /// [RenderBox] subclasses typically don't overridden [getDryBaseline]. Instead, + /// consider overriding [computeDryBaseline] such that it returns a baseline + /// location that is consistent with [getDistanceToActualBaseline]. See the + /// documentation for the [computeDryBaseline] method for more details. + /// + /// This method is usually called by the [computeDryBaseline] or the + /// [computeDryLayout] implementation of a parent [RenderBox] to get the + /// baseline location of a [RenderBox] child. Unlike [getDistanceToBaseline], + /// this method takes a [BoxConstraints] as an argument and computes the + /// baseline location as if the [RenderBox] was laid out by the parent using + /// that [BoxConstraints]. + /// + /// The "dry" in the method name means this method, like [getDryLayout], has + /// no observable side effects when called, as opposed to "wet" layout methods + /// such as [performLayout] (which changes this [RenderBox]'s [size], and the + /// offsets of its children if any). Since this method does not depend on the + /// current layout, unlike [getDistanceToBaseline], it's ok to call this method + /// when this [RenderBox]'s layout is outdated. + /// + /// Similar to the intrinsic width/height and [getDryLayout], calling this + /// function in [performLayout] is expensive, as it can result in O(N^2) layout + /// performance, where N is the number of render objects in the render subtree. + /// Typically this method should be only called by the parent [RenderBox]'s + /// [computeDryBaseline] or [computeDryLayout] implementation. + double? getDryBaseline(covariant BoxConstraints constraints, TextBaseline baseline) { + final double? baselineOffset = _computeIntrinsics(_CachedLayoutCalculation.baseline, (constraints, baseline), _computeDryBaseline).offset; + // This assert makes sure computeDryBaseline always gets called in debug mode, + // in case the computeDryBaseline implementation invokes debugCannotComputeDryLayout. + assert(baselineOffset == computeDryBaseline(constraints, baseline)); + return baselineOffset; + } + + bool _computingThisDryBaseline = false; + BaselineOffset _computeDryBaseline((BoxConstraints, TextBaseline) pair) { + assert(() { + assert(!_computingThisDryBaseline); + _computingThisDryBaseline = true; + return true; + }()); + final BaselineOffset result = BaselineOffset(computeDryBaseline(pair.$1, pair.$2)); + assert(() { + assert(_computingThisDryBaseline); + _computingThisDryBaseline = false; + return true; + }()); + return result; + } + + /// Computes the value returned by [getDryBaseline]. + /// + /// This method is for overriding only and shouldn't be called directly. To + /// get this [RenderBox]'s speculative baseline location for the given + /// `constraints`, call [getDryBaseline] instead. + /// + /// The "dry" in the method name means the implementation must not produce + /// observable side effects when called. For example, it must not change the + /// [size] of the [RenderBox], or its children's paint offsets, otherwise that + /// would results in UI changes when [paint] is called, or hit-testing behavior + /// changes when [hitTest] is called. Moreover, accessing the current layout + /// of this [RenderBox] or child [RenderBox]es (including accessing [size], or + /// `child.size`) usually indicates a bug in the implementaion, as the current + /// layout is typically calculated using a set of [BoxConstraints] that's + /// different from the `constraints` given as the first parameter. To get the + /// size of this [RenderBox] or a child [RenderBox] in this method's + /// implementatin, use the [getDryLayout] method instead. + /// + /// The implementation must return a value that represents the distance from + /// the top of the box to the first baseline of the box's contents, for the + /// given `constraints`, or `null` if the [RenderBox] has no baselines. It's + /// the same exact value [RenderBox.computeDistanceToActualBaseline] would + /// return, when this [RenderBox] was laid out at `constraints` in the same + /// exact state. + /// + /// Not all [RenderBox]es support dry baseline computation. For example, to + /// compute the dry baseline of a [LayoutBuilder], its `builder` may have to + /// be called with different constraints, which may have side effects such as + /// updating the widget tree, violating the "dry" contract. In such cases the + /// [RenderBox] must call [debugCannotComputeDryLayout] in an assert, and + /// return a dummy baseline offset value (such as `null`). + @protected + double? computeDryBaseline(covariant BoxConstraints constraints, TextBaseline baseline) { + assert(debugCannotComputeDryLayout( + error: FlutterError.fromParts([ + ErrorSummary('The ${objectRuntimeType(this, 'RenderBox')} class does not implement "computeDryBaseline".'), + ErrorHint( + 'If you are not writing your own RenderBox subclass, then this is not\n' + 'your fault. Contact support: https://github.com/flutter/flutter/issues/new?template=2_bug.yml', + ), + ]), + )); + return null; + } + + static bool _debugDryLayoutCalculationValid = true; - /// Called from [computeDryLayout] within an assert if the given [RenderBox] - /// subclass does not support calculating a dry layout. + /// Called from [computeDryLayout] or [computeDryBaseline] within an assert if + /// the given [RenderBox] subclass does not support calculating a dry layout. /// /// When asserts are enabled and [debugCheckingIntrinsics] is not true, this /// method will either throw the provided [FlutterError] or it will create and @@ -1947,7 +2128,7 @@ abstract class RenderBox extends RenderObject { assert(() { if (!RenderObject.debugCheckingIntrinsics) { if (reason != null) { - assert(error ==null); + assert(error == null); throw FlutterError.fromParts([ ErrorSummary('The ${objectRuntimeType(this, 'RenderBox')} class does not support dry layout.'), if (reason.isNotEmpty) ErrorDescription(reason), @@ -1956,7 +2137,7 @@ abstract class RenderBox extends RenderObject { assert(error != null); throw error!; } - _dryLayoutCalculationValid = false; + _debugDryLayoutCalculationValid = false; return true; }()); return true; @@ -1981,19 +2162,33 @@ abstract class RenderBox extends RenderObject { final Size? size = _size; if (size is _DebugSize) { assert(size._owner == this); - if (RenderObject.debugActiveLayout != null && - !RenderObject.debugActiveLayout!.debugDoingThisLayoutWithCallback) { - assert( - debugDoingThisResize || debugDoingThisLayout || _computingThisDryLayout || - (RenderObject.debugActiveLayout == parent && size._canBeUsedByParent), - 'RenderBox.size accessed beyond the scope of resize, layout, or ' - 'permitted parent access. RenderBox can always access its own size, ' - 'otherwise, the only object that is allowed to read RenderBox.size ' - 'is its parent, if they have said they will. If you hit this assert ' - 'trying to access a child\'s size, pass "parentUsesSize: true" to ' - "that child's layout().", - ); - } + final RenderObject? parent = this.parent; + // Whether the size getter is accessed during layout (but not in a + // layout callback). + final bool doingRegularLayout = !(RenderObject.debugActiveLayout?.debugDoingThisLayoutWithCallback ?? true); + final bool sizeAccessAllowed = !doingRegularLayout + || debugDoingThisResize + || debugDoingThisLayout + || _computingThisDryLayout + || RenderObject.debugActiveLayout == parent && size._canBeUsedByParent; + assert(sizeAccessAllowed, + 'RenderBox.size accessed beyond the scope of resize, layout, or ' + 'permitted parent access. RenderBox can always access its own size, ' + 'otherwise, the only object that is allowed to read RenderBox.size ' + 'is its parent, if they have said they will. It you hit this assert ' + 'trying to access a child\'s size, pass "parentUsesSize: true" to ' + "that child's layout() in ${objectRuntimeType(this, 'RenderBox')}.performLayout.", + ); + final RenderBox? renderBoxDoingDryBaseline = _computingThisDryBaseline + ? this + : (parent is RenderBox && parent._computingThisDryBaseline ? parent : null); + assert(renderBoxDoingDryBaseline == null, + 'RenderBox.size accessed in ' + '${objectRuntimeType(renderBoxDoingDryBaseline, 'RenderBox')}.computeDryBaseline.' + 'The computeDryBaseline method must not access ' + '${renderBoxDoingDryBaseline == this ? "the RenderBox's own size" : "the size of its child"},' + "because it's established in peformLayout or peformResize using different BoxConstraints." + ); assert(size == _size); } return true; @@ -2122,7 +2317,6 @@ abstract class RenderBox extends RenderObject { size = size; // ignore: no_self_assignments } - Map? _cachedBaselines; static bool _debugDoingBaseline = false; static bool _debugSetDoingBaseline(bool value) { _debugDoingBaseline = value; @@ -2145,19 +2339,19 @@ abstract class RenderBox extends RenderObject { /// /// When implementing a [RenderBox] subclass, to override the baseline /// computation, override [computeDistanceToActualBaseline]. + /// + /// See also: + /// + /// * [getDryBaseline], which returns the baseline location of this + /// [RenderBox] at a certain [BoxConstraints]. double? getDistanceToBaseline(TextBaseline baseline, { bool onlyReal = false }) { assert(!_debugDoingBaseline, 'Please see the documentation for computeDistanceToActualBaseline for the required calling conventions of this method.'); - assert(!debugNeedsLayout); - assert(() { - if (owner!.debugDoingLayout) { - return (RenderObject.debugActiveLayout == parent) && parent!.debugDoingThisLayout; - } - if (owner!.debugDoingPaint) { - return ((RenderObject.debugActivePaint == parent) && parent!.debugDoingThisPaint) || - ((RenderObject.debugActivePaint == this) && debugDoingThisPaint); - } - return false; - }()); + assert(!debugNeedsLayout || RenderObject.debugCheckingIntrinsics); + assert(RenderObject.debugCheckingIntrinsics || switch (owner!) { + PipelineOwner(debugDoingLayout: true) => RenderObject.debugActiveLayout == parent && parent!.debugDoingThisLayout, + PipelineOwner(debugDoingPaint: true) => RenderObject.debugActivePaint == parent && parent!.debugDoingThisPaint || (RenderObject.debugActivePaint == this && debugDoingThisPaint), + PipelineOwner() => false, + }); assert(_debugSetDoingBaseline(true)); final double? result; try { @@ -2180,8 +2374,11 @@ abstract class RenderBox extends RenderObject { @mustCallSuper double? getDistanceToActualBaseline(TextBaseline baseline) { assert(_debugDoingBaseline, 'Please see the documentation for computeDistanceToActualBaseline for the required calling conventions of this method.'); - _cachedBaselines ??= {}; - return _cachedBaselines!.putIfAbsent(baseline, () => computeDistanceToActualBaseline(baseline)); + return _computeIntrinsics( + _CachedLayoutCalculation.baseline, + (constraints, baseline), + ((BoxConstraints, TextBaseline) pair) => BaselineOffset(computeDistanceToActualBaseline(pair.$2)), + ).offset; } /// Returns the distance from the y-coordinate of the position of the box to @@ -2331,7 +2528,7 @@ abstract class RenderBox extends RenderObject { } // Checking that getDryLayout computes the same size. - _dryLayoutCalculationValid = true; + _debugDryLayoutCalculationValid = true; RenderObject.debugCheckingIntrinsics = true; final Size dryLayoutSize; try { @@ -2339,7 +2536,7 @@ abstract class RenderBox extends RenderObject { } finally { RenderObject.debugCheckingIntrinsics = false; } - if (_dryLayoutCalculationValid && dryLayoutSize != size) { + if (_debugDryLayoutCalculationValid && dryLayoutSize != size) { throw FlutterError.fromParts([ ErrorSummary('The size given to the ${objectRuntimeType(this, 'RenderBox')} class differs from the size computed by computeDryLayout.'), ErrorDescription( @@ -2360,42 +2557,99 @@ abstract class RenderBox extends RenderObject { }()); } - bool _clearCachedData() { - if ((_cachedBaselines != null && _cachedBaselines!.isNotEmpty) || - (_cachedIntrinsicDimensions != null && _cachedIntrinsicDimensions!.isNotEmpty) || - (_cachedDryLayoutSizes != null && _cachedDryLayoutSizes!.isNotEmpty)) { - // If we have cached data, then someone must have used our data. - // Since the parent will shortly be marked dirty, we can forget that they - // used the baseline and/or intrinsic dimensions. If they use them again, - // then we'll fill the cache again, and if we get dirty again, we'll - // notify them again. - _cachedBaselines?.clear(); - _cachedIntrinsicDimensions?.clear(); - _cachedDryLayoutSizes?.clear(); + void _debugVerifyDryBaselines() { + assert(() { + final List messages = [ + ErrorDescription( + 'The constraints used were $constraints.', + ), + ErrorHint( + 'If you are not writing your own RenderBox subclass, then this is not\n' + 'your fault. Contact support: https://github.com/flutter/flutter/issues/new?template=2_bug.yml', + ) + ]; + + for (final TextBaseline baseline in TextBaseline.values) { + assert(!RenderObject.debugCheckingIntrinsics); + RenderObject.debugCheckingIntrinsics = true; + _debugDryLayoutCalculationValid = true; + final double? dryBaseline; + final double? realBaseline; + try { + dryBaseline = getDryBaseline(constraints, baseline); + realBaseline = getDistanceToBaseline(baseline, onlyReal: true); + } finally { + RenderObject.debugCheckingIntrinsics = false; + } + assert(!RenderObject.debugCheckingIntrinsics); + if (!_debugDryLayoutCalculationValid || dryBaseline == realBaseline) { + continue; + } + if ((dryBaseline == null) != (realBaseline == null)) { + final (String methodReturnedNull, String methodReturnedNonNull) = dryBaseline == null + ? ('computeDryBaseline', 'computeDistanceToActualBaseline') + : ('computeDistanceToActualBaseline', 'computeDryBaseline'); + throw FlutterError.fromParts([ + ErrorSummary( + 'The $baseline location returned by ${objectRuntimeType(this, 'RenderBox')}.computeDistanceToActualBaseline ' + 'differs from the baseline location computed by computeDryBaseline.' + ), + ErrorDescription( + 'The $methodReturnedNull method returned null while the $methodReturnedNonNull returned a non-null $baseline of ${dryBaseline ?? realBaseline}. ' + 'Did you forget to implement $methodReturnedNull for ${objectRuntimeType(this, 'RenderBox')}?' + ), + ...messages, + ]); + } else { + throw FlutterError.fromParts([ + ErrorSummary( + 'The $baseline location returned by ${objectRuntimeType(this, 'RenderBox')}.computeDistanceToActualBaseline ' + 'differs from the baseline location computed by computeDryBaseline.' + ), + DiagnosticsProperty( + 'The RenderBox was', + this, + ), + ErrorDescription( + 'The computeDryBaseline method returned $dryBaseline,\n' + 'while the computeDistanceToActualBaseline method returned $realBaseline.\n' + 'Consider checking the implementations of the following methods on the ${objectRuntimeType(this, 'RenderBox')} class and make sure they are consistent:\n' + ' * computeDistanceToActualBaseline\n' + ' * computeDryBaseline\n' + ' * performLayout\n' + ), + ...messages, + ]); + } + } return true; - } - return false; + }()); } @override void markNeedsLayout() { - if (_clearCachedData() && parent is RenderObject) { + // If `_layoutCacheStorage.clear` returns true, then this [RenderBox]'s layout + // is used by the parent's layout algorithm (it's possible that the parent + // only used the intrinsics for paint, but there's no good way to detect that + // so we conservatively assume it's a layout dependency). + // + // A render object's performLayout implementation may depend on the baseline + // location or the intrinsic dimensions of a descendant, even when there are + // relayout boundaries between them. The `_layoutCacheStorage` being non-empty + // indicates that the parent depended on this RenderBox's baseline location, + // or intrinsic sizes, and thus may need relayout, regardless of relayout + // boundaries. + // + // Some calculations may fail (dry baseline, for example). The layout + // dependency is still established, but only from the RenderBox that failed + // to compute the dry baseline to the ancestor that queried the dry baseline. + if (_layoutCacheStorage.clear() && parent != null) { markParentNeedsLayout(); return; } super.markNeedsLayout(); } - @override - void layout(Constraints constraints, {bool parentUsesSize = false}) { - if (hasSize && constraints != this.constraints && - _cachedBaselines != null && _cachedBaselines!.isNotEmpty) { - // The cached baselines data may need update if the constraints change. - _cachedBaselines?.clear(); - } - super.layout(constraints, parentUsesSize: parentUsesSize); - } - /// {@macro flutter.rendering.RenderObject.performResize} /// /// By default this method sets [size] to the result of [computeDryLayout] @@ -2710,6 +2964,22 @@ abstract class RenderBox extends RenderObject { @override void debugPaint(PaintingContext context, Offset offset) { assert(() { + // Only perform the baseline checks after `PipelineOwner.flushLayout` completes. + // We can't run this check in the same places we run other intrinsics checks + // (in the `RenderBox.size` setter, or after `performResize`), because + // `getDistanceToBaseline` may depend on the layout of the child so it's + // the safest to only call `getDistanceToBaseline` after the entire tree + // finishes doing layout. + // + // Descendant `RenderObject`s typically call `debugPaint` before their + // parents do. This means the baseline implementations are checked from + // descendants to ancestors, allowing us to spot the `RenderBox` with an + // inconsistent implementation, instead of its ancestors that only reported + // inconsistent baseline values because one of its ancestors has an + // inconsistent implementation. + if (debugCheckIntrinsicSizes) { + _debugVerifyDryBaselines(); + } if (debugPaintSizeEnabled) { debugPaintSize(context, offset); } @@ -2812,12 +3082,12 @@ mixin RenderBoxContainerDefaultsMixin menuItems = ['one', 'two', 'three', 'four']; void onChanged(T _) { } -final Type dropdownButtonType = DropdownButton( - onChanged: (_) { }, - items: const >[], -).runtimeType; - Finder _iconRichText(Key iconKey) { return find.descendant( of: find.byKey(iconKey), @@ -566,8 +561,7 @@ void main() { } }); - testWidgets('DropdownButtonFormField with isDense:true does not clip large scale text', - (WidgetTester tester) async { + testWidgets('DropdownButtonFormField with isDense:true does not clip large scale text', (WidgetTester tester) async { final Key buttonKey = UniqueKey(); const String value = 'two'; @@ -588,9 +582,11 @@ void main() { return DropdownMenuItem( key: ValueKey(item), value: item, - child: Text(item, - key: ValueKey('${item}Text'), - style: const TextStyle(fontSize: 20.0)), + child: Text( + item, + key: ValueKey('${item}Text'), + style: const TextStyle(fontSize: 20.0), + ), ); }).toList(), ), @@ -601,8 +597,7 @@ void main() { ), ); - final RenderBox box = - tester.renderObject(find.byType(dropdownButtonType)); + final RenderBox box = tester.renderObject(find.byType(DropdownButton)); expect(box.size.height, 64.0); }); @@ -633,7 +628,7 @@ void main() { ), ); - final RenderBox box = tester.renderObject(find.byType(dropdownButtonType)); + final RenderBox box = tester.renderObject(find.byType(DropdownButton)); expect(box.size.height, 48.0); }); @@ -1077,7 +1072,7 @@ void main() { expect(find.text(currentValue), findsOneWidget); // Tap the DropdownButtonFormField widget - await tester.tap(find.byType(dropdownButtonType)); + await tester.tap(find.byType(DropdownButton)); await tester.pumpAndSettle(); // Tap the first dropdown menu item. diff --git a/packages/flutter/test/material/toggle_buttons_test.dart b/packages/flutter/test/material/toggle_buttons_test.dart index 0e38e4612e5840..c9bea29d648b6c 100644 --- a/packages/flutter/test/material/toggle_buttons_test.dart +++ b/packages/flutter/test/material/toggle_buttons_test.dart @@ -1274,7 +1274,7 @@ void main() { }); testWidgets('Material2 - ToggleButtons text baseline alignment', (WidgetTester tester) async { - // The point size of the fonts must be a multiple of 4 until + // The font size must be a multiple of 4 until // https://github.com/flutter/flutter/issues/122066 is resolved. await tester.pumpWidget( boilerplate( diff --git a/packages/flutter/test/rendering/box_test.dart b/packages/flutter/test/rendering/box_test.dart index 33bbab592d8a32..25a11b7d55aaab 100644 --- a/packages/flutter/test/rendering/box_test.dart +++ b/packages/flutter/test/rendering/box_test.dart @@ -1227,6 +1227,23 @@ void main() { ); layout(goodRoot, onErrors: () { assert(false); }); }); + + group('BaselineOffset', () { + test('minOf', () { + expect(BaselineOffset.noBaseline.minOf(BaselineOffset.noBaseline), BaselineOffset.noBaseline); + + expect(BaselineOffset.noBaseline.minOf(const BaselineOffset(1)), const BaselineOffset(1)); + expect(const BaselineOffset(1).minOf(BaselineOffset.noBaseline), const BaselineOffset(1)); + + expect(const BaselineOffset(2).minOf(const BaselineOffset(1)), const BaselineOffset(1)); + expect(const BaselineOffset(1).minOf(const BaselineOffset(2)), const BaselineOffset(1)); + }); + + test('+', () { + expect(BaselineOffset.noBaseline + 2, BaselineOffset.noBaseline); + expect(const BaselineOffset(1) + 2, const BaselineOffset(3)); + }); + }); } class _DummyHitTestTarget implements HitTestTarget { diff --git a/packages/flutter/test/rendering/cached_intrinsics_test.dart b/packages/flutter/test/rendering/cached_intrinsics_test.dart index bc42ef20c4c0e8..800625d2579c78 100644 --- a/packages/flutter/test/rendering/cached_intrinsics_test.dart +++ b/packages/flutter/test/rendering/cached_intrinsics_test.dart @@ -37,6 +37,34 @@ class RenderTestBox extends RenderBox { } } +class RenderDryBaselineTestBox extends RenderTestBox { + double? baselineOverride; + + @override + double? computeDryBaseline(covariant BoxConstraints constraints, TextBaseline baseline) { + calls += 1; + return baselineOverride ?? constraints.biggest.height / 2.0; + } +} + +class RenderBadDryBaselineTestBox extends RenderTestBox { + @override + double? computeDryBaseline(covariant BoxConstraints constraints, TextBaseline baseline) { + return size.height / 2.0; + } +} + +class RenderCannotComputeDryBaselineTestBox extends RenderTestBox { + bool shouldAssert = true; + @override + double? computeDryBaseline(covariant BoxConstraints constraints, TextBaseline baseline) { + if (shouldAssert) { + assert(debugCannotComputeDryLayout(reason: 'no dry baseline for you')); + } + return null; + } +} + void main() { TestRenderingFlutterBinding.ensureInitialized(); @@ -134,4 +162,99 @@ void main() { expect(test.calls, 3); // Use the cached data if the layout constraints do not change. }); + + group('Dry baseline', () { + test('computeDryBaseline results are cached and shared with computeDistanceToActualBaseline', () { + const double viewHeight = 200.0; + const BoxConstraints constraints = BoxConstraints.tightFor(width: 200.0, height: viewHeight); + final RenderDryBaselineTestBox test = RenderDryBaselineTestBox(); + final RenderBox baseline = RenderBaseline( + baseline: 0.0, + baselineType: TextBaseline.alphabetic, + child: test, + ); + + final RenderConstrainedBox root = RenderConstrainedBox( + additionalConstraints: constraints, + child: baseline, + ); + + layout(RenderPositionedBox(child: root)); + expect(test.calls, 1); + + // The baseline widget loosens the input constraints when passing on to child. + expect(test.getDryBaseline(constraints.loosen(), TextBaseline.alphabetic), test.boxSize.height / 2); + // There's cache for the constraints so this should be 1, but we always evaluate + // computeDryBaseline in debug mode in case it asserts even if the baseline + // cache hits. + expect(test.calls, 2); + + const BoxConstraints newConstraints = BoxConstraints.tightFor(width: 10.0, height: 10.0); + expect(test.getDryBaseline(newConstraints.loosen(), TextBaseline.alphabetic), 5.0); + // Should be 3 but there's an additional computeDryBaseline call in getDryBaseline, + // in an assert. + expect(test.calls, 4); + + root.additionalConstraints = newConstraints; + pumpFrame(); + expect(test.calls, 4); + }); + + test('Asserts when a RenderBox cannot compute dry baseline', () { + final RenderCannotComputeDryBaselineTestBox test = RenderCannotComputeDryBaselineTestBox(); + layout(RenderBaseline(baseline: 0.0, baselineType: TextBaseline.alphabetic, child: test)); + + final BoxConstraints incomingConstraints = test.constraints; + assert(incomingConstraints != const BoxConstraints()); + expect( + () => test.getDryBaseline(const BoxConstraints(), TextBaseline.alphabetic), + throwsA(isA().having((AssertionError e) => e.message, 'message', contains('no dry baseline for you'))), + ); + + // Still throws when there is cache. + expect( + () => test.getDryBaseline(incomingConstraints, TextBaseline.alphabetic), + throwsA(isA().having((AssertionError e) => e.message, 'message', contains('no dry baseline for you'))), + ); + }); + + test('Cactches inconsistencies between computeDryBaseline and computeDistanceToActualBaseline', () { + final RenderDryBaselineTestBox test = RenderDryBaselineTestBox(); + layout(test, phase: EnginePhase.composite); + + FlutterErrorDetails? error; + test.markNeedsLayout(); + test.baselineOverride = 123; + pumpFrame(phase: EnginePhase.composite, onErrors: () { + error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails(); + }); + + expect(error?.exceptionAsString(), contains('differs from the baseline location computed by computeDryBaseline')); + }); + + test('Accessing RenderBox.size in computeDryBaseline is not allowed', () { + final RenderBadDryBaselineTestBox test = RenderBadDryBaselineTestBox(); + FlutterErrorDetails? error; + layout(test, phase: EnginePhase.composite, onErrors: () { + error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails(); + }); + + expect(error?.exceptionAsString(), contains('RenderBox.size accessed in RenderBadDryBaselineTestBox.computeDryBaseline.')); + }); + + test('debug baseline checks do not freak out when debugCannotComputeDryLayout is called', () { + FlutterErrorDetails? error; + void onErrors() { + error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails(); + } + final RenderCannotComputeDryBaselineTestBox test = RenderCannotComputeDryBaselineTestBox(); + layout(test, phase: EnginePhase.composite, onErrors: onErrors); + expect(error, isNull); + + test.shouldAssert = false; + test.markNeedsLayout(); + pumpFrame(phase: EnginePhase.composite, onErrors: onErrors); + expect(error?.exceptionAsString(), contains('differs from the baseline location computed by computeDryBaseline')); + }); + }); } diff --git a/packages/flutter/test/rendering/stack_test.dart b/packages/flutter/test/rendering/stack_test.dart index 5edbb8f1e8a450..bfb2e813cde4ce 100644 --- a/packages/flutter/test/rendering/stack_test.dart +++ b/packages/flutter/test/rendering/stack_test.dart @@ -119,6 +119,7 @@ void main() { visitedChildren.add(child); } + layout(stack); stack.visitChildrenForSemantics(visitor); expect(visitedChildren, hasLength(1)); diff --git a/packages/flutter/test/widgets/baseline_test.dart b/packages/flutter/test/widgets/baseline_test.dart index 707707fdc8fa4d..b3cb839d316166 100644 --- a/packages/flutter/test/widgets/baseline_test.dart +++ b/packages/flutter/test/widgets/baseline_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { @@ -44,6 +45,8 @@ void main() { }); testWidgets('Chip caches baseline', (WidgetTester tester) async { + final bool checkIntrinsicSizes = debugCheckIntrinsicSizes; + debugCheckIntrinsicSizes = false; int calls = 0; await tester.pumpWidget( MaterialApp( @@ -53,6 +56,7 @@ void main() { baselineType: TextBaseline.alphabetic, child: Chip( label: BaselineDetector(() { + assert(!debugCheckIntrinsicSizes); calls += 1; }), ), @@ -66,9 +70,12 @@ void main() { tester.renderObject(find.byType(BaselineDetector)).dirty(); await tester.pump(); expect(calls, 2); + debugCheckIntrinsicSizes = checkIntrinsicSizes; }); testWidgets('ListTile caches baseline', (WidgetTester tester) async { + final bool checkIntrinsicSizes = debugCheckIntrinsicSizes; + debugCheckIntrinsicSizes = false; int calls = 0; await tester.pumpWidget( MaterialApp( @@ -78,6 +85,7 @@ void main() { baselineType: TextBaseline.alphabetic, child: ListTile( title: BaselineDetector(() { + assert(!debugCheckIntrinsicSizes); calls += 1; }), ), @@ -91,6 +99,7 @@ void main() { tester.renderObject(find.byType(BaselineDetector)).dirty(); await tester.pump(); expect(calls, 2); + debugCheckIntrinsicSizes = checkIntrinsicSizes; }); testWidgets("LayoutBuilder returns child's baseline", (WidgetTester tester) async {