Make `FittedBox` not throw when child has zero size. #150430

PurplePolyhedron · 2024-06-18T15:27:04Z

Before, FittedBox would throw when child size is zero, unless the constraint is tight and fit is not BoxFit.scaleDown.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

goderbauer

Thank you for the fix and sorry for the delay in reviewing it.

goderbauer · 2024-07-02T22:57:47Z

packages/flutter/lib/src/rendering/proxy_box.dart

- case BoxFit.none:
- size = constraints.constrainSizeAndAttemptToPreserveAspectRatio(child!.size);
+ if (child!.size.isEmpty) {
+ size = constraints.biggest;


Our default is usually constraints.smallest for these cases (see also the else branch bellow). Was there a good reason to use biggest here? Or should this also be smallest?

My bad, it should be constraints.smallest there. I made some mistakes in previous testing and thought the default behaviour of FittedBox is constraints.biggest.

I've changed it to smallest. And I updated computeDryLayout to match performLayout since I forgot about computeDryLayout perviously.

change fitted box default size to constraint.smallest and match computeDryLayout to performLayout

goderbauer

LGTM!

LongCatIsLooong · 2024-07-03T18:27:10Z

packages/flutter/lib/src/rendering/proxy_box.dart

- case BoxFit.none:
- size = constraints.constrainSizeAndAttemptToPreserveAspectRatio(child!.size);
+ if (child!.size.isEmpty) {
+ size = constraints.smallest;


The assertion was in BoxConstraints.constrainSizeAndAttemptToPreserveAspectRatio. I think we should try to fix that method so it can handle an empty size as input?

I am not sure how it should behave when only one side of the target size is 0. For example if height is finite while width is 0, Do you think BoxConstraints.constrainSizeAndAttemptToPreserveAspectRatio should trying to match the height(only constrain size and ignored the aspect ratio), or return constraints.smallest regardless(treat any zero size as empty)?

Yeah I think in your example the method should return constrainSize(inputSize), instead of returning smallest and ignoring the input size.

The reason for that is I think returning smallest when the child size is empty feels like a special case, one that feels a bit artificial (if we do that then the documentation needs to be updated).
Say the input BoxConstraints is loose and unbounded (i.e., BoxConstraints()), then RenderFittedBox always takes the same size as its child when the child size is not empty, so ideally we don't have the special case it when the child size is empty, and always return the child size regardless if it's empty.

remove guard in RenderFittedBox.computeDryLayout since zero size child is now valid.

LongCatIsLooong · 2024-07-08T19:50:22Z

@goderbauer would you like to take another look and see if the updated version makes sense to you?

goderbauer

Still LGTM

LongCatIsLooong

Thank you for contributing!

…er/flutter#150430)

flutter/flutter@fafd67d...5103d75 2024-07-09 [email protected] [tool] Remove some usages of deprecated usage package (flutter/flutter#151359) 2024-07-09 43759233 [email protected] Write the package config location to the test bootstrap. (flutter/flutter#150440) 2024-07-09 [email protected] [deps] Roll dart-lang/native packages (flutter/flutter#151403) 2024-07-08 [email protected] [tool] make `testUsingContext` provide a `Stdio` (with `hasTerminal` unset) override by default (flutter/flutter#151357) 2024-07-08 120297255 [email protected] Make `FittedBox` not throw when child has zero size. (flutter/flutter#150430) 2024-07-08 [email protected] Update `DataTable` documentation (flutter/flutter#151356) 2024-07-08 [email protected] `MaterialState` â�� `WidgetState` in documentation (flutter/flutter#151376) 2024-07-08 41930132 [email protected] [ios]A typical news app benchmark with bottom ad banner (flutter/flutter#150991) 2024-07-08 [email protected] Re-enable `SemanticsAction.focus` matchers (flutter/flutter#150990) 2024-07-08 [email protected] Added SliverFloatingHeader.snapMode (flutter/flutter#151289) 2024-07-08 [email protected] Factor out deprecated names in example code (flutter/flutter#151374) 2024-07-08 [email protected] Add cedric vanden bosch to authors (flutter/flutter#151313) 2024-07-08 [email protected] Roll Packages from 97bad7e to 14341d1 (5 revisions) (flutter/flutter#151417) 2024-07-08 [email protected] Update doc-import to primary configured import, _goldens_io.dart (flutter/flutter#151390) 2024-07-08 [email protected] [Reland] - Enable `explicitChildNodes` for the `AlertDialog` content (flutter/flutter#149597) 2024-07-08 32538273 [email protected] Add tests for material_state_border_side.0_test.dart (flutter/flutter#151089) 2024-07-08 [email protected] Roll Flutter Engine from ca79a56a66d7 to 69075e7e87d4 (1 revision) (flutter/flutter#151393) 2024-07-08 32538273 [email protected] Add tests for action_listener.0.dart (flutter/flutter#150606) 2024-07-07 [email protected] Roll Flutter Engine from 5ca3b856ee5a to ca79a56a66d7 (1 revision) (flutter/flutter#151387) 2024-07-06 [email protected] Roll Flutter Engine from 3600ec613a00 to 5ca3b856ee5a (1 revision) (flutter/flutter#151378) 2024-07-06 [email protected] Roll Flutter Engine from d1ebc5fde630 to 3600ec613a00 (1 revision) (flutter/flutter#151377) 2024-07-06 [email protected] Roll Flutter Engine from e6b09697df1a to d1ebc5fde630 (1 revision) (flutter/flutter#151362) 2024-07-05 49699333 dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.3 to 4.3.4 (flutter/flutter#151354) 2024-07-05 [email protected] Roll Flutter Engine from 4ee09d3b7f3b to e6b09697df1a (2 revisions) (flutter/flutter#151352) 2024-07-05 [email protected] Add tests for color_filtered.0.dart example. (flutter/flutter#151064) 2024-07-05 [email protected] de-duplicate code in analyze.dart (flutter/flutter#151279) 2024-07-05 [email protected] Roll Packages from 754de19 to 97bad7e (1 revision) (flutter/flutter#151350) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md

PurplePolyhedron added 3 commits June 19, 2024 00:33

Update fitted_box layout

20d04d0

Update fitted_box_test

ce90eaa

Update fitted_box_test

9caa556

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 18, 2024

Update fitted_box_test

4dbe44e

goderbauer self-requested a review June 18, 2024 22:02

goderbauer reviewed Jul 2, 2024

View reviewed changes

Update proxy_box

45a4d1b

change fitted box default size to constraint.smallest and match computeDryLayout to performLayout

goderbauer approved these changes Jul 3, 2024

View reviewed changes

LongCatIsLooong reviewed Jul 3, 2024

View reviewed changes

PurplePolyhedron added 6 commits July 9, 2024 02:56

Merge branch 'flutter:master' into fittedbox_size

999a669

revert proxy_box

8a083a7

Update constrainSizeAndAttemptToPreserveAspectRatio

c424553

Update constrainSizeAndAttemptToPreserveAspectRatio test

3921c4a

fix typo

d98b4f9

Update RenderFittedBox.computeDryLayout

f8e485a

remove guard in RenderFittedBox.computeDryLayout since zero size child is now valid.

goderbauer approved these changes Jul 8, 2024

View reviewed changes

LongCatIsLooong approved these changes Jul 8, 2024

View reviewed changes

LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 8, 2024

auto-submit bot merged commit f194cd3 into flutter:master Jul 8, 2024
73 checks passed

engine-flutter-autoroll mentioned this pull request Jul 9, 2024

Roll Flutter from fafd67d34d68 to 9ab8b6e15043 (25 revisions) flutter/packages#7082

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 9, 2024

f194cd329 Make FittedBox not throw when child has zero size. (flutt…

ff91732

…er/flutter#150430)

engine-flutter-autoroll mentioned this pull request Jul 9, 2024

Roll Flutter from fafd67d34d68 to 5ebc993dffe8 (26 revisions) flutter/packages#7083

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 9, 2024

f194cd329 Make FittedBox not throw when child has zero size. (flutt…

13adb77

…er/flutter#150430)

engine-flutter-autoroll mentioned this pull request Jul 9, 2024

Roll Flutter from fafd67d34d68 to 5103d7574361 (27 revisions) flutter/packages#7084

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 9, 2024

f194cd329 Make FittedBox not throw when child has zero size. (flutt…

79c6232

…er/flutter#150430)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `FittedBox` not throw when child has zero size. #150430

Make `FittedBox` not throw when child has zero size. #150430

PurplePolyhedron commented Jun 18, 2024 •

edited

Loading

goderbauer left a comment

goderbauer Jul 2, 2024

PurplePolyhedron Jul 3, 2024 •

edited

Loading

PurplePolyhedron Jul 3, 2024

goderbauer left a comment

LongCatIsLooong Jul 3, 2024

PurplePolyhedron Jul 4, 2024 •

edited

Loading

LongCatIsLooong Jul 4, 2024

LongCatIsLooong commented Jul 8, 2024

goderbauer left a comment

LongCatIsLooong left a comment

Make FittedBox not throw when child has zero size. #150430

Make FittedBox not throw when child has zero size. #150430

Conversation

PurplePolyhedron commented Jun 18, 2024 • edited Loading

Pre-launch Checklist

goderbauer left a comment

Choose a reason for hiding this comment

goderbauer Jul 2, 2024

Choose a reason for hiding this comment

PurplePolyhedron Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

PurplePolyhedron Jul 3, 2024

Choose a reason for hiding this comment

goderbauer left a comment

Choose a reason for hiding this comment

LongCatIsLooong Jul 3, 2024

Choose a reason for hiding this comment

PurplePolyhedron Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

LongCatIsLooong Jul 4, 2024

Choose a reason for hiding this comment

LongCatIsLooong commented Jul 8, 2024

goderbauer left a comment

Choose a reason for hiding this comment

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Make `FittedBox` not throw when child has zero size. #150430

Make `FittedBox` not throw when child has zero size. #150430

PurplePolyhedron commented Jun 18, 2024 •

edited

Loading

PurplePolyhedron Jul 3, 2024 •

edited

Loading

PurplePolyhedron Jul 4, 2024 •

edited

Loading