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

[Impeller] Do not build scene unless 3d define is true #45028

Merged
auto-submit merged 13 commits into from
Aug 24, 2023
Merged

[Impeller] Do not build scene unless 3d define is true #45028

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2fb18cf
[Impeller] Do not build scene unless 3d define is true
dnfield Aug 23, 2023
6913bc8
fix
dnfield Aug 23, 2023
24b7ff1
more
dnfield Aug 23, 2023
2589276
more
dnfield Aug 23, 2023
02580cf
format
dnfield Aug 23, 2023
97065b1
skip 3d test if no 3d
dnfield Aug 23, 2023
0d1d87a
Merge remote-tracking branch 'upstream/main' into 3d
dnfield Aug 23, 2023
9ab6107
test 3d on mac unopt, update malioc
dnfield Aug 24, 2023
79fcdbd
option name
dnfield Aug 24, 2023
a183a3e
config
dnfield Aug 24, 2023
36c9dca
Merge branch 'main' into 3d
dnfield Aug 24, 2023
6260b35
Unintended change
dnfield Aug 24, 2023
4a268b1
Merge branch '3d' of github.com:dnfield/engine into 3d
dnfield Aug 24, 2023
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
3 changes: 2 additions & 1 deletion ci/builders/mac_unopt.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"debug",
"--unoptimized",
"--no-lto",
"--prebuilt-dart-sdk"
"--prebuilt-dart-sdk",
"--impeller-enable-3d"

@bdero bdero Aug 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this unopt build get collected into artifacts that are shipped to Flutter users in any form? The --impeller-enable-3d flag also switches on the "experimental" version of dart:ui, which allows for appending scene nodes that we should continue not exposing to Flutter users.

@jonahwilliams jonahwilliams Aug 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we don't ship unopt artifacts

@zanderso zanderso Aug 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The command to run tests for host_debug_unopt is defined down on line 167. Are there any additional tests to add there?

