Unverified Commit 5b2281e7 authored by Dan Field's avatar Dan Field Committed by GitHub

Make Flex only overflow on epsilon (#31890)

parent b04d38be
...@@ -31,3 +31,10 @@ const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue: ...@@ -31,3 +31,10 @@ const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue:
/// a particular block of code will not be executed in debug mode, and hence /// a particular block of code will not be executed in debug mode, and hence
/// can be removed. /// can be removed.
const bool kDebugMode = !kReleaseMode && !kProfileMode; const bool kDebugMode = !kReleaseMode && !kProfileMode;
/// The epsilon of tolerable double precision error.
///
/// This is used in various places in the framework to allow for floating point
/// precision loss in calculations. Differences below this threshold are safe to
/// disregard.
const double precisionErrorTolerance = 1e-10;
...@@ -470,6 +470,9 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl ...@@ -470,6 +470,9 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
// Set during layout if overflow occurred on the main axis. // Set during layout if overflow occurred on the main axis.
double _overflow; double _overflow;
// Check whether any meaningful overflow is present. Values below an epsilon
// are treated as not overflowing.
bool get _hasOverflow => _overflow > precisionErrorTolerance;
@override @override
void setupParentData(RenderBox child) { void setupParentData(RenderBox child) {
...@@ -849,7 +852,6 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl ...@@ -849,7 +852,6 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
} }
actualSizeDelta = actualSize - allocatedSize; actualSizeDelta = actualSize - allocatedSize;
_overflow = math.max(0.0, -actualSizeDelta); _overflow = math.max(0.0, -actualSizeDelta);
final double remainingSpace = math.max(0.0, actualSizeDelta); final double remainingSpace = math.max(0.0, actualSizeDelta);
double leadingSpace; double leadingSpace;
double betweenSpace; double betweenSpace;
...@@ -941,7 +943,7 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl ...@@ -941,7 +943,7 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
@override @override
void paint(PaintingContext context, Offset offset) { void paint(PaintingContext context, Offset offset) {
if (_overflow <= 0.0) { if (!_hasOverflow) {
defaultPaint(context, offset); defaultPaint(context, offset);
return; return;
} }
...@@ -997,12 +999,12 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl ...@@ -997,12 +999,12 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
} }
@override @override
Rect describeApproximatePaintClip(RenderObject child) => _overflow > 0.0 ? Offset.zero & size : null; Rect describeApproximatePaintClip(RenderObject child) => _hasOverflow ? Offset.zero & size : null;
@override @override
String toStringShort() { String toStringShort() {
String header = super.toStringShort(); String header = super.toStringShort();
if (_overflow is double && _overflow > 0.0) if (_overflow is double && _hasOverflow)
header += ' OVERFLOWING'; header += ' OVERFLOWING';
return header; return header;
} }
......
...@@ -690,9 +690,6 @@ class SliverGeometry extends Diagnosticable { ...@@ -690,9 +690,6 @@ class SliverGeometry extends Diagnosticable {
/// * [RenderViewport.cacheExtent] for a description of a viewport's cache area. /// * [RenderViewport.cacheExtent] for a description of a viewport's cache area.
final double cacheExtent; final double cacheExtent;
/// The epsilon of tolerable double precision error.
static const double precisionErrorTolerance = 1e-10;
/// Asserts that this geometry is internally consistent. /// Asserts that this geometry is internally consistent.
/// ///
/// Does nothing if asserts are disabled. Always returns true. /// Does nothing if asserts are disabled. Always returns true.
......
...@@ -120,7 +120,7 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { ...@@ -120,7 +120,7 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
final double firstChildScrollOffset = earliestScrollOffset - paintExtentOf(firstChild); final double firstChildScrollOffset = earliestScrollOffset - paintExtentOf(firstChild);
// firstChildScrollOffset may contain double precision error // firstChildScrollOffset may contain double precision error
if (firstChildScrollOffset < -SliverGeometry.precisionErrorTolerance) { if (firstChildScrollOffset < -precisionErrorTolerance) {
// The first child doesn't fit within the viewport (underflow) and // The first child doesn't fit within the viewport (underflow) and
// there may be additional children above it. Find the real first child // there may be additional children above it. Find the real first child
// and then correct the scroll position so that there's room for all and // and then correct the scroll position so that there's room for all and
......
...@@ -20,6 +20,38 @@ void main() { ...@@ -20,6 +20,38 @@ void main() {
expect(flex.size.height, equals(200.0), reason: 'flex height'); expect(flex.size.height, equals(200.0), reason: 'flex height');
}); });
test('Inconsequential overflow is ignored', () {
// These values are meant to simulate slight rounding errors in addition
// or subtraction in the layout code for Flex.
const double slightlyLarger = 438.8571428571429;
const double slightlySmaller = 438.85714285714283;
final List<dynamic> exceptions = <dynamic>[];
final FlutterExceptionHandler oldHandler = FlutterError.onError;
FlutterError.onError = (FlutterErrorDetails details) {
exceptions.add(details.exception);
};
const BoxConstraints square = BoxConstraints.tightFor(width: slightlyLarger, height: 100.0);
final RenderConstrainedBox box1 = RenderConstrainedBox(additionalConstraints: square);
final RenderFlex flex = RenderFlex(
textDirection: TextDirection.ltr,
mainAxisSize: MainAxisSize.min,
);
final RenderConstrainedOverflowBox parent = RenderConstrainedOverflowBox(
minWidth: 0.0,
maxWidth: slightlySmaller,
minHeight: 0.0,
maxHeight: 400.0,
child: flex,
);
flex.add(box1);
layout(parent);
expect(flex.size, const Size(slightlySmaller, 100.0));
pumpFrame(phase: EnginePhase.paint);
expect(exceptions, isEmpty);
FlutterError.onError = oldHandler;
});
test('Vertical Overflow', () { test('Vertical Overflow', () {
final RenderConstrainedBox flexible = RenderConstrainedBox( final RenderConstrainedBox flexible = RenderConstrainedBox(
additionalConstraints: const BoxConstraints.expand() additionalConstraints: const BoxConstraints.expand()
......
...@@ -207,10 +207,11 @@ Matcher isInstanceOf<T>() => test_package.TypeMatcher<T>(); ...@@ -207,10 +207,11 @@ Matcher isInstanceOf<T>() => test_package.TypeMatcher<T>();
/// Asserts that two [double]s are equal, within some tolerated error. /// Asserts that two [double]s are equal, within some tolerated error.
/// ///
/// Two values are considered equal if the difference between them is within /// Two values are considered equal if the difference between them is within
/// 1e-10 of the larger one. This is an arbitrary value which can be adjusted /// [precisionErrorTolerance] of the larger one. This is an arbitrary value
/// using the `epsilon` argument. This matcher is intended to compare floating /// which can be adjusted using the `epsilon` argument. This matcher is intended
/// point numbers that are the result of different sequences of operations, such /// to compare floating point numbers that are the result of different sequences
/// that they may have accumulated slightly different errors. /// of operations, such that they may have accumulated slightly different
/// errors.
/// ///
/// See also: /// See also:
/// ///
...@@ -218,24 +219,25 @@ Matcher isInstanceOf<T>() => test_package.TypeMatcher<T>(); ...@@ -218,24 +219,25 @@ Matcher isInstanceOf<T>() => test_package.TypeMatcher<T>();
/// required and not named. /// required and not named.
/// * [inInclusiveRange], which matches if the argument is in a specified /// * [inInclusiveRange], which matches if the argument is in a specified
/// range. /// range.
Matcher moreOrLessEquals(double value, { double epsilon = 1e-10 }) { Matcher moreOrLessEquals(double value, { double epsilon = precisionErrorTolerance }) {
return _MoreOrLessEquals(value, epsilon); return _MoreOrLessEquals(value, epsilon);
} }
/// Asserts that two [Rect]s are equal, within some tolerated error. /// Asserts that two [Rect]s are equal, within some tolerated error.
/// ///
/// Two values are considered equal if the difference between them is within /// Two values are considered equal if the difference between them is within
/// 1e-10 of the larger one. This is an arbitrary value which can be adjusted /// [precisionErrorTolerance] of the larger one. This is an arbitrary value
/// using the `epsilon` argument. This matcher is intended to compare floating /// which can be adjusted using the `epsilon` argument. This matcher is intended
/// point numbers that are the result of different sequences of operations, such /// to compare floating point numbers that are the result of different sequences
/// that they may have accumulated slightly different errors. /// of operations, such that they may have accumulated slightly different
/// errors.
/// ///
/// See also: /// See also:
/// ///
/// * [moreOrLessEquals], which is for [double]s. /// * [moreOrLessEquals], which is for [double]s.
/// * [within], which offers a generic version of this functionality that can /// * [within], which offers a generic version of this functionality that can
/// be used to match [Rect]s as well as other types. /// be used to match [Rect]s as well as other types.
Matcher rectMoreOrLessEquals(Rect value, { double epsilon = 1e-10 }) { Matcher rectMoreOrLessEquals(Rect value, { double epsilon = precisionErrorTolerance }) {
return _IsWithinDistance<Rect>(_rectDistance, value, epsilon); return _IsWithinDistance<Rect>(_rectDistance, value, epsilon);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment