Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller Scene] Add DisplayList OP and Dart bindings #38676

Merged
merged 24 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,8 @@ ORIGIN: ../../../flutter/lib/ui/dart_runtime_hooks.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/dart_ui.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/dart_ui.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/dart_wrapper.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/experiments/compositing_3d.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/experiments/scene.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/experiments/setup_hooks.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/experiments/ui.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/geometry.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/hash_codes.dart + ../../../flutter/LICENSE
Expand Down Expand Up @@ -1783,6 +1784,10 @@ ORIGIN: ../../../flutter/lib/ui/painting/picture_recorder.cc + ../../../flutter/
ORIGIN: ../../../flutter/lib/ui/painting/picture_recorder.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/rrect.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/rrect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/scene/scene_node.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/scene/scene_node.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/scene/scene_shader.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/scene/scene_shader.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/shader.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/shader.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/single_frame_codec.cc + ../../../flutter/LICENSE
Expand All @@ -1805,6 +1810,7 @@ ORIGIN: ../../../flutter/lib/ui/semantics/semantics_update_builder.cc + ../../..
ORIGIN: ../../../flutter/lib/ui/semantics/semantics_update_builder.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/semantics/string_attribute.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/semantics/string_attribute.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/setup_hooks.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/snapshot_delegate.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4175,7 +4181,8 @@ FILE: ../../../flutter/lib/ui/dart_runtime_hooks.h
FILE: ../../../flutter/lib/ui/dart_ui.cc
FILE: ../../../flutter/lib/ui/dart_ui.h
FILE: ../../../flutter/lib/ui/dart_wrapper.h
FILE: ../../../flutter/lib/ui/experiments/compositing_3d.dart
FILE: ../../../flutter/lib/ui/experiments/scene.dart
FILE: ../../../flutter/lib/ui/experiments/setup_hooks.dart
FILE: ../../../flutter/lib/ui/experiments/ui.dart
FILE: ../../../flutter/lib/ui/geometry.dart
FILE: ../../../flutter/lib/ui/hash_codes.dart
Expand Down Expand Up @@ -4255,6 +4262,10 @@ FILE: ../../../flutter/lib/ui/painting/picture_recorder.cc
FILE: ../../../flutter/lib/ui/painting/picture_recorder.h
FILE: ../../../flutter/lib/ui/painting/rrect.cc
FILE: ../../../flutter/lib/ui/painting/rrect.h
FILE: ../../../flutter/lib/ui/painting/scene/scene_node.cc
FILE: ../../../flutter/lib/ui/painting/scene/scene_node.h
FILE: ../../../flutter/lib/ui/painting/scene/scene_shader.cc
FILE: ../../../flutter/lib/ui/painting/scene/scene_shader.h
FILE: ../../../flutter/lib/ui/painting/shader.cc
FILE: ../../../flutter/lib/ui/painting/shader.h
FILE: ../../../flutter/lib/ui/painting/single_frame_codec.cc
Expand All @@ -4277,6 +4288,7 @@ FILE: ../../../flutter/lib/ui/semantics/semantics_update_builder.cc
FILE: ../../../flutter/lib/ui/semantics/semantics_update_builder.h
FILE: ../../../flutter/lib/ui/semantics/string_attribute.cc
FILE: ../../../flutter/lib/ui/semantics/string_attribute.h
FILE: ../../../flutter/lib/ui/setup_hooks.dart
FILE: ../../../flutter/lib/ui/snapshot_delegate.h
FILE: ../../../flutter/lib/ui/text.dart
FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc
Expand Down
8 changes: 8 additions & 0 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ source_set("display_list") {
"//flutter/impeller/runtime_stage",
"//third_party/skia",
]

if (!defined(defines)) {
defines = []
}
if (impeller_enable_3d) {
defines += [ "IMPELLER_ENABLE_3D" ]
public_deps += [ "//flutter/impeller/scene" ]
}
}

