Skip to content

Commit

Permalink
[impeller] enable framebuffer blit when available (#56596)
Browse files Browse the repository at this point in the history
depends on #56573
fixes flutter/flutter#158523

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
  • Loading branch information
gaaclarke authored Nov 18, 2024
1 parent 30acf31 commit 554786c
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 26 deletions.
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) {
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;
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),
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

0 comments on commit 554786c

Please sign in to comment.