Introduce methods for computing the baseline location of a RenderBox without affecting the current layout #144655
Conversation
github-actions
bot
added
framework
…without affecting the current layout
LongCatIsLooong
force-pushed
the
RenderBox-dryBaseline-base
branch
from
a77c9d3 to
b7f071e
Compare
LongCatIsLooong
force-pushed
the
RenderBox-dryBaseline-base
branch
from
1a6a7b6 to
f363c34
Compare
LongCatIsLooong
force-pushed
the
RenderBox-dryBaseline-base
branch
from
7caddad to
d60a731
Compare
LongCatIsLooong
force-pushed
the
RenderBox-dryBaseline-base
branch
from
374bc48 to
e283dda
Compare
|
Is there a summary somewhere of what this does compared to |
|
@Hixie https://github.com/flutter/flutter/pull/144655/files#diff-f2acffc9e582820293ac5a6dfbe0f8ac16491e992d4e89e074412b8d637f3cc6R1966-R1978 does that summary make sense to you? well apparently that link doesn't work well if the file's collapsed.
|
|
Perfect, thanks! |
|
|
||
| /// When true, debugAssertDoesMeetConstraints() is currently | ||
| /// executing asserts for verifying the consistent behavior of | ||
| /// When true, a debug method (debugAssertDoesMeetConstraints(), for instance) |
There was a problem hiding this comment.
Can we link to the method:
| /// When true, a debug method (debugAssertDoesMeetConstraints(), for instance) | |
| /// When true, a debug method ([debugAssertDoesMeetConstraints], for instance) |
| /// is currently executing asserts for verifying the consistent behavior of | ||
| /// intrinsic dimensions methods. | ||
| /// | ||
| /// This should only be set by debugAssertDoesMeetConstraints() |
There was a problem hiding this comment.
The first paragraph made it sound like this is also used by other methods, this one says it can only be used by debugAssertDoesMeetConstraints. Can you clarify?
There was a problem hiding this comment.
Removed the line that says "it should only be called in debugAssertDoesMeetConstraints().
|
|
||
| Map<String, String> debugFillTimelineArguments(Map<String, String> timelineArguments, Input input); | ||
| String eventLabel(RenderBox renderBox); | ||
| } |
There was a problem hiding this comment.
memoize is kind of explained in the class comment, but debugFillTimelineArguments and eventLabel probably also deserve a quick doc.
| return result; | ||
| } | ||
| return _computeDryLayout(constraints); | ||
| Size getDryLayout(covariant BoxConstraints constraints) { |
There was a problem hiding this comment.
code organization nit: maybe organize it so that all the "dryLayout" related stuff is grouped together followed by all the "dryBaseline" (since it seems all the intrinsic above are also in their own group)?
| } | ||
|
|
||
| /// Returns the distance from the top of the box to the first baseline of the | ||
| /// box's contents at the given `constraints`, or `null` if this [RenderBox] |
There was a problem hiding this comment.
Not 100% sure, but this reads better to me:
| /// box's contents at the given `constraints`, or `null` if this [RenderBox] | |
| /// box's contents for the given `constraints`, or `null` if this [RenderBox] |
| } | ||
|
|
||
| @override | ||
| void layout(Constraints constraints, {bool parentUsesSize = false}) { |
There was a problem hiding this comment.
For my own understanding: Is it correct that we no longer need to clear the cache here because the cache is constraints aware now?
There was a problem hiding this comment.
right, the dry version and the wet version of the baseline calculation now share the same cache. We can't do the same for size because performLayout may also compute the child offsets while computeDryLayout only computes the size.
| if (debugCheckIntrinsicSizes) { | ||
| _debugVerifyDryBaselines(); | ||
| } |
There was a problem hiding this comment.
This feels slightly out of place here since it is not painting anything? Why is this the best place for it and not where we do the other intrinsics consistency checks?
There was a problem hiding this comment.
I moved the "// Only perform the baseline checks after performLayout." paragraph here.
| // Only perform the baseline checks after performLayout. Currently this | ||
| // method is also run in the RenderBox.size setter and after | ||
| // performResize, and it's not safe to access the baseline location in | ||
| // those two places since it may depend on the child layout. |
There was a problem hiding this comment.
What do you mean by this comment? Isn't this only called from debugPaint?
There was a problem hiding this comment.
uhm I probably meant this should only be called in paint. Updated.
There was a problem hiding this comment.
Oh I think by "this method is also run in the ..." I was referring to other intrinsic checks not this method.
| DiagnosticsProperty<RenderObject>( | ||
| 'The RenderBox was', | ||
| this, | ||
| ), |
There was a problem hiding this comment.
Wondering, why this isn't also part of the previous error message?
There was a problem hiding this comment.
I think the previous case is to catch when people forget to implement computeDistanceToActualBaseline or computeDryBaseline. So which RenderBox instance doesn't really matter, we only have to give people the runtime type of the problematic RenderBox subclass?
| ' * computeDistanceToActualBaseline\n' | ||
| ' * computeDryBaseline\n' | ||
| ' * performLayout\n' | ||
| 'methods of the ${objectRuntimeType(this, 'RenderBox')} class and make sure they are consistent.' |
There was a problem hiding this comment.
Consider rewording this so this line appears above the * list and the * list is the very last thing of the message. Currently, it reads awkwardly.
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
LGTM modulo that one comment about the fallback value
LongCatIsLooong
force-pushed
the
RenderBox-dryBaseline-base
branch
from
0c10f27 to
e891d1d
Compare
LongCatIsLooong
added
the
autosubmit
…nderBox without affecting the current layout (flutter/flutter#144655)
flutter/flutter@f217fc1...d31a85b 2024-03-19 [email protected] Roll Flutter Engine from 4cec701f4635 to d8fbcfbd799c (1 revision) (flutter/flutter#145390) 2024-03-19 [email protected] Roll Flutter Engine from c9fbe6bab899 to 4cec701f4635 (1 revision) (flutter/flutter#145386) 2024-03-19 [email protected] Roll Flutter Engine from de6b8f49b849 to c9fbe6bab899 (2 revisions) (flutter/flutter#145383) 2024-03-19 [email protected] Activate shortcuts based on NumLock state (flutter/flutter#145146) 2024-03-19 [email protected] Roll Flutter Engine from 59519ceee30a to de6b8f49b849 (1 revision) (flutter/flutter#145381) 2024-03-19 [email protected] Roll Flutter Engine from ac8f1f233d6f to 59519ceee30a (2 revisions) (flutter/flutter#145379) 2024-03-19 [email protected] Roll Flutter Engine from 4a86b5b17c39 to ac8f1f233d6f (3 revisions) (flutter/flutter#145375) 2024-03-19 [email protected] Roll Flutter Engine from c0d1b0d5d43f to 4a86b5b17c39 (1 revision) (flutter/flutter#145373) 2024-03-19 [email protected] Roll Flutter Engine from 3d909f14118e to c0d1b0d5d43f (1 revision) (flutter/flutter#145371) 2024-03-19 [email protected] Roll Flutter Engine from 0ee413ee276a to 3d909f14118e (5 revisions) (flutter/flutter#145370) 2024-03-18 [email protected] Roll Flutter Engine from 89df726bf13a to 0ee413ee276a (3 revisions) (flutter/flutter#145365) 2024-03-18 [email protected] Roll Flutter Engine from 3fde3678a357 to 89df726bf13a (1 revision) (flutter/flutter#145359) 2024-03-18 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.24.7 to 3.24.8 (flutter/flutter#145358) 2024-03-18 [email protected] Roll pub packages + update DAP tests (flutter/flutter#145349) 2024-03-18 [email protected] Fix for issue 140372 (flutter/flutter#144947) 2024-03-18 [email protected] Introduce methods for computing the baseline location of a RenderBox without affecting the current layout (flutter/flutter#144655) 2024-03-18 [email protected] Roll Flutter Engine from 016206de75bc to 3fde3678a357 (2 revisions) (flutter/flutter#145350) 2024-03-18 [email protected] Roll Flutter Engine from 90c4d64d410f to 016206de75bc (2 revisions) (flutter/flutter#145345) 2024-03-18 [email protected] Switch hot_mode_dev_cycle_linux__benchmark to run in postsubmit (flutter/flutter#145343) 2024-03-18 [email protected] Roll Flutter Engine from 9162c8309e24 to 90c4d64d410f (1 revision) (flutter/flutter#145342) 2024-03-18 [email protected] Add --no-dds to Mac_arm64_ios version of hot_mode_dev_cycle_ios__benchmark (flutter/flutter#145335) 2024-03-18 [email protected] Roll Flutter Engine from 89fb886a162c to 9162c8309e24 (2 revisions) (flutter/flutter#145336) 2024-03-18 [email protected] Roll Flutter Engine from 86de0f75606f to 89fb886a162c (1 revision) (flutter/flutter#145327) 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
…nderBox without affecting the current layout (flutter/flutter#144655)
3 participants
Extracted from #138369
Introduces
RenderBox.{compute,get}DryBaselinefor computing the baseline location inRenderBox.computeDryLayout.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.