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

[impeller] enable framebuffer blit when available #56596

Merged
merged 10 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions impeller/golden_tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ if (is_mac) {
"//flutter/impeller/display_list:aiks_unittests_golden",
"//flutter/impeller/display_list:display_list_unittests_golden",
"//flutter/impeller/fixtures",
"//flutter/impeller/renderer:renderer_unittests_golden",
"//flutter/third_party/angle:libEGL",
"//flutter/third_party/angle:libGLESv2",
"//flutter/third_party/googletest:gtest",
Expand Down
52 changes: 38 additions & 14 deletions impeller/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -96,29 +96,53 @@ impeller_component("renderer") {
deps = [ "//flutter/fml" ]
}

impeller_component("renderer_unittests") {
testonly = true

sources = [
template("renderer_unittests_component") {
target_name = invoker.target_name
predefined_sources = [
"blit_pass_unittests.cc",
"capabilities_unittests.cc",
"device_buffer_unittests.cc",
"pipeline_descriptor_unittests.cc",
"pool_unittests.cc",
"renderer_unittests.cc",
]
impeller_component(target_name) {
testonly = true
if (defined(invoker.defines)) {
defines = invoker.defines
} else {
defines = []
}
sources = predefined_sources
if (defined(invoker.deps)) {
deps = invoker.deps
} else {
deps = []
}
deps += [
":renderer",
"../fixtures",
"../playground:playground_test",
"../tessellator:tessellator_libtess",
"//flutter/impeller/display_list:display_list",
"//flutter/testing:testing_lib",
]
if (defined(invoker.public_configs)) {
public_configs = invoker.public_configs
}
}
}

deps = [
":renderer",
"../fixtures",
"../playground:playground_test",
"../tessellator:tessellator_libtess",
"//flutter/testing:testing_lib",
]
renderer_unittests_component("renderer_unittests") {
deps = [ "//flutter/impeller/display_list:aiks_unittests" ]
}

if (impeller_enable_compute) {
sources += [ "compute_unittests.cc" ]
}
renderer_unittests_component("renderer_unittests_golden") {
deps = [ "//flutter/impeller/display_list:aiks_unittests_golden" ]
defines = [
"IMPELLER_GOLDEN_TESTS",
"IMPELLER_ENABLE_VALIDATION=1",
]
}

impeller_component("renderer_dart_unittests") {
Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/gles/blit_command_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ bool BlitResizeTextureCommandGLES::Encode(const ReactorGLES& reactor) const {
return false;
}

destination->SetCoordinateSystem(source->GetCoordinateSystem());

GLuint read_fbo = GL_NONE;
GLuint draw_fbo = GL_NONE;
fml::ScopedCleanupClosure delete_fbos([&gl, &read_fbo, &draw_fbo]() {
Expand Down
9 changes: 5 additions & 4 deletions impeller/renderer/backend/gles/capabilities_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) {
default_glyph_atlas_format_ = PixelFormat::kR8UNormInt;
}

if (desc->GetGlVersion().major_version >= 3) {
Copy link
Member

Choose a reason for hiding this comment

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

Completely off topic, but we should do a GL version check in ProcTableGLES and reset or not bother to resolve FOR_EACH_IMPELLER_GLES3_PROC methods when we less than ES 3. On older versions of Android, I remember seeing instances where we'd get a function pointer for ES3 functions even on ES2 contexts but those wouldn't work if called.

When I first wrote that bit, I didn't think we'd ever resolve ES3 procs. So its a blind spot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, absolutely agree. I can file an issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

supports_texture_to_texture_blits_ = true;
}

supports_framebuffer_fetch_ = desc->HasExtension(kFramebufferFetchExt);

if (desc->HasExtension(kTextureBorderClampExt) ||
Expand Down Expand Up @@ -158,10 +162,7 @@ bool CapabilitiesGLES::SupportsSSBO() const {
}

bool CapabilitiesGLES::SupportsTextureToTextureBlits() const {
// TODO(158523): Switch this to true for improved performance
// on GLES 3.0+ devices. Note that this wasn't enabled because
// there were some rendering issues on some devices.
return false;
return supports_texture_to_texture_blits_;
}

bool CapabilitiesGLES::SupportsFramebufferFetch() const {
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/gles/capabilities_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class CapabilitiesGLES final
ISize GetMaximumRenderPassAttachmentSize() const override;

private:
bool supports_texture_to_texture_blits_ = false;
bool supports_framebuffer_fetch_ = false;
bool supports_decal_sampler_address_mode_ = false;
bool supports_offscreen_msaa_ = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ TEST(CapabilitiesGLES, CanInitializeWithDefaults) {

EXPECT_FALSE(capabilities->SupportsOffscreenMSAA());
EXPECT_FALSE(capabilities->SupportsSSBO());
EXPECT_FALSE(capabilities->SupportsTextureToTextureBlits());
EXPECT_TRUE(capabilities->SupportsTextureToTextureBlits());
EXPECT_FALSE(capabilities->SupportsFramebufferFetch());
EXPECT_FALSE(capabilities->SupportsCompute());
EXPECT_FALSE(capabilities->SupportsComputeSubgroups());
Expand Down
41 changes: 40 additions & 1 deletion impeller/renderer/blit_pass_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
// found in the LICENSE file.

#include <cstdint>
#include "flutter/display_list/dl_builder.h"
#include "flutter/impeller/display_list/aiks_unittests.h"
#include "flutter/impeller/display_list/dl_image_impeller.h"
#include "fml/mapping.h"
#include "gtest/gtest.h"
#include "impeller/base/validation.h"
Expand All @@ -16,7 +19,12 @@
namespace impeller {
namespace testing {

using BlitPassTest = PlaygroundTest;
using flutter::DisplayListBuilder;
using flutter::DlColor;
using flutter::DlImageSampling;
using flutter::DlPaint;

using BlitPassTest = AiksTest;
INSTANTIATE_PLAYGROUND_SUITE(BlitPassTest);

TEST_P(BlitPassTest, BlitAcrossDifferentPixelFormatsFails) {
Expand Down Expand Up @@ -230,5 +238,36 @@ TEST_P(BlitPassTest, CanResizeTextures) {
EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok());
}

TEST_P(BlitPassTest, CanResizeTexturesPlayground) {
auto context = GetContext();
auto cmd_buffer = context->CreateCommandBuffer();
auto blit_pass = cmd_buffer->CreateBlitPass();

std::shared_ptr<Texture> src = CreateTextureForFixture("kalimba.jpg");

TextureDescriptor dst_format;
dst_format.storage_mode = StorageMode::kDevicePrivate;
dst_format.format = PixelFormat::kR8G8B8A8UNormInt;
dst_format.size = {src->GetSize().width / 2, src->GetSize().height};
dst_format.usage = TextureUsage::kShaderRead | TextureUsage::kShaderWrite;
auto dst = context->GetResourceAllocator()->CreateTexture(dst_format);

ASSERT_TRUE(dst);
ASSERT_TRUE(src);

EXPECT_TRUE(blit_pass->ResizeTexture(src, dst));
EXPECT_TRUE(blit_pass->EncodeCommands(GetContext()->GetResourceAllocator()));
EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok());

DisplayListBuilder builder;
builder.Scale(GetContentScale().x, GetContentScale().y);
DlPaint paint;
paint.setColor(DlColor::kRed());
auto image = DlImageImpeller::Make(dst);
builder.DrawImage(image, SkPoint::Make(100.0, 100.0),
DlImageSampling::kNearestNeighbor, &paint);
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}

} // namespace testing
} // namespace impeller
2 changes: 1 addition & 1 deletion lib/ui/painting/image_decoder_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ DecompressResult ImageDecoderImpeller::DecompressTexture(
!capabilities->SupportsTextureToTextureBlits()) {
//----------------------------------------------------------------------------
/// 2. If the decoded image isn't the requested target size and the src size
/// exceeds the device max texture size, perform a slow CPU reisze.
/// exceeds the device max texture size, perform a slow CPU resize.
///
TRACE_EVENT0("impeller", "SlowCPUDecodeScale");
const auto scaled_image_info = image_info.makeDimensions(target_size);
Expand Down
8 changes: 7 additions & 1 deletion shell/platform/android/android_context_gl_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ AndroidContextGLImpeller::AndroidContextGLImpeller(
}

impeller::egl::ConfigDescriptor desc;
desc.api = impeller::egl::API::kOpenGLES2;
desc.api = impeller::egl::API::kOpenGLES3;
Copy link
Member

Choose a reason for hiding this comment

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

Is this valid to do? Defiintely outside of my wheelhouse. - we do a lot of GLES 2 stuff, do we need to worry about GLES 3 having different semantics?

@chinmaygarde

Copy link
Member

Choose a reason for hiding this comment

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

Nope. That's how you'd do it. ES3 is backwards compatible with the same sematics.

Copy link
Member

Choose a reason for hiding this comment

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

Coolio

desc.color_format = impeller::egl::ColorFormat::kRGBA8888;
desc.depth_bits = impeller::egl::DepthBits::kTwentyFour;
desc.stencil_bits = impeller::egl::StencilBits::kEight;
Expand All @@ -101,6 +101,12 @@ AndroidContextGLImpeller::AndroidContextGLImpeller(
desc.surface_type = impeller::egl::SurfaceType::kWindow;
std::unique_ptr<impeller::egl::Config> onscreen_config =
display_->ChooseConfig(desc);

if (!onscreen_config) {
desc.api = impeller::egl::API::kOpenGLES2;
onscreen_config = display_->ChooseConfig(desc);
}

if (!onscreen_config) {
// Fallback for Android emulator.
desc.samples = impeller::egl::Samples::kOne;
Expand Down
18 changes: 14 additions & 4 deletions shell/platform/android/android_context_gl_impeller_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,28 +97,38 @@ TEST_F(AndroidContextGLImpellerTest, FallbackForEmulator) {
auto display = std::make_unique<MockDisplay>();
EXPECT_CALL(*display, IsValid).WillRepeatedly(Return(true));
std::unique_ptr<Config> first_result;
auto second_result =
std::make_unique<Config>(ConfigDescriptor(), window_egl_config);
std::unique_ptr<Config> second_result;
auto third_result =
std::make_unique<Config>(ConfigDescriptor(), window_egl_config);
auto fourth_result =
std::make_unique<Config>(ConfigDescriptor(), pbuffer_egl_config);
EXPECT_CALL(
*display,
ChooseConfig(Matcher<ConfigDescriptor>(AllOf(
Field(&ConfigDescriptor::api, impeller::egl::API::kOpenGLES3),
Copy link
Member Author

Choose a reason for hiding this comment

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

@jonahwilliams looks like we could have used these tests for asserting we are asking for a depth buffer. It had been a while since I looked at these.

Copy link
Member

Choose a reason for hiding this comment

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

I thought I did that by adding EGL_DEPTH_SIZE 24 above?

Copy link
Member

Choose a reason for hiding this comment

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

🤷‍♂️

Field(&ConfigDescriptor::samples, impeller::egl::Samples::kFour),
Field(&ConfigDescriptor::surface_type,
impeller::egl::SurfaceType::kWindow)))))
.WillOnce(Return(ByMove(std::move(first_result))));
EXPECT_CALL(
*display,
ChooseConfig(Matcher<ConfigDescriptor>(AllOf(
Field(&ConfigDescriptor::api, impeller::egl::API::kOpenGLES2),
Field(&ConfigDescriptor::samples, impeller::egl::Samples::kFour),
Field(&ConfigDescriptor::surface_type,
impeller::egl::SurfaceType::kWindow)))))
.WillOnce(Return(ByMove(std::move(second_result))));
EXPECT_CALL(
*display,
ChooseConfig(Matcher<ConfigDescriptor>(
AllOf(Field(&ConfigDescriptor::samples, impeller::egl::Samples::kOne),
Field(&ConfigDescriptor::surface_type,
impeller::egl::SurfaceType::kWindow)))))
.WillOnce(Return(ByMove(std::move(second_result))));
.WillOnce(Return(ByMove(std::move(third_result))));
EXPECT_CALL(*display, ChooseConfig(Matcher<ConfigDescriptor>(
Field(&ConfigDescriptor::surface_type,
impeller::egl::SurfaceType::kPBuffer))))
.WillOnce(Return(ByMove(std::move(third_result))));
.WillOnce(Return(ByMove(std::move(fourth_result))));
ON_CALL(*display, ChooseConfig(_))
.WillByDefault(Return(ByMove(std::unique_ptr<Config>())));
auto context =
Expand Down
3 changes: 3 additions & 0 deletions testing/impeller_golden_tests_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,9 @@ impeller_Play_AiksTest_VerticesGeometryUVPositionDataWithTranslate_Vulkan.png
impeller_Play_AiksTest_VerticesGeometryUVPositionData_Metal.png
impeller_Play_AiksTest_VerticesGeometryUVPositionData_OpenGLES.png
impeller_Play_AiksTest_VerticesGeometryUVPositionData_Vulkan.png
impeller_Play_BlitPassTest_CanResizeTexturesPlayground_Metal.png
impeller_Play_BlitPassTest_CanResizeTexturesPlayground_OpenGLES.png
impeller_Play_BlitPassTest_CanResizeTexturesPlayground_Vulkan.png
impeller_Play_DlGoldenTest_Bug147807_Metal.png
impeller_Play_DlGoldenTest_Bug147807_OpenGLES.png
impeller_Play_DlGoldenTest_Bug147807_Vulkan.png
Expand Down