Introduce caching mechanism during compile semantics tree monorepo and formatted version #161195

chunhtai · 2025-01-06T23:32:42Z

old pr: #150394

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

goderbauer

Some initial comments from my first read-through.

My first impression is that this approach seems more manageable and scalable than the previous one. 👍

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

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

goderbauer · 2025-01-10T18:58:25Z

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

-    if (!_isConfigWritable) {
-      _config = _config.copy();
-      _isConfigWritable = true;
+  void buildSemantics({required Set<int> usedSemanticsIds, required _SemanticsGeometry geometry}) {


What's the difference between buildSemantics and buildSemanticsSubtree? Some documentation would be helpful.

goderbauer · 2025-01-10T19:00:22Z

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

+    required this.blocksUserActions,
+    required this.explicitChildNodes,
+    required this.tagsForChildren,
+  });


For my own understanding: Why are the geometrics not part of the parent data? I thought those are passed down from the parent as well?

geometric can only be determined when we know the shape of semantics tree.

the parentData is used for determine the shape of semantics tree, e.g. explicitChildNode, blocksUseractions.

basically we can't determine both geometry, parentData, and semantics tree shape in one pass.

we could merge them and make parentData mutable and do a two pass to update the parentData, but i felt that is harder to manage.

+1 to keeping parentData immutable. Maybe leave this as a comment here that geometrics are not included because we use parentData to determine the shape of the tree and geometrics can only be calculated after the shape of the tree is clear.

goderbauer · 2025-01-10T19:35:49Z

packages/flutter/lib/src/semantics/semantics.dart

+  @Deprecated(
+    'This was caches for internal calculation and is no longer needed. '
+    'This feature was deprecated after v3.27.0-0.0.pre.',
+  )


FWIW, I think we should deprecate/remove the entire elevation/thickness feature from semantics. It's unused and has never been used.

packages/flutter/lib/src/semantics/semantics.dart

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

goderbauer · 2025-01-10T20:02:11Z

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

+        explicitChildNodes: explicitChildNodesForChildren,
+        tagsForChildren: tagsForChildren,
+      );
+      childSemantics.didUpdateParentData(childParentData);


I wonder if parent data should just be a getter/setter?

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

goderbauer · 2025-01-10T20:25:10Z

How good is our semantics test coverage? Did you do some manual testing on this (e.g. fire up a larger app and navigate around a bit / interact with it with a11y tools for a bit to see if everything works as expected and behaves?)

goderbauer · 2025-01-10T20:30:24Z

Have you had a chance to look at the Google testing failures? Are we breaking something we shouldn't break?

chunhtai · 2025-01-14T23:15:43Z

Have you had a chance to look at the Google testing failures? Are we breaking something we shouldn't break?

They are expected, either a color change due to more consistent id, and an actual bug fix where the test was expecting incorrect semantics tree

chunhtai · 2025-01-14T23:17:26Z

How good is our semantics test coverage? Did you do some manual testing on this (e.g. fire up a larger app and navigate around a bit / interact with it with a11y tools for a bit to see if everything works as expected and behaves?)

I did do some manual test, but i think our test framework is quite good since it had caught a lot of corner cases

chunhtai · 2025-01-27T22:29:13Z

This is ready for review, the google test failure is due to RenderTable doesn't implment the redepthChildren method, so if two table are nested it won't be able to calculate the depth correct which lead to issues when calculate geometry transform in semantics, I will put up another pr to fix it.

chunhtai · 2025-01-27T22:46:08Z

table redepth pr #162282

goderbauer

LGTM

goderbauer · 2025-02-03T22:17:34Z

packages/flutter/lib/src/semantics/semantics.dart