@dnfield dnfield Aug 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as long as impeller_unittests runs it's fine - which it should as part of run_tests.py on this platform

],
"name": "host_debug_unopt",
"ninja": {
Expand Down
2 changes: 2 additions & 0 deletions impeller/aiks/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impeller_component("aiks_unittests") {
"//flutter/impeller/geometry:geometry_asserts",
"//flutter/impeller/golden_tests:golden_playground_test",
"//flutter/impeller/playground:playground_test",
"//flutter/impeller/scene",
"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
"//flutter/testing:testing_lib",
]
Expand All @@ -92,6 +93,7 @@ impeller_component("aiks_unittests_golden") {
"//flutter/impeller/geometry:geometry_asserts",
"//flutter/impeller/golden_tests:golden_playground_test",
"//flutter/impeller/playground:playground_test",
"//flutter/impeller/scene",
"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
"//flutter/testing:testing_lib",
]
Expand Down
2 changes: 2 additions & 0 deletions impeller/aiks/aiks_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2056,6 +2056,7 @@ TEST_P(AiksTest, SaveLayerFiltersScaleWithTransform) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

#if IMPELLER_ENABLE_3D

@bdero bdero Aug 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should activate this flag in a CI build as part of this patch, otherwise Scene will just bitrot.

@dnfield dnfield Aug 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've updated the mac unpt build (which AFAIU runs the impeller unittests) to add --impeller-enable-3d.

TEST_P(AiksTest, SceneColorSource) {
// Load up the scene.
auto mapping =
Expand Down Expand Up @@ -2096,6 +2097,7 @@ TEST_P(AiksTest, SceneColorSource) {

ASSERT_TRUE(OpenPlaygroundHere(callback));
}
#endif // IMPELLER_ENABLE_3D

TEST_P(AiksTest, PaintWithFilters) {
// validate that a paint with a color filter "HasFilters", no other filters
Expand Down
9 changes: 7 additions & 2 deletions impeller/aiks/color_source.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@
#include "impeller/entity/contents/linear_gradient_contents.h"
#include "impeller/entity/contents/radial_gradient_contents.h"
#include "impeller/entity/contents/runtime_effect_contents.h"
#include "impeller/entity/contents/scene_contents.h"
#include "impeller/entity/contents/solid_color_contents.h"
#include "impeller/entity/contents/sweep_gradient_contents.h"
#include "impeller/entity/contents/tiled_texture_contents.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/matrix.h"
#include "impeller/geometry/scalar.h"
#include "impeller/runtime_stage/runtime_stage.h"
#include "impeller/scene/node.h"

#if IMPELLER_ENABLE_3D
#include "impeller/entity/contents/scene_contents.h" // nogncheck
#include "impeller/scene/node.h" // nogncheck
#endif // IMPELLER_ENABLE_3D

namespace impeller {

Expand Down Expand Up @@ -210,6 +213,7 @@ ColorSource ColorSource::MakeRuntimeEffect(
return result;
}

#if IMPELLER_ENABLE_3D
ColorSource ColorSource::MakeScene(std::shared_ptr<scene::Node> scene_node,
Matrix camera_transform) {
ColorSource result;
Expand All @@ -224,6 +228,7 @@ ColorSource ColorSource::MakeScene(std::shared_ptr<scene::Node> scene_node,
};
return result;
}
#endif // IMPELLER_ENABLE_3D

ColorSource::Type ColorSource::GetType() const {
return type_;
Expand Down
7 changes: 6 additions & 1 deletion impeller/aiks/color_source.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
#include "impeller/geometry/matrix.h"
#include "impeller/geometry/point.h"
#include "impeller/runtime_stage/runtime_stage.h"
#include "impeller/scene/node.h"

#if IMPELLER_ENABLE_3D
#include "impeller/scene/node.h" // nogncheck
#endif // IMPELLER_ENABLE_3D

namespace impeller {

Expand Down Expand Up @@ -85,8 +88,10 @@ class ColorSource {
std::shared_ptr<std::vector<uint8_t>> uniform_data,
std::vector<RuntimeEffectContents::TextureInput> texture_inputs);

#if IMPELLER_ENABLE_3D
static ColorSource MakeScene(std::shared_ptr<scene::Node> scene_node,
Matrix camera_transform);
#endif // IMPELLER_ENABLE_3D

Type GetType() const;

Expand Down
1 change: 1 addition & 0 deletions impeller/display_list/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impeller_component("display_list_unittests") {
deps = [
":display_list",
"../playground:playground_test",
"//flutter/impeller/scene",
"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
]

Expand Down
5 changes: 4 additions & 1 deletion impeller/display_list/dl_dispatcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "impeller/entity/contents/linear_gradient_contents.h"
#include "impeller/entity/contents/radial_gradient_contents.h"
#include "impeller/entity/contents/runtime_effect_contents.h"
#include "impeller/entity/contents/scene_contents.h"
#include "impeller/entity/contents/sweep_gradient_contents.h"
#include "impeller/entity/contents/tiled_texture_contents.h"
#include "impeller/entity/entity.h"
Expand All @@ -37,6 +36,10 @@
#include "impeller/geometry/sigma.h"
#include "impeller/typographer/backends/skia/text_frame_skia.h"

#if IMPELLER_ENABLE_3D
#include "impeller/entity/contents/scene_contents.h"
#endif // IMPELLER_ENABLE_3D

namespace impeller {

#define UNIMPLEMENTED \
Expand Down
11 changes: 8 additions & 3 deletions impeller/entity/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ impeller_component("entity") {
"contents/radial_gradient_contents.h",
"contents/runtime_effect_contents.cc",
"contents/runtime_effect_contents.h",
"contents/scene_contents.cc",
"contents/scene_contents.h",
"contents/solid_color_contents.cc",
"contents/solid_color_contents.h",
"contents/solid_rrect_blur_contents.cc",
Expand Down Expand Up @@ -261,10 +259,17 @@ impeller_component("entity") {
"../archivist",
"../image:image_skia_backend",
"../renderer",
"../scene",
"../typographer",
]

if (impeller_enable_3d) {
sources += [
"contents/scene_contents.cc",
"contents/scene_contents.h",
]
public_deps += [ "../scene" ]
}

deps = [ "//flutter/fml" ]
}

Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/contents/content_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ ContentContext::ContentContext(
lazy_glyph_atlas_(
std::make_shared<LazyGlyphAtlas>(std::move(typographer_context))),
tessellator_(std::make_shared<Tessellator>()),
#if IMPELLER_ENABLE_3D
scene_context_(std::make_shared<scene::SceneContext>(context_)),
#endif // IMPELLER_ENABLE_3D
render_target_cache_(std::make_shared<RenderTargetCache>(
context_->GetResourceAllocator())) {
if (!context_ || !context_->IsValid()) {
Expand Down Expand Up @@ -413,9 +415,11 @@ std::shared_ptr<Texture> ContentContext::MakeSubpass(
return subpass_texture;
}

#if IMPELLER_ENABLE_3D
std::shared_ptr<scene::SceneContext> ContentContext::GetSceneContext() const {
return scene_context_;
}
#endif // IMPELLER_ENABLE_3D

std::shared_ptr<Tessellator> ContentContext::GetTessellator() const {
return tessellator_;
Expand Down
9 changes: 8 additions & 1 deletion impeller/entity/contents/content_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "impeller/renderer/capabilities.h"
#include "impeller/renderer/pipeline.h"
#include "impeller/renderer/render_target.h"
#include "impeller/scene/scene_context.h"
#include "impeller/typographer/typographer_context.h"

#ifdef IMPELLER_DEBUG
Expand Down Expand Up @@ -113,6 +112,10 @@
#include "impeller/entity/framebuffer_blend_screen.frag.h"
#include "impeller/entity/framebuffer_blend_softlight.frag.h"

#if IMPELLER_ENABLE_3D
#include "impeller/scene/scene_context.h" // nogncheck
#endif

namespace impeller {

#ifdef IMPELLER_DEBUG
Expand Down Expand Up @@ -348,7 +351,9 @@ class ContentContext {

bool IsValid() const;

#if IMPELLER_ENABLE_3D
std::shared_ptr<scene::SceneContext> GetSceneContext() const;
#endif // IMPELLER_ENABLE_3D

std::shared_ptr<Tessellator> GetTessellator() const;

Expand Down Expand Up @@ -873,7 +878,9 @@ class ContentContext {

bool is_valid_ = false;
std::shared_ptr<Tessellator> tessellator_;
#if IMPELLER_ENABLE_3D
std::shared_ptr<scene::SceneContext> scene_context_;
#endif // IMPELLER_ENABLE_3D
std::shared_ptr<RenderTargetAllocator> render_target_cache_;
bool wireframe_ = false;

Expand Down
7 changes: 6 additions & 1 deletion impeller/renderer/backend/vulkan/render_pass_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ SharedHandleVK<vk::Framebuffer> RenderPassVK::CreateVKFramebuffer(
auto [result, framebuffer] =
context.GetDevice().createFramebufferUnique(fb_info);

FML_LOG(ERROR) << "create framebuffer result is " << vk::to_string(result)
<< " "; // << framebuffer.get();
if (result != vk::Result::eSuccess) {
VALIDATION_LOG << "Could not create framebuffer: " << vk::to_string(result);
return {};
Expand Down Expand Up @@ -636,12 +638,15 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const {

vk::RenderPassBeginInfo pass_info;
pass_info.renderPass = *render_pass;
pass_info.framebuffer = *framebuffer;
FML_LOG(ERROR) << target_size;
pass_info.renderArea.extent.width = static_cast<uint32_t>(target_size.width);
pass_info.renderArea.extent.height =
static_cast<uint32_t>(target_size.height);
pass_info.setClearValues(clear_values);

VkRenderPassBeginInfo my_info = pass_info;
FML_LOG(ERROR) << "my_info is " << my_info.sType << " " << my_info.renderPass
<< " " << my_info.framebuffer;
{
TRACE_EVENT0("impeller", "EncodeRenderPassCommands");
cmd_buffer.beginRenderPass(pass_info, vk::SubpassContents::eInline);
Expand Down
Loading