if (enable_unittests) {
Expand Down
9 changes: 9 additions & 0 deletions display_list/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ void DisplayList::Dispatch(Dispatcher& dispatcher,
break;

FOR_EACH_DISPLAY_LIST_OP(DL_OP_DISPATCH)
#ifdef IMPELLER_ENABLE_3D
DL_OP_DISPATCH(SetSceneColorSource)
#endif // IMPELLER_ENABLE_3D

#undef DL_OP_DISPATCH

Expand All @@ -209,6 +212,9 @@ void DisplayList::DisposeOps(uint8_t* ptr, uint8_t* end) {
break;

FOR_EACH_DISPLAY_LIST_OP(DL_OP_DISPOSE)
#ifdef IMPELLER_ENABLE_3D
DL_OP_DISPOSE(SetSceneColorSource)
#endif // IMPELLER_ENABLE_3D

#undef DL_OP_DISPOSE

Expand Down Expand Up @@ -247,6 +253,9 @@ static bool CompareOps(uint8_t* ptrA,
break;

FOR_EACH_DISPLAY_LIST_OP(DL_OP_EQUALS)
#ifdef IMPELLER_ENABLE_3D
DL_OP_EQUALS(SetSceneColorSource)
#endif // IMPELLER_ENABLE_3D

#undef DL_OP_EQUALS

Expand Down
7 changes: 6 additions & 1 deletion display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,12 @@ namespace flutter {
V(DrawShadowTransparentOccluder)

#define DL_OP_TO_ENUM_VALUE(name) k##name,
enum class DisplayListOpType { FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE) };
enum class DisplayListOpType {
FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE)
#ifdef IMPELLER_ENABLE_3D
DL_OP_TO_ENUM_VALUE(SetSceneColorSource)
#endif // IMPELLER_ENABLE_3D
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like a lot of these ifdef blocks could be eliminated in favor of a single ifdef that controls whether or not the FOR_EACH macro contains V(SetSceneColorSource) If that V() entry doesn't exist then the code isn't generated for that op (and vice versa).

};
#undef DL_OP_TO_ENUM_VALUE

class Dispatcher;
Expand Down
9 changes: 9 additions & 0 deletions display_list/display_list_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "flutter/display_list/display_list_canvas_dispatcher.h"
#include "flutter/display_list/display_list_color_source.h"
#include "flutter/display_list/display_list_ops.h"
#include "fml/logging.h"

namespace flutter {

Expand Down Expand Up @@ -202,6 +203,14 @@ void DisplayListBuilder::onSetColorSource(const DlColorSource* source) {
Push<SetRuntimeEffectColorSourceOp>(0, 0, effect);
break;
}
#ifdef IMPELLER_ENABLE_3D
case DlColorSourceType::kScene: {
const DlSceneColorSource* scene = source->asScene();
FML_DCHECK(scene);
Push<SetSceneColorSourceOp>(0, 0, scene);
break;
}
#endif // IMPELLER_ENABLE_3D
case DlColorSourceType::kUnknown:
Push<SetSkColorSourceOp>(0, 0, source->skia_object());
break;
Expand Down
57 changes: 57 additions & 0 deletions display_list/display_list_color_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
#include "third_party/skia/include/effects/SkGradientShader.h"
#include "third_party/skia/include/effects/SkRuntimeEffect.h"

#ifdef IMPELLER_ENABLE_3D
#include "impeller/geometry/matrix.h" // nogncheck
#include "impeller/scene/node.h" // nogncheck
namespace flutter {
class DlSceneColorSource;
}
#endif // IMPELLER_ENABLE_3D

namespace flutter {

class DlColorColorSource;
Expand Down Expand Up @@ -51,6 +59,9 @@ enum class DlColorSourceType {
kConicalGradient,
kSweepGradient,
kRuntimeEffect,
#ifdef IMPELLER_ENABLE_3D
kScene,
#endif // IMPELLER_ENABLE_3D
kUnknown
};

Expand Down Expand Up @@ -160,6 +171,10 @@ class DlColorSource
return nullptr;
}

#ifdef IMPELLER_ENABLE_3D
virtual const DlSceneColorSource* asScene() const { return nullptr; }
#endif // IMPELLER_ENABLE_3D

// If this filter contains images, specifies the owning context for those
// images.
// Images with a DlImage::OwningContext::kRaster must only call skia_object
Expand Down Expand Up @@ -754,6 +769,48 @@ class DlRuntimeEffectColorSource final : public DlColorSource {
FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlRuntimeEffectColorSource);
};

#ifdef IMPELLER_ENABLE_3D
class DlSceneColorSource final : public DlColorSource {
public:
DlSceneColorSource(std::shared_ptr<impeller::scene::Node> node,
impeller::Matrix camera_matrix)
: node_(std::move(node)), camera_matrix_(camera_matrix) {}

const DlSceneColorSource* asScene() const override { return this; }

std::shared_ptr<DlColorSource> shared() const override {
return std::make_shared<DlSceneColorSource>(node_, camera_matrix_);
}

DlColorSourceType type() const override { return DlColorSourceType::kScene; }
size_t size() const override { return sizeof(*this); }

bool is_opaque() const override { return false; }

std::shared_ptr<impeller::scene::Node> scene_node() const { return node_; }

impeller::Matrix camera_matrix() const { return camera_matrix_; }

sk_sp<SkShader> skia_object() const override { return nullptr; }

protected:
bool equals_(DlColorSource const& other) const override {
FML_DCHECK(other.type() == DlColorSourceType::kScene);
auto that = static_cast<DlSceneColorSource const*>(&other);
if (node_ != that->node_) {
return false;
}
return true;
}

private:
std::shared_ptr<impeller::scene::Node> node_;
impeller::Matrix camera_matrix_; // the view-projection matrix of the scene.

FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlSceneColorSource);
};
#endif // IMPELLER_ENABLE_3D

class DlUnknownColorSource final : public DlColorSource {
public:
DlUnknownColorSource(sk_sp<SkShader> shader)
Expand Down
20 changes: 20 additions & 0 deletions display_list/display_list_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,26 @@ struct SetRuntimeEffectColorSourceOp : DLOp {
}
};

#ifdef IMPELLER_ENABLE_3D
struct SetSceneColorSourceOp : DLOp {
static const auto kType = DisplayListOpType::kSetSceneColorSource;

SetSceneColorSourceOp(const DlSceneColorSource* source)
: source(source->scene_node(), source->camera_matrix()) {}

const DlSceneColorSource source;

void dispatch(DispatchContext& ctx) const {
ctx.dispatcher.setColorSource(&source);
}

DisplayListCompare equals(const SetSceneColorSourceOp* other) const {
return (source == other->source) ? DisplayListCompare::kEqual
: DisplayListCompare::kNotEqual;
}
};
#endif // IMPELLER_ENABLE_3D

// 4 byte header + 16 byte payload uses 24 total bytes (4 bytes unused)
struct SetSharedImageFilterOp : DLOp {
static const auto kType = DisplayListOpType::kSetSharedImageFilter;
Expand Down
14 changes: 14 additions & 0 deletions impeller/display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ impeller_component("display_list") {
"//flutter/fml",
"//third_party/skia",
]

if (!defined(defines)) {
defines = []
}
if (impeller_enable_3d) {
defines += [ "IMPELLER_ENABLE_3D" ]
Copy link
Member

Choose a reason for hiding this comment

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

Why behind a flag?

Copy link
Member

Choose a reason for hiding this comment

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

When I added the stubs originally, I believe there were some lint complaints about unused code.

Copy link
Member Author

@bdero bdero Jan 9, 2023

Choose a reason for hiding this comment

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

Fuchsia doesn't build the shaders right now, so I ended up guarding everything to avoid the Fuchsia build depending on Impeller shader headers (Nodes depend on Meshes, Meshes depend on Materials, Materials depend on the shader headers).

Copy link
Member Author

@bdero bdero Jan 9, 2023

Choose a reason for hiding this comment

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

A cleaner solution would probably be to have the DL op and friends use an agnostic type (e.g. DlSceneNode) and compile an empty version that debug errors when 3D support is off.

}
}

impeller_component("display_list_unittests") {
Expand All @@ -37,4 +44,11 @@ impeller_component("display_list_unittests") {
":display_list",
"../playground:playground_test",
]

if (!defined(defines)) {
defines = []
}
if (impeller_enable_3d) {
defines += [ "IMPELLER_ENABLE_3D" ]
}
}
25 changes: 17 additions & 8 deletions impeller/display_list/display_list_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ static std::optional<Paint::ColorSourceType> ToColorSourceType(
return Paint::ColorSourceType::kSweepGradient;
case flutter::DlColorSourceType::kRuntimeEffect:
return Paint::ColorSourceType::kRuntimeEffect;
#ifdef IMPELLER_ENABLE_3D
case flutter::DlColorSourceType::kScene:
return Paint::ColorSourceType::kScene;
#endif // IMPELLER_ENABLE_3D
case flutter::DlColorSourceType::kUnknown:
return std::nullopt;
}
Expand Down Expand Up @@ -502,19 +506,24 @@ void DisplayListDispatcher::setColorSource(
return;
}
case Paint::ColorSourceType::kScene: {
// const flutter::DlSceneColorSource* scene_color_source =
// source->asScene(); std::shared_ptr<scene::Node> scene_node =
// scene_color_source->node(); Matrix camera_transform =
// scene_color_node->camera_transform();
#ifdef IMPELLER_ENABLE_3D
const flutter::DlSceneColorSource* scene_color_source = source->asScene();
std::shared_ptr<scene::Node> scene_node =
scene_color_source->scene_node();
Matrix camera_transform = scene_color_source->camera_matrix();

paint_.color_source = [/*scene_node, camera_transform*/]() {
paint_.color_source = [scene_node, camera_transform]() {
auto contents = std::make_shared<SceneContents>();
// contents->SetNode(scene_node);
// contents->SetCameraTransform(camera_transform);
contents->SetNode(scene_node);
contents->SetCameraTransform(camera_transform);
return contents;
};
}
#else // IMPELLER_ENABLE_3D
FML_LOG(ERROR) << "ColorSourceType::kScene can only be used if Impeller "
"Scene is enabled.";
#endif // IMPELLER_ENABLE_3D
return;
}
case Paint::ColorSourceType::kConicalGradient:
UNIMPLEMENTED;
break;
Expand Down
28 changes: 28 additions & 0 deletions impeller/display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "impeller/geometry/constants.h"
#include "impeller/geometry/point.h"
#include "impeller/playground/widgets.h"
#include "impeller/scene/node.h"
#include "third_party/imgui/imgui.h"
#include "third_party/skia/include/core/SkBlurTypes.h"
#include "third_party/skia/include/core/SkClipOp.h"
Expand Down Expand Up @@ -1158,5 +1159,32 @@ TEST_P(DisplayListTest, DrawShapes) {
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}

#ifdef IMPELLER_ENABLE_3D
TEST_P(DisplayListTest, SceneColorSource) {
// Load up the scene.
auto mapping =
flutter::testing::OpenFixtureAsMapping("flutter_logo_baked.glb.ipscene");
ASSERT_NE(mapping, nullptr);

std::shared_ptr<scene::Node> gltf_scene =
impeller::scene::Node::MakeFromFlatbuffer(
*mapping, *GetContext()->GetResourceAllocator());
ASSERT_NE(gltf_scene, nullptr);

flutter::DisplayListBuilder builder;

auto color_source = std::make_shared<flutter::DlSceneColorSource>(
gltf_scene,
Matrix::MakePerspective(Degrees(45), GetWindowSize(), 0.1, 1000) *
Matrix::MakeLookAt({3, 2, -5}, {0, 0, 0}, {0, 1, 0}));

flutter::DlPaint paint = flutter::DlPaint().setColorSource(color_source);

builder.drawPaint(paint);

ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}
#endif

} // namespace testing
} // namespace impeller
2 changes: 2 additions & 0 deletions impeller/entity/contents/scene_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ bool SceneContents::Render(const ContentContext& renderer,
TiledTextureContents contents;
contents.SetGeometry(GetGeometry());
contents.SetTexture(subpass_target.GetRenderTargetTexture());
contents.SetMatrix(
Matrix::MakeScale(1 / entity.GetTransformation().GetScale()));
return contents.Render(renderer, entity, pass);
}

Expand Down
19 changes: 13 additions & 6 deletions impeller/scene/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,19 @@ Matrix Node::GetGlobalTransform() const {
}

bool Node::AddChild(std::shared_ptr<Node> node) {
// This ensures that cycles are impossible.
if (node->parent_ != nullptr) {
VALIDATION_LOG
<< "Cannot add a node as a child which already has a parent.";
return false;
}
// TODO(bdero): Figure out a better paradigm/rules for nodes with multiple
// parents. We should probably disallow this, make deep
// copying of nodes cheap and easy, add mesh instancing, etc.
// Today, the parent link is only used for skin posing, and so
// it's reasonable to not have a check and allow multi-parenting.
// Even still, there should still be some kind of cycle
// prevention/detection, ideally at the protocol level.
//
// if (node->parent_ != nullptr) {
// VALIDATION_LOG
// << "Cannot add a node as a child which already has a parent.";
// return false;
// }
node->parent_ = this;
children_.push_back(std::move(node));

Expand Down
Loading