@@ -2010,6 +2010,10 @@ class SemanticsNode with DiagnosticableTreeMixin {
  /// See also:
  ///
  ///  * [elevation], the actual elevation of this [SemanticsNode].
+  @Deprecated(
+    'This was caches for internal calculation and is no longer needed. '


Suggested change

'This was caches for internal calculation and is no longer needed. '

'This was a cache for internal calculations and is no longer needed. '

Should we also slap a deprecation notice on the (related) elevation and depth property?

I think depth is used in dirty node compilation, but sgtm to also deprecate elevation related API

goderbauer · 2025-02-03T22:18:26Z

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

@@ -1144,11 +1144,13 @@ class RenderParagraph extends RenderBox
    _semanticsInfo = text.getSemanticsInformation();
    bool needsAssembleSemanticsNode = false;
    bool needsChildConfigurationsDelegate = false;
+    bool hasTextSpan = false;


This appears to be unused?

goderbauer · 2025-02-03T22:25:44Z

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

+        if (!kReleaseMode) {
+          FlutterTimeline.startSync('Semantics.updateChildren');
+        }


Just wondering: Is it useful that each node gets its own timeline event? They are just gonna be back-to-back in the timeline without a way to identify which node is getting processed. Should we instead just do one timeline event for the entire for loop?

yeah that sounds good

goderbauer · 2025-02-03T22:26:16Z

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

+      }());
+
+      for (final RenderObject node in nodesToProcess) {
+        if (!kReleaseMode) {


Same question here and below.

goderbauer · 2025-02-03T22:28:04Z

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

+        }
+      }
+
+      for (final RenderObject node in nodesToProcess.reversed) {


Do the three for loops in this function correspond to the phases documented on _RenderObjectSemantics? If so, maybe leave a comment at the top of the function that links to that explanation there.

goderbauer · 2025-02-03T22:49:15Z

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

+/// Walks the [_childrenAndElevationAdjustments] and produce semantics node for
+/// each [_RenderObjectSemantics] plus the sibling nodes.
+///
+/// Since phase 2, 3, 4 each depends on previous step to finished updating the


Suggested change

/// Since phase 2, 3, 4 each depends on previous step to finished updating the

/// Phase 2, 3, 4 each depends on previous step to finished updating

goderbauer · 2025-02-03T22:50:41Z

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

+/// Phase 1 and 2 are done in [updateChildren], Phase 3 is done in
+/// [ensureGeometry], and phase 4 is done in [ensureSemanticsNode].


Can you move this sentence all the way to the beginning just after "The _RenderObjectSemantics tree is compiled in four phases."? This is the context I was missing and asking for in a previous comment.

(Overall this is a great doc comment, though! Thank you for writing this up and providing context for future readers!)

goderbauer · 2025-02-03T22:51:16Z

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

+  // TODO(chunhtai): Figure out what to do when incomplete fragments are asked
+  // to form a semantics node.


Is that supposed to happen in a real app? Shouldn't it just throw/assert if that's illegal?

right now it is impossible that this can happen because i force them to merge into a _RenderObjectSemantics (by making the assumption in _collectChildMergeUpAndSiblingGroup). but that is not an ideal solution. The ideal solution is that they should form a semantics node if they are asked. but then we need a way for render object to assign geometry to those semantics node which is not possible in the current api

goderbauer · 2025-02-03T22:55:14Z

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

+  /// rendering subtree and forms a [_RenderObjectSemantics] tree where children
+  /// are stored in [_childrenAndElevationAdjustments].
+  ///
+  /// This method does the following:


Maybe explicitly state again that it implements phase x of the four phases documented on [_RenderObjectSemantics]. (and also do this for the other methods that implement a phase)

goderbauer · 2025-02-03T23:00:03Z

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

+  /// This method will in turn call the [_buildSemanticsSubtree].
+  ///
+  /// This method will short-circuit itself if the [cachedSemanticsNode] is


Suggested change

/// This method will in turn call the [_buildSemanticsSubtree].

///

/// This method will short-circuit itself if the [cachedSemanticsNode] is

/// This method will in turn call [_buildSemanticsSubtree].

///

/// This method will short-circuit itself if the [cachedSemanticsNode]

…orepo and formatted version (flutter/flutter#161195)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Jan 6, 2025

chunhtai changed the title ~~Introduce caching mechanism during compile semantics tree~~ Introduce caching mechanism during compile semantics tree monorepo and formatted version Jan 6, 2025

chunhtai mentioned this pull request Jan 6, 2025

Introduce caching mechanism during compile semantics tree #150394

Closed

9 tasks

chunhtai force-pushed the issues/150234-take2 branch from f3d4d12 to d8c7576 Compare January 7, 2025 20:37

chunhtai requested review from goderbauer and mdebbar January 7, 2025 21:37

goderbauer reviewed Jan 10, 2025

View reviewed changes

chunhtai force-pushed the issues/150234-take2 branch 3 times, most recently from 4f39357 to 86a2866 Compare January 13, 2025 22:27

chunhtai requested a review from goderbauer January 14, 2025 23:17

chunhtai force-pushed the issues/150234-take2 branch from 3aa8c92 to 3322d3b Compare January 22, 2025 21:12

github-actions bot added the f: material design flutter/packages/flutter/material repository. label Jan 22, 2025

chunhtai force-pushed the issues/150234-take2 branch from 076a4f0 to 1b32144 Compare January 23, 2025 19:04

github-actions bot added the a: text input Entering text in a text field or keyboard related problems label Jan 23, 2025

chunhtai force-pushed the issues/150234-take2 branch 2 times, most recently from 31e4f89 to a9f05bb Compare January 27, 2025 18:48

goderbauer approved these changes Feb 3, 2025

View reviewed changes

chunhtai added 4 commits February 5, 2025 10:32

Introduce caching mechanism during compile semantics tree

877b40e

update

81973f6

update

98d69ae

update

5f67dc7

engine-flutter-autoroll mentioned this pull request Feb 8, 2025

Roll Flutter from 8e2a6fc3fd2c to 829fe68e9dff (100 revisions) flutter/packages#8589

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

d804fe8

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 8, 2025

Roll Flutter from 8e2a6fc3fd2c to 829fe68e9dff (100 revisions) flutter/packages#8591

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

d9f9db3

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 9, 2025

Roll Flutter from 8e2a6fc3fd2c to 5156de731b45 (102 revisions) flutter/packages#8593

Closed

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

1d85de0 Introduce caching mechanism during compile semantics tree mon…

6b63df1

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 9, 2025

Roll Flutter from 8e2a6fc3fd2c to 5156de731b45 (102 revisions) flutter/packages#8594

Closed

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

1d85de0 Introduce caching mechanism during compile semantics tree mon…

0917c20

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to e7cb7a30bb1c (104 revisions) flutter/packages#8597

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

2d406a3

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to c89d563944e3 (105 revisions) flutter/packages#8599

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

eb0e20b

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 33a4c95de079 (106 revisions) flutter/packages#8600

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

21dc49a

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 1d766ed8d93e (107 revisions) flutter/packages#8601

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

a7b2f34

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 78e5d7995cdb (108 revisions) flutter/packages#8602

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

f6d9f4f

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

6e5d04f

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

75765b9

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

9cde305

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

b63a4ac

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

9c4b3b9

…orepo and formatted version (flutter/flutter#161195)

Piinks mentioned this pull request Feb 14, 2025

Flutter Notable Commits - 2025 #161460

Open

eyebrowsoffire mentioned this pull request Mar 5, 2025

Regression in bench_material3_semantics.skwasm_st.Semantics.GetFragment.average #160448

Closed

chunhtai mentioned this pull request May 19, 2025

Scrolling jank in ListView when semantics are enabled #61193

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

cf1168f

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

fadf3db

…orepo and formatted version (flutter/flutter#161195)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

1d85de0 Introduce caching mechanism during compile semantics tree mon…

9fe4970

…orepo and formatted version (flutter/flutter#161195)

chunhtai mentioned this pull request Jun 6, 2025

Fixes inputDecoration sibling explicit child not included in semantic… #170079

Merged

9 tasks

	'This was caches for internal calculation and is no longer needed. '
	'This was a cache for internal calculations and is no longer needed. '

	/// Since phase 2, 3, 4 each depends on previous step to finished updating the
	/// Phase 2, 3, 4 each depends on previous step to finished updating

		/// Phase 1 and 2 are done in [updateChildren], Phase 3 is done in
		/// [ensureGeometry], and phase 4 is done in [ensureSemanticsNode].

		// TODO(chunhtai): Figure out what to do when incomplete fragments are asked
		// to form a semantics node.

Introduce caching mechanism during compile semantics tree monorepo and formatted version #161195

Introduce caching mechanism during compile semantics tree monorepo and formatted version #161195

Uh oh!

Conversation

chunhtai commented Jan 6, 2025

Pre-launch Checklist

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goderbauer Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goderbauer commented Jan 10, 2025

Uh oh!

goderbauer commented Jan 10, 2025

Uh oh!

chunhtai commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai commented Jan 14, 2025

Uh oh!

chunhtai commented Jan 27, 2025

Uh oh!

chunhtai commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer Jan 10, 2025 •

edited

Loading

chunhtai commented Jan 14, 2025 •

edited

Loading

chunhtai commented Jan 27, 2025 •

edited

Loading