From 8838d2e87899201b875eca61ebfb566808ee787c Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 24 Oct 2023 14:12:11 -0700 Subject: [PATCH 1/3] Switch from glBlitFramebuffer to implicit MSAA resolution. --- .../backend/gles/capabilities_gles.cc | 33 ++++++------ .../renderer/backend/gles/capabilities_gles.h | 4 ++ .../renderer/backend/gles/render_pass_gles.cc | 50 ++++--------------- .../backend/vulkan/capabilities_vk.cc | 5 ++ .../renderer/backend/vulkan/capabilities_vk.h | 3 ++ impeller/renderer/capabilities.cc | 3 ++ impeller/renderer/capabilities.h | 5 ++ impeller/renderer/render_target.cc | 16 ++++++ 8 files changed, 62 insertions(+), 57 deletions(-) diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index 54ac1e70d3e6f..881829839bb86 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -32,7 +32,9 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { max_cube_map_texture_size = value; } - if (gl.GetDescription()->IsES()) { + auto const desc = gl.GetDescription(); + + if (desc->IsES()) { GLint value = 0; gl.GetIntegerv(GL_MAX_FRAGMENT_UNIFORM_VECTORS, &value); max_fragment_uniform_vectors = value; @@ -56,7 +58,7 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { max_texture_size = ISize{value, value}; } - if (gl.GetDescription()->IsES()) { + if (desc->IsES()) { GLint value = 0; gl.GetIntegerv(GL_MAX_VARYING_VECTORS, &value); max_varying_vectors = value; @@ -74,7 +76,7 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { max_vertex_texture_image_units = value; } - if (gl.GetDescription()->IsES()) { + if (desc->IsES()) { GLint value = 0; gl.GetIntegerv(GL_MAX_VERTEX_UNIFORM_VECTORS, &value); max_vertex_uniform_vectors = value; @@ -92,31 +94,26 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { num_compressed_texture_formats = value; } - if (gl.GetDescription()->IsES()) { + if (desc->IsES()) { GLint value = 0; gl.GetIntegerv(GL_NUM_SHADER_BINARY_FORMATS, &value); num_shader_binary_formats = value; } - supports_framebuffer_fetch_ = - gl.GetDescription()->HasExtension(kFramebufferFetchExt); + supports_framebuffer_fetch_ = desc->HasExtension(kFramebufferFetchExt); - if (gl.GetDescription()->HasExtension(kTextureBorderClampExt) || - gl.GetDescription()->HasExtension(kNvidiaTextureBorderClampExt) || - gl.GetDescription()->HasExtension(kOESTextureBorderClampExt)) { + if (desc->HasExtension(kTextureBorderClampExt) || + desc->HasExtension(kNvidiaTextureBorderClampExt) || + desc->HasExtension(kOESTextureBorderClampExt)) { supports_decal_sampler_address_mode_ = true; } - if (gl.GetDescription()->HasExtension( - "GL_EXT_multisampled_render_to_texture") && - // The current implementation of MSAA support in Impeller GLES requires - // the use of glBlitFramebuffer, which is not available on all GLES - // implementations. We can't use MSAA on these platforms yet. - gl.BlitFramebuffer.IsAvailable()) { + if (desc->HasExtension("GL_EXT_multisampled_render_to_texture")) { + supports_implicit_msaa_ = true; + // We hard-code 4x MSAA, so let's make sure it's supported. GLint value = 0; gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); - supports_offscreen_msaa_ = value >= 4; } } @@ -140,6 +137,10 @@ bool CapabilitiesGLES::SupportsOffscreenMSAA() const { return supports_offscreen_msaa_; } +bool CapabilitiesGLES::SupportsImplicitResolvingMSAA() const { + return supports_implicit_msaa_; +} + bool CapabilitiesGLES::SupportsSSBO() const { return false; } diff --git a/impeller/renderer/backend/gles/capabilities_gles.h b/impeller/renderer/backend/gles/capabilities_gles.h index edb9a1b33bb41..47dcaa33f3fb0 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.h +++ b/impeller/renderer/backend/gles/capabilities_gles.h @@ -76,6 +76,9 @@ class CapabilitiesGLES final // |Capabilities| bool SupportsOffscreenMSAA() const override; + // |Capabilities| + bool SupportsImplicitResolvingMSAA() const override; + // |Capabilities| bool SupportsSSBO() const override; @@ -119,6 +122,7 @@ class CapabilitiesGLES final bool supports_framebuffer_fetch_ = false; bool supports_decal_sampler_address_mode_ = false; bool supports_offscreen_msaa_ = false; + bool supports_implicit_msaa_ = false; }; } // namespace impeller diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 5536a3cd6c40a..be8d975f6bc51 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -4,6 +4,7 @@ #include "impeller/renderer/backend/gles/render_pass_gles.h" +#include "GLES3/gl3.h" #include "flutter/fml/trace_event.h" #include "fml/closure.h" #include "fml/logging.h" @@ -127,7 +128,6 @@ struct RenderPassData { Scalar clear_depth = 1.0; std::shared_ptr color_attachment; - std::shared_ptr resolve_attachment; std::shared_ptr depth_attachment; std::shared_ptr stencil_attachment; @@ -474,45 +474,6 @@ struct RenderPassData { } } - // When we have a resolve_attachment, MSAA is being used. We blit from the - // MSAA FBO to the resolve FBO, otherwise the resolve FBO ends up being - // incomplete (because it has no attachments). - // - // Note that this only works on OpenGLES 3.0+, or put another way, in older - // versions of OpenGLES, MSAA is not currently supported by Impeller. It's - // possible to work around this issue a few different ways (not yet done). - // - // TODO(matanlurey): See https://github.com/flutter/flutter/issues/137093. - if (!is_default_fbo && pass_data.resolve_attachment) { - // MSAA should not be enabled if BlitFramebuffer is not available. - FML_DCHECK(gl.BlitFramebuffer.IsAvailable()); - - GLuint draw_fbo = GL_NONE; - gl.GenFramebuffers(1u, &draw_fbo); - gl.BindFramebuffer(GL_FRAMEBUFFER, draw_fbo); - - auto resolve = TextureGLES::Cast(pass_data.resolve_attachment.get()); - if (!resolve->SetAsFramebufferAttachment( - GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kColor0)) { - return false; - } - - gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, draw_fbo); - gl.BindFramebuffer(GL_READ_FRAMEBUFFER, fbo); - auto size = pass_data.resolve_attachment->GetSize(); - gl.BlitFramebuffer(0, // srcX0 - 0, // srcY0 - size.width, // srcX1 - size.height, // srcY1 - 0, // dstX0 - 0, // dstY0 - size.width, // dstX1 - size.height, // dstY1 - GL_COLOR_BUFFER_BIT, // mask - GL_NEAREST // filter - ); - } - if (gl.DiscardFramebufferEXT.IsAvailable()) { std::vector attachments; @@ -569,12 +530,19 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { /// Setup color data. /// pass_data->color_attachment = color0.texture; - pass_data->resolve_attachment = color0.resolve_texture; pass_data->clear_color = color0.clear_color; pass_data->clear_color_attachment = CanClearAttachment(color0.load_action); pass_data->discard_color_attachment = CanDiscardAttachmentWhenDone(color0.store_action); + // When we are using EXT_multisampled_render_to_texture, it is implicitly + // resolved when we bind the texture to the framebuffer. We don't need to + // discard the attachment when we are done. + if (color0.resolve_texture) { + FML_DCHECK(context.GetCapabilities()->SupportsImplicitResolvingMSAA()); + pass_data->discard_color_attachment = false; + } + //---------------------------------------------------------------------------- /// Setup depth data. /// diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 52b39fc9bd860..ada7712ad66f7 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -409,6 +409,11 @@ bool CapabilitiesVK::SupportsOffscreenMSAA() const { return true; } +// |Capabilities| +bool CapabilitiesVK::SupportsImplicitResolvingMSAA() const { + return false; +} + // |Capabilities| bool CapabilitiesVK::SupportsSSBO() const { return true; diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 34e763927d8db..e03be1695ae17 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -60,6 +60,9 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| bool SupportsOffscreenMSAA() const override; + // |Capabilities| + bool SupportsImplicitResolvingMSAA() const override; + // |Capabilities| bool SupportsSSBO() const override; diff --git a/impeller/renderer/capabilities.cc b/impeller/renderer/capabilities.cc index 928d96f73865d..0774bae361c20 100644 --- a/impeller/renderer/capabilities.cc +++ b/impeller/renderer/capabilities.cc @@ -20,6 +20,9 @@ class StandardCapabilities final : public Capabilities { return supports_offscreen_msaa_; } + // |Capabilities| + bool SupportsImplicitResolvingMSAA() const override { return false; } + // |Capabilities| bool SupportsSSBO() const override { return supports_ssbo_; } diff --git a/impeller/renderer/capabilities.h b/impeller/renderer/capabilities.h index 21085ae41994f..b83e8c0935422 100644 --- a/impeller/renderer/capabilities.h +++ b/impeller/renderer/capabilities.h @@ -19,6 +19,11 @@ class Capabilities { /// color/stencil textures. virtual bool SupportsOffscreenMSAA() const = 0; + /// @brief Whether the context backend supports multisampled rendering to + /// the on-screen surface without requiring an explicit resolve of + /// the MSAA color attachment. + virtual bool SupportsImplicitResolvingMSAA() const = 0; + /// @brief Whether the context backend supports binding Shader Storage Buffer /// Objects (SSBOs) to pipelines. virtual bool SupportsSSBO() const = 0; diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 87d158e0447d3..40416a63a8837 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -286,6 +286,11 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_tex_desc.size = size; color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + // TODO: Document. + if (context.GetCapabilities()->SupportsImplicitResolvingMSAA()) { + color0_tex_desc.storage_mode = StorageMode::kDevicePrivate; + } + auto color0_msaa_tex = allocator.CreateTexture(color0_tex_desc); if (!color0_msaa_tex) { VALIDATION_LOG << "Could not create multisample color texture."; @@ -322,6 +327,17 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0.texture = color0_msaa_tex; color0.resolve_texture = color0_resolve_tex; + if (context.GetCapabilities()->SupportsImplicitResolvingMSAA()) { + // If implicit MSAA is supported, then the resolve texture is not needed + // because the multisample texture is automatically resolved. We instead + // provide a view of the multisample texture as the resolve texture (because + // the HAL does expect a resolve texture). + // + // In practice, this is used for GLES 2.0 EXT_multisampled_render_to_texture + // https://registry.khronos.org/OpenGL/extensions/EXT/EXT_multisampled_render_to_texture.txt + color0.resolve_texture = color0_msaa_tex; + } + target.SetColorAttachment(color0, 0u); // Create MSAA stencil texture. From 60b7bbb5ae3d01aa3fa822ec7524627feeefc932 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 24 Oct 2023 14:19:40 -0700 Subject: [PATCH 2/3] Update comment. --- impeller/renderer/render_target.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 40416a63a8837..3a9f1e0468e81 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -286,8 +286,8 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_tex_desc.size = size; color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - // TODO: Document. if (context.GetCapabilities()->SupportsImplicitResolvingMSAA()) { + // See below ("SupportsImplicitResolvingMSAA") for more details. color0_tex_desc.storage_mode = StorageMode::kDevicePrivate; } From a32512be458ccd38318010886b88983d8003d20e Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 24 Oct 2023 14:44:26 -0700 Subject: [PATCH 3/3] Tweak const. --- impeller/renderer/backend/gles/capabilities_gles.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index 881829839bb86..9b2251c054f39 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -19,6 +19,10 @@ static const constexpr char* kNvidiaTextureBorderClampExt = static const constexpr char* kOESTextureBorderClampExt = "GL_OES_texture_border_clamp"; +// https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_multisampled_render_to_texture.txt +static const constexpr char* kMultisampledRenderToTextureExt = + "GL_EXT_multisampled_render_to_texture"; + CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { { GLint value = 0; @@ -108,7 +112,7 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { supports_decal_sampler_address_mode_ = true; } - if (desc->HasExtension("GL_EXT_multisampled_render_to_texture")) { + if (desc->HasExtension(kMultisampledRenderToTextureExt)) { supports_implicit_msaa_ = true; // We hard-code 4x MSAA, so let's make sure it's supported.