Introduce the PipelineOwner tree #122231

goderbauer · 2023-03-08T20:47:14Z

Implements the PipelineOwner tree as specified in https://flutter.dev/go/multiple-views.

gspencergoog · 2023-03-08T23:04:23Z

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

@@ -859,6 +859,20 @@ class _LocalSemanticsHandle implements SemanticsHandle {
 /// are visible on screen. You can create other pipeline owners to manage
 /// off-screen objects, which can flush their pipelines independently of the
 /// on-screen render objects.
+///
+/// [PipelineOwner]s can be organized in a tree to manage multiple render trees,


Suggested change

/// [PipelineOwner]s can be organized in a tree to manage multiple render trees,

/// [PipelineOwner]s are organized in a tree to manage multiple render trees,

Aren't they always arranged this way (even a single main view can have subviews).

I wanted to express here that the PipelineOwner class can also be used stand-alone. It fully functions if it is not part of a tree, for example for the use case mentioned above where you create another pipeline owner to manage off-screen objects. You are not required to turn them into a tree or attach them to any (existing) tree. Of course, one could argue, that just having a single node (the root) is also a form of tree...

Oh, OK, ignore my comment then.

Of course, one could argue, that just having a single node (the root) is also a form of tree...

We don't need to be as pedantic as all that. :-)

gspencergoog · 2023-03-08T23:09:23Z

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

@@ -945,6 +967,7 @@ class PipelineOwner {
  /// always returns false.
  bool get debugDoingLayout => _debugDoingLayout;
  bool _debugDoingLayout = false;
+  bool _debugDoingChildrenLayout = false;


Suggested change

bool _debugDoingChildrenLayout = false;

bool _debugDoingChildLayout = false;

...because it just reads better. I'd either say "I'm doing child layout" or "I'm laying out children", but not "I'm doing children layout".

gspencergoog · 2023-03-08T23:10:35Z

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

-  /// [semanticsOwner] field will revert to null, and the previous owner will be
-  /// disposed.
+  /// An owner is created by [ensureSemantics] or when the [PipelineManifold] to
+  /// which this owner is connected to has [PipelineManifold.enableSemantics]


Suggested change

/// which this owner is connected to has [PipelineManifold.enableSemantics]

/// which this owner is connected has [PipelineManifold.enableSemantics]

gspencergoog · 2023-03-08T23:12:25Z

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

+    _updateSemanticsOwner();
+
+    // If onNeedVisualUpdate is specified, it has already been called when the node was dirtied in the first place.
+    if (onNeedVisualUpdate == null && (_nodesNeedingLayout.isNotEmpty || _nodesNeedingCompositingBitsUpdate.isNotEmpty || _nodesNeedingPaint.isNotEmpty || _nodesNeedingSemantics.isNotEmpty)) {


Maybe wrap this line

gspencergoog · 2023-03-08T23:14:03Z

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

+  /// Adds `child` to this [PipelineOwner].
+  ///
+  /// During the phases of frame production (see [RendererBinding.drawFrame]),
+  /// the parent [PipelineOwner] will complete a phase first for the nodes it


Suggested change

/// the parent [PipelineOwner] will complete a phase first for the nodes it

/// the parent [PipelineOwner] will complete a phase for the nodes it

gspencergoog · 2023-03-08T23:17:10Z

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

+  ///
+  /// No children may be removed after the [PipelineOwner] has started calling
+  /// [flushLayout] on any of its children until the end of the current frame.
+  void dropChild(PipelineOwner child) {


Why use adopt/drop? They're not really opposites of each other. Why not addChild/removeChild? If you want to use adopt, then the opposite is probably "abandon". Which feels a bit bleak. :-)

That's the terminology we use for every other tree node class in the framework:

https://master-api.flutter.dev/flutter/foundation/AbstractNode/adoptChild.html

https://master-api.flutter.dev/flutter/foundation/AbstractNode/dropChild.html

Sigh. Okay, consistency is important, but it's too bad that they aren't opposites.

gspencergoog · 2023-03-08T23:18:48Z

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

+///
+/// [PipelineOwner]s can register listeners with the [PipelineManifold] to be
+/// informed when certain values provided by the [PipelineManifold] change.
+abstract class PipelineManifold implements Listenable {


I really like that name.

gspencergoog · 2023-03-08T23:19:27Z

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

+/// particular binding implementation.
+///
+/// The root of the [PipelineOwner] tree is attached to a [PipelineManifold]
+/// by passing it to [PipelineOwner.attach]. Children are attached to the same


Suggested change

/// by passing it to [PipelineOwner.attach]. Children are attached to the same

/// by passing the manifold to [PipelineOwner.attach]. Children are attached to the same

gspencergoog · 2023-03-08T23:20:15Z

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

+  ///    implementations typically use to back this property.
+  bool get semanticsEnabled;
+
+  /// Called be a [PipelineOwner] connected to this [PipelineManifold] when a


Suggested change

/// Called be a [PipelineOwner] connected to this [PipelineManifold] when a

/// Called by a [PipelineOwner] connected to this [PipelineManifold] when a

gspencergoog · 2023-03-08T23:20:46Z

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

+  bool get semanticsEnabled;
+
+  /// Called be a [PipelineOwner] connected to this [PipelineManifold] when a
+  /// render object associated with that pipeline owner wishes to update its


maybe link RenderObject?

Suggested change

/// render object associated with that pipeline owner wishes to update its

/// [RenderObject] associated with that pipeline owner wishes to update its

goderbauer · 2023-03-09T22:14:33Z

All comments are addressed and all failing checks should pass now. PTAL @gspencergoog.

gspencergoog

flar · 2023-03-10T21:10:58Z

It looks like this PR is causing failures in the tree: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20web_canvaskit_tests_7_last/7125/overview

This reverts commit f73c358.

Introduce the PipelineOwner tree

…22425) This reverts commit f73c358.

This reverts commit 670f9d2.

Reland "Introduce the PipelineOwner tree (#122231)"

flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Mar 8, 2023

goderbauer requested a review from gspencergoog March 8, 2023 20:48

goderbauer mentioned this pull request Mar 8, 2023

☂️ Multi-View rendering (framework-side) #121573

Closed

4 tasks

gspencergoog reviewed Mar 8, 2023

View reviewed changes

goderbauer force-pushed the pipelineOwnerTree branch from 0889d78 to e1220e7 Compare March 9, 2023 17:50

goderbauer requested a review from gspencergoog March 9, 2023 17:50

goderbauer force-pushed the pipelineOwnerTree branch from 448e790 to fb46684 Compare March 9, 2023 22:13

goderbauer added 12 commits March 9, 2023 16:38

Introduce PipelineOwner tree

b011481

some tests

ba2b80f

TODO

988597e

++

f14b38c

more tests

d54fef7

++

e64d2f3

review

442d8b5

fix doc references

927a948

fix another doc reference

f9d26d5

fix CI crash and add test for it

5c664db

fix imports

c212c3e

fix test

a514117

gspencergoog approved these changes Mar 10, 2023

View reviewed changes

goderbauer force-pushed the pipelineOwnerTree branch from 83448f5 to a514117 Compare March 10, 2023 00:50

goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2023

auto-submit bot merged commit f73c358 into flutter:master Mar 10, 2023
33 checks passed

goderbauer deleted the pipelineOwnerTree branch March 10, 2023 19:57

flar mentioned this pull request Mar 10, 2023

Revert "Introduce the PipelineOwner tree" #122425

Merged

flar added a commit that referenced this pull request Mar 10, 2023

Revert "Introduce the PipelineOwner tree (#122231)"

81a8ec9

This reverts commit f73c358.

flar added a commit that referenced this pull request Mar 10, 2023

Revert "Introduce the PipelineOwner tree (#122231)" (#122425)

670f9d2

This reverts commit f73c358.

hangyujin pushed a commit to hangyujin/flutter that referenced this pull request Mar 11, 2023

Introduce the PipelineOwner tree (flutter#122231)

9165d12

Introduce the PipelineOwner tree

hangyujin pushed a commit to hangyujin/flutter that referenced this pull request Mar 11, 2023

Revert "Introduce the PipelineOwner tree (flutter#122231)" (flutter#1…

145f811

…22425) This reverts commit f73c358.

goderbauer added a commit to goderbauer/flutter that referenced this pull request Mar 11, 2023

Reland "Introduce the PipelineOwner tree (flutter#122231)"

3f897bd

This reverts commit 670f9d2.

engine-flutter-autoroll mentioned this pull request Mar 11, 2023

Roll Flutter from 5a279edc239f to f9ad42a32d7e (69 revisions) flutter/packages#3443

Closed

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

f73c358e4 Introduce the PipelineOwner tree (flutter/flutter#122231)

3557649

engine-flutter-autoroll mentioned this pull request Mar 12, 2023

Roll Flutter from 5a279edc239f to 9ba0d08ebc07 (72 revisions) flutter/packages#3445

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2023

f73c358e4 Introduce the PipelineOwner tree (flutter/flutter#122231)

baf08ef

auto-submit bot pushed a commit that referenced this pull request Mar 13, 2023

Reland "Introduce the PipelineOwner tree (#122231)" (#122452)

12ef753

Reland "Introduce the PipelineOwner tree (#122231)"

HansMuller mentioned this pull request Mar 16, 2023

Flutter Framework Team: Notable Commits #121415

Open

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

f73c358e4 Introduce the PipelineOwner tree (flutter/flutter#122231)

60a3948

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

f73c358e4 Introduce the PipelineOwner tree (flutter/flutter#122231)

a2a282e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce the PipelineOwner tree #122231

Introduce the PipelineOwner tree #122231

goderbauer commented Mar 8, 2023

gspencergoog Mar 8, 2023

goderbauer Mar 9, 2023 •

edited

gspencergoog Mar 9, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

goderbauer Mar 9, 2023

gspencergoog Mar 9, 2023

goderbauer Mar 9, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

gspencergoog Mar 8, 2023

goderbauer commented Mar 9, 2023

gspencergoog left a comment

flar commented Mar 10, 2023

	/// [PipelineOwner]s can be organized in a tree to manage multiple render trees,
	/// [PipelineOwner]s are organized in a tree to manage multiple render trees,

	bool _debugDoingChildrenLayout = false;
	bool _debugDoingChildLayout = false;

	/// which this owner is connected to has [PipelineManifold.enableSemantics]
	/// which this owner is connected has [PipelineManifold.enableSemantics]

	/// the parent [PipelineOwner] will complete a phase first for the nodes it
	/// the parent [PipelineOwner] will complete a phase for the nodes it

	/// by passing it to [PipelineOwner.attach]. Children are attached to the same
	/// by passing the manifold to [PipelineOwner.attach]. Children are attached to the same

	/// Called be a [PipelineOwner] connected to this [PipelineManifold] when a
	/// Called by a [PipelineOwner] connected to this [PipelineManifold] when a

	/// render object associated with that pipeline owner wishes to update its
	/// [RenderObject] associated with that pipeline owner wishes to update its

Introduce the PipelineOwner tree #122231

Introduce the PipelineOwner tree #122231

Conversation

goderbauer commented Mar 8, 2023

Choose a reason for hiding this comment

goderbauer Mar 9, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goderbauer commented Mar 9, 2023

gspencergoog left a comment

Choose a reason for hiding this comment

flar commented Mar 10, 2023

goderbauer Mar 9, 2023 •

edited