Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@chinmaygarde
Copy link
Member

This moves all the logic of construction of the layer tree into flow.

Copy link
Member

@jason-simmons jason-simmons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

m_currentLayer->Add(std::move(layer));
m_currentLayer = newLayer;
layer_builder_.PushPhysicalModel(
rrect.sk_rrect, //
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add text here or remove these empty comments

(a few other PushModel calls have similar formatting)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added those so clang format displayed them a separate line. I'll add comments.

@chinmaygarde
Copy link
Member Author

Haven't tested this on Fuchsia yet. Will hold off on landing this till I do.

#include "flutter/flow/layers/color_filter_layer.h"
#include "flutter/flow/layers/container_layer.h"
#include "flutter/flow/layers/layer.h"
#include "flutter/flow/layers/layer_builder.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary header should be first, followed by a blank line, followed by the rest of the headers. This pattern ensures that each header can build on its own (and doesn't require other headers to be included first).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@abarth
Copy link
Contributor

abarth commented Oct 11, 2017

LGTM

@chinmaygarde chinmaygarde merged commit 1793f49 into flutter:master Oct 11, 2017
liyuqian added a commit to liyuqian/engine that referenced this pull request Sep 14, 2018
This essentially reverts flutter#4197
as no one is (or soon will be) implementing an alternative LayerBuilder.
Let's just put everything in SceneBuilder to reduce the YAGNI
(you aren't gonna need it) smell. This will also make retained rendering
API changes much easier.
liyuqian added a commit that referenced this pull request Sep 14, 2018
This essentially reverts #4197
as no one is (or soon will be) implementing an alternative LayerBuilder.
Let's just put everything in SceneBuilder to reduce the YAGNI
(you aren't gonna need it) smell. This will also make retained rendering
API changes much easier.
amirh pushed a commit to amirh/engine that referenced this pull request Sep 21, 2018
This essentially reverts flutter#4197
as no one is (or soon will be) implementing an alternative LayerBuilder.
Let's just put everything in SceneBuilder to reduce the YAGNI
(you aren't gonna need it) smell. This will also make retained rendering
API changes much easier.
amirh pushed a commit to amirh/engine that referenced this pull request Sep 21, 2018
This essentially reverts flutter#4197
as no one is (or soon will be) implementing an alternative LayerBuilder.
Let's just put everything in SceneBuilder to reduce the YAGNI
(you aren't gonna need it) smell. This will also make retained rendering
API changes much easier.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants