From 8c39197a819d956db965635716bd9d07002c6039 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Tue, 6 Feb 2024 22:04:41 +0000 Subject: [PATCH 1/4] Update embedder support for Impeller/OpenGL to load some missing shaders and configure a depth attachment --- shell/platform/embedder/embedder.cc | 27 ++++++++++++------- .../embedder/embedder_surface_gl_impeller.cc | 8 ++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index b165ca6c96274..d9fba12a2797e 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -981,25 +981,34 @@ MakeRenderTargetFromBackingStoreImpeller( color0.load_action = impeller::LoadAction::kClear; color0.store_action = impeller::StoreAction::kStore; - impeller::TextureDescriptor stencil0_tex; - stencil0_tex.type = impeller::TextureType::kTexture2D; - stencil0_tex.format = impeller::PixelFormat::kR8G8B8A8UNormInt; - stencil0_tex.size = size; - stencil0_tex.usage = static_cast( + impeller::TextureDescriptor depth_stencil_texture_desc; + depth_stencil_texture_desc.type = impeller::TextureType::kTexture2D; + depth_stencil_texture_desc.format = impeller::PixelFormat::kR8G8B8A8UNormInt; + depth_stencil_texture_desc.size = size; + depth_stencil_texture_desc.usage = static_cast( impeller::TextureUsage::kRenderTarget); - stencil0_tex.sample_count = impeller::SampleCount::kCount1; + depth_stencil_texture_desc.sample_count = impeller::SampleCount::kCount1; + + auto depth_stencil_tex = std::make_shared( + gl_context.GetReactor(), depth_stencil_texture_desc, + impeller::TextureGLES::IsWrapped::kWrapped); + + impeller::DepthAttachment depth0; + depth0.clear_depth = 0; + depth0.texture = depth_stencil_tex; + depth0.load_action = impeller::LoadAction::kClear; + depth0.store_action = impeller::StoreAction::kDontCare; impeller::StencilAttachment stencil0; stencil0.clear_stencil = 0; - stencil0.texture = std::make_shared( - gl_context.GetReactor(), stencil0_tex, - impeller::TextureGLES::IsWrapped::kWrapped); + stencil0.texture = depth_stencil_tex; stencil0.load_action = impeller::LoadAction::kClear; stencil0.store_action = impeller::StoreAction::kDontCare; impeller::RenderTarget render_target_desc; render_target_desc.SetColorAttachment(color0, 0u); + render_target_desc.SetDepthAttachment(depth0); render_target_desc.SetStencilAttachment(stencil0); return std::make_unique( diff --git a/shell/platform/embedder/embedder_surface_gl_impeller.cc b/shell/platform/embedder/embedder_surface_gl_impeller.cc index c061c091a912b..2cb3be879b6e5 100644 --- a/shell/platform/embedder/embedder_surface_gl_impeller.cc +++ b/shell/platform/embedder/embedder_surface_gl_impeller.cc @@ -7,6 +7,8 @@ #include #include "impeller/entity/gles/entity_shaders_gles.h" +#include "impeller/entity/gles/framebuffer_blend_shaders_gles.h" +#include "impeller/entity/gles/modern_shaders_gles.h" #include "impeller/renderer/backend/gles/context_gles.h" #include "impeller/renderer/backend/gles/proc_table_gles.h" @@ -68,6 +70,12 @@ EmbedderSurfaceGLImpeller::EmbedderSurfaceGLImpeller( std::make_shared( impeller_entity_shaders_gles_data, impeller_entity_shaders_gles_length), + std::make_shared( + impeller_modern_shaders_gles_data, + impeller_modern_shaders_gles_length), + std::make_shared( + impeller_framebuffer_blend_shaders_gles_data, + impeller_framebuffer_blend_shaders_gles_length), #if IMPELLER_ENABLE_3D std::make_shared( impeller_scene_shaders_gles_data, impeller_scene_shaders_gles_length), From 42842b0b188b441ff69379e2c4e954ed80a04442 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 7 Feb 2024 00:27:46 +0000 Subject: [PATCH 2/4] test that renders a scene with an Impeller OpenGL engine --- .../backend/gles/buffer_bindings_gles.cc | 16 +++++++++ .../backend/gles/capabilities_gles.cc | 6 ++++ .../renderer/backend/gles/capabilities_gles.h | 3 ++ .../renderer/backend/gles/description_gles.cc | 9 +++++ .../renderer/backend/gles/description_gles.h | 3 ++ .../renderer/backend/gles/render_pass_gles.cc | 16 ++++----- shell/platform/embedder/BUILD.gn | 1 + .../platform/embedder/fixtures/solid_red.png | Bin 0 -> 3252 bytes .../embedder/tests/embedder_gl_unittests.cc | 32 ++++++++++++++++++ 9 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 shell/platform/embedder/fixtures/solid_red.png diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index 4be33ce6fcbf3..9d719b3b2cf48 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -15,6 +15,11 @@ namespace impeller { +// This prefix is used in the names of inputs generated by ANGLE's framebuffer +// fetch emulation. +static constexpr std::string_view kAngleInputAttachmentPrefix = + "ANGLEInputAttachment"; + BufferBindingsGLES::BufferBindingsGLES() = default; BufferBindingsGLES::~BufferBindingsGLES() = default; @@ -110,6 +115,17 @@ bool BufferBindingsGLES::ReadUniformsBindings(const ProcTableGLES& gl, &uniform_type, // type name.data() // name ); + + // Skip unrecognized variables generated by ANGLE. + if (gl.GetCapabilities()->IsANGLE()) { + if (written_count >= + static_cast(kAngleInputAttachmentPrefix.length()) && + std::string_view(name.data(), kAngleInputAttachmentPrefix.length()) == + kAngleInputAttachmentPrefix) { + continue; + } + } + auto location = gl.GetUniformLocation(program, name.data()); if (location == -1) { VALIDATION_LOG << "Could not query the location of an active uniform."; diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index e832aca15f276..e379497edfd43 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -117,6 +117,8 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); supports_offscreen_msaa_ = value >= 4; } + + is_angle_ = desc->IsANGLE(); } size_t CapabilitiesGLES::GetMaxTextureUnits(ShaderStage stage) const { @@ -188,4 +190,8 @@ PixelFormat CapabilitiesGLES::GetDefaultDepthStencilFormat() const { return PixelFormat::kD24UnormS8Uint; } +bool CapabilitiesGLES::IsANGLE() const { + return is_angle_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/gles/capabilities_gles.h b/impeller/renderer/backend/gles/capabilities_gles.h index baa6d76d378c2..dc58081f05e44 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.h +++ b/impeller/renderer/backend/gles/capabilities_gles.h @@ -74,6 +74,8 @@ class CapabilitiesGLES final size_t GetMaxTextureUnits(ShaderStage stage) const; + bool IsANGLE() const; + // |Capabilities| bool SupportsOffscreenMSAA() const override; @@ -121,6 +123,7 @@ class CapabilitiesGLES final bool supports_decal_sampler_address_mode_ = false; bool supports_offscreen_msaa_ = false; bool supports_implicit_msaa_ = false; + bool is_angle_ = false; }; } // namespace impeller diff --git a/impeller/renderer/backend/gles/description_gles.cc b/impeller/renderer/backend/gles/description_gles.cc index 40d583bca535d..855c192079a6b 100644 --- a/impeller/renderer/backend/gles/description_gles.cc +++ b/impeller/renderer/backend/gles/description_gles.cc @@ -40,6 +40,10 @@ static bool DetermineIfES(const std::string& version) { return HasPrefix(version, "OpenGL ES"); } +static bool DetermineIfANGLE(const std::string& version) { + return version.find("ANGLE") != std::string::npos; +} + static std::optional DetermineVersion(std::string version) { // Format for OpenGL "OpenGLES". @@ -81,6 +85,7 @@ DescriptionGLES::DescriptionGLES(const ProcTableGLES& gl) gl_version_string_(GetGLString(gl, GL_VERSION)), sl_version_string_(GetGLString(gl, GL_SHADING_LANGUAGE_VERSION)) { is_es_ = DetermineIfES(gl_version_string_); + is_angle_ = DetermineIfANGLE(gl_version_string_); auto gl_version = DetermineVersion(gl_version_string_); if (!gl_version.has_value()) { @@ -164,6 +169,10 @@ bool DescriptionGLES::IsES() const { return is_es_; } +bool DescriptionGLES::IsANGLE() const { + return is_angle_; +} + bool DescriptionGLES::HasExtension(const std::string& ext) const { return extensions_.find(ext) != extensions_.end(); } diff --git a/impeller/renderer/backend/gles/description_gles.h b/impeller/renderer/backend/gles/description_gles.h index 818ea0d0cc358..496e1eec02b79 100644 --- a/impeller/renderer/backend/gles/description_gles.h +++ b/impeller/renderer/backend/gles/description_gles.h @@ -34,6 +34,8 @@ class DescriptionGLES { /// @brief Returns whether GLES includes the debug extension. bool HasDebugExtension() const; + bool IsANGLE() const; + private: Version gl_version_; Version sl_version_; @@ -43,6 +45,7 @@ class DescriptionGLES { std::string gl_version_string_; std::string sl_version_string_; std::set extensions_; + bool is_angle_ = false; bool is_valid_ = false; DescriptionGLES(const DescriptionGLES&) = delete; diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 0bb937320c384..b3d61f9b255a8 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -473,23 +473,21 @@ struct RenderPassData { if (gl.DiscardFramebufferEXT.IsAvailable()) { std::vector attachments; + // TODO(jonahwilliams): discarding stencil or depth on the default fbo + // causes Angle to discard the entire render target. Until we know the + // reason, default to storing. + bool angle_safe = gl.GetCapabilities()->IsANGLE() ? !is_default_fbo : true; + if (pass_data.discard_color_attachment) { attachments.push_back(is_default_fbo ? GL_COLOR_EXT : GL_COLOR_ATTACHMENT0); } - if (pass_data.discard_depth_attachment) { + if (pass_data.discard_depth_attachment && angle_safe) { attachments.push_back(is_default_fbo ? GL_DEPTH_EXT : GL_DEPTH_ATTACHMENT); } -// TODO(jonahwilliams): discarding the stencil on the default fbo when running -// on Windows causes Angle to discard the entire render target. Until we know -// the reason, default to storing. -#ifdef FML_OS_WIN - if (pass_data.discard_stencil_attachment && !is_default_fbo) { -#else - if (pass_data.discard_stencil_attachment) { -#endif + if (pass_data.discard_stencil_attachment && angle_safe) { attachments.push_back(is_default_fbo ? GL_STENCIL_EXT : GL_STENCIL_ATTACHMENT); } diff --git a/shell/platform/embedder/BUILD.gn b/shell/platform/embedder/BUILD.gn index aa97972de12e1..e0297c799acd5 100644 --- a/shell/platform/embedder/BUILD.gn +++ b/shell/platform/embedder/BUILD.gn @@ -250,6 +250,7 @@ test_fixtures("fixtures") { "fixtures/scene_without_custom_compositor.png", "fixtures/scene_without_custom_compositor_with_xform.png", "fixtures/snapshot_large_scene.png", + "fixtures/solid_red.png", "fixtures/verifyb143464703.png", "fixtures/verifyb143464703_soft_noxform.png", ] diff --git a/shell/platform/embedder/fixtures/solid_red.png b/shell/platform/embedder/fixtures/solid_red.png new file mode 100644 index 0000000000000000000000000000000000000000..89f889b95dcb75ae7a60517e086d71ae98dec8b5 GIT binary patch literal 3252 zcmeAS@N?(olHy`uVBq!ia0y~yU{+vYV2a>i1B%QlYbpRzEX7WqAsj$Z!;#X#z`&F3 z>EaktG3V`dLq-M#o&y^i>(4(}B%{Lht8DH}W1#xRzg9r``U1A`G83j@Om2>}KM zg=7Z?hK8O|!O;L2O$?(MVYFlzEfq&=gwfh?v^E^A4M%Ik(b^DuZ5Wd;&bDrrenu>? PL&4zb>gTe~DWM4fq#N9) literal 0 HcmV?d00001 diff --git a/shell/platform/embedder/tests/embedder_gl_unittests.cc b/shell/platform/embedder/tests/embedder_gl_unittests.cc index 90c48def74699..a8b411bd7bcc3 100644 --- a/shell/platform/embedder/tests/embedder_gl_unittests.cc +++ b/shell/platform/embedder/tests/embedder_gl_unittests.cc @@ -26,6 +26,7 @@ #include "flutter/fml/thread.h" #include "flutter/lib/ui/painting/image.h" #include "flutter/runtime/dart_vm.h" +#include "flutter/shell/platform/embedder/embedder_surface_gl_impeller.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "flutter/shell/platform/embedder/tests/embedder_config_builder.h" #include "flutter/shell/platform/embedder/tests/embedder_test.h" @@ -4699,6 +4700,37 @@ TEST_F(EmbedderTest, latch.Wait(); } +TEST_F(EmbedderTest, CanRenderWithImpellerOpenGL) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + EmbedderConfigBuilder builder(context); + + builder.AddCommandLineArgument("--enable-impeller"); + builder.SetDartEntrypoint("draw_solid_red"); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); + builder.SetCompositor(); + builder.SetRenderTargetType( + EmbedderTestBackingStoreProducer::RenderTargetType::kOpenGLFramebuffer); + + auto rendered_scene = context.GetNextSceneImage(); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // Send a window metrics events so frames may be scheduled. + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 800; + event.height = 600; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + + ASSERT_TRUE(ImageMatchesFixture( + FixtureNameForBackend(EmbedderTestContextType::kOpenGLContext, + "solid_red.png"), + rendered_scene)); +} + INSTANTIATE_TEST_SUITE_P( EmbedderTestGlVk, EmbedderTestMultiBackend, From b7a5992a8869e07edf0dbea9dc740d10d9dc3785 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Sat, 10 Feb 2024 01:15:38 +0000 Subject: [PATCH 3/4] call the framebuffer destruction callback in EmbedderRenderTargetImpeller --- shell/platform/embedder/embedder.cc | 6 +++++- .../embedder/embedder_render_target_impeller.cc | 13 ++++++++++--- .../embedder/embedder_render_target_impeller.h | 4 +++- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index d9fba12a2797e..7773bd73707db 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1011,10 +1011,14 @@ MakeRenderTargetFromBackingStoreImpeller( render_target_desc.SetDepthAttachment(depth0); render_target_desc.SetStencilAttachment(stencil0); + fml::closure framebuffer_destruct = + [callback = framebuffer->destruction_callback, + user_data = framebuffer->user_data]() { callback(user_data); }; + return std::make_unique( backing_store, aiks_context, std::make_unique(std::move(render_target_desc)), - on_release); + on_release, framebuffer_destruct); #else return nullptr; #endif diff --git a/shell/platform/embedder/embedder_render_target_impeller.cc b/shell/platform/embedder/embedder_render_target_impeller.cc index 44f23b62654d2..52d520fdf0b78 100644 --- a/shell/platform/embedder/embedder_render_target_impeller.cc +++ b/shell/platform/embedder/embedder_render_target_impeller.cc @@ -13,15 +13,22 @@ EmbedderRenderTargetImpeller::EmbedderRenderTargetImpeller( FlutterBackingStore backing_store, std::shared_ptr aiks_context, std::unique_ptr impeller_target, - fml::closure on_release) + fml::closure on_release, + fml::closure framebuffer_destruction_callback) : EmbedderRenderTarget(backing_store, std::move(on_release)), aiks_context_(std::move(aiks_context)), - impeller_target_(std::move(impeller_target)) { + impeller_target_(std::move(impeller_target)), + framebuffer_destruction_callback_( + std::move(framebuffer_destruction_callback)) { FML_DCHECK(aiks_context_); FML_DCHECK(impeller_target_); } -EmbedderRenderTargetImpeller::~EmbedderRenderTargetImpeller() = default; +EmbedderRenderTargetImpeller::~EmbedderRenderTargetImpeller() { + if (framebuffer_destruction_callback_) { + framebuffer_destruction_callback_(); + } +} sk_sp EmbedderRenderTargetImpeller::GetSkiaSurface() const { return nullptr; diff --git a/shell/platform/embedder/embedder_render_target_impeller.h b/shell/platform/embedder/embedder_render_target_impeller.h index 2425402008f8a..4e7a2b7d88e12 100644 --- a/shell/platform/embedder/embedder_render_target_impeller.h +++ b/shell/platform/embedder/embedder_render_target_impeller.h @@ -15,7 +15,8 @@ class EmbedderRenderTargetImpeller final : public EmbedderRenderTarget { FlutterBackingStore backing_store, std::shared_ptr aiks_context, std::unique_ptr impeller_target, - fml::closure on_release); + fml::closure on_release, + fml::closure framebuffer_destruction_callback); // |EmbedderRenderTarget| ~EmbedderRenderTargetImpeller() override; @@ -35,6 +36,7 @@ class EmbedderRenderTargetImpeller final : public EmbedderRenderTarget { private: std::shared_ptr aiks_context_; std::unique_ptr impeller_target_; + fml::closure framebuffer_destruction_callback_; FML_DISALLOW_COPY_AND_ASSIGN(EmbedderRenderTargetImpeller); }; From 1164ae4dc7752fa7088cc52711a5e07ec91e45be Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Tue, 13 Feb 2024 01:01:33 +0000 Subject: [PATCH 4/4] fixes for mac and windows hosts --- shell/platform/embedder/embedder.cc | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 7773bd73707db..4633fcad552d2 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -310,22 +310,27 @@ InferOpenGLPlatformViewCreationCallback( // damage are always 1. Once the function that computes damage implements // support for multiple damage rectangles, GLPresentInfo should also // contain the number of damage rectangles. - const size_t num_rects = 1; - std::array frame_damage_rect = { - SkIRectToFlutterRect(*(gl_present_info.frame_damage))}; - std::array buffer_damage_rect = { - SkIRectToFlutterRect(*(gl_present_info.buffer_damage))}; + std::optional frame_damage_rect; + if (gl_present_info.frame_damage) { + frame_damage_rect = + SkIRectToFlutterRect(*(gl_present_info.frame_damage)); + } + std::optional buffer_damage_rect; + if (gl_present_info.buffer_damage) { + buffer_damage_rect = + SkIRectToFlutterRect(*(gl_present_info.buffer_damage)); + } FlutterDamage frame_damage{ .struct_size = sizeof(FlutterDamage), - .num_rects = frame_damage_rect.size(), - .damage = frame_damage_rect.data(), + .num_rects = frame_damage_rect ? size_t{1} : size_t{0}, + .damage = frame_damage_rect ? &frame_damage_rect.value() : nullptr, }; FlutterDamage buffer_damage{ .struct_size = sizeof(FlutterDamage), - .num_rects = buffer_damage_rect.size(), - .damage = buffer_damage_rect.data(), + .num_rects = buffer_damage_rect ? size_t{1} : size_t{0}, + .damage = buffer_damage_rect ? &buffer_damage_rect.value() : nullptr, }; // Construct the present information concerning the frame being rendered. @@ -1088,7 +1093,7 @@ MakeRenderTargetFromBackingStoreImpeller( return std::make_unique( backing_store, aiks_context, std::make_unique(std::move(render_target_desc)), - on_release); + on_release, fml::closure()); #else return nullptr; #endif