Introduce the PipelineOwner tree by goderbauer · Pull Request #122231 · flutter/flutter

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

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

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

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

Maybe wrap this line

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

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

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

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

I really like that name.

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

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

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

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

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

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.

hannah-hyj pushed a commit to hannah-hyj/flutter that referenced this pull request Mar 11, 2023

Introduce the PipelineOwner tree (flutter#122231)

9165d12

Introduce the PipelineOwner tree

hannah-hyj pushed a commit to hannah-hyj/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

f73c358 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

f73c358 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

Closed

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

f73c358 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

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

a2a282e

	/// [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

Conversation

goderbauer commented Mar 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer Mar 9, 2023 • 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!

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 commented Mar 9, 2023

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

flar commented Mar 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

goderbauer Mar 9, 2023 •

edited

Loading