From 26a1b71cbc192439f344606dcbe0582d3337ddd1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 19 Nov 2024 07:59:34 -0800 Subject: [PATCH 1/4] [Impeller] support GLES 3.0 multisampling. --- impeller/display_list/canvas.cc | 9 ++- .../backend/gles/capabilities_gles.cc | 4 ++ .../renderer/backend/gles/proc_table_gles.h | 1 + .../renderer/backend/gles/render_pass_gles.cc | 57 +++++++++++++++++-- .../renderer/backend/gles/texture_gles.cc | 42 ++++++++++---- 5 files changed, 95 insertions(+), 18 deletions(-) diff --git a/impeller/display_list/canvas.cc b/impeller/display_list/canvas.cc index eae343d2fcd2f..da15d9d84b07b 100644 --- a/impeller/display_list/canvas.cc +++ b/impeller/display_list/canvas.cc @@ -1667,10 +1667,15 @@ bool Canvas::BlitToOnscreen() { auto offscreen_target = render_passes_.back() .inline_pass_context->GetPassTarget() .GetRenderTarget(); - + // Unlike other backends the blit to restore the onscreen fails for GLES + // if the src is a multisample framebuffer, even if we specifically target + // the non-multisampled resolve of the dst. + bool is_gles_and_must_skip_blit = renderer_.GetContext()->GetBackendType() == + Context::BackendType::kOpenGLES; if (renderer_.GetContext() ->GetCapabilities() - ->SupportsTextureToTextureBlits()) { + ->SupportsTextureToTextureBlits() && + !is_gles_and_must_skip_blit) { auto blit_pass = command_buffer->CreateBlitPass(); blit_pass->AddCopy(offscreen_target.GetRenderTargetTexture(), render_target_.GetRenderTargetTexture()); diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index 2f3c4882851fa..6761eb5395b2b 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -127,6 +127,10 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { GLint value = 0; gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); supports_offscreen_msaa_ = value >= 4; + } else if (desc->GetGlVersion().major_version >= 3 && desc->IsES()) { + GLint value = 0; + gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); + supports_offscreen_msaa_ = value >= 4; } is_es_ = desc->IsES(); is_angle_ = desc->IsANGLE(); diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index 2917f19754714..72f3bb3ab0ca4 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -262,6 +262,7 @@ void(glDepthRange)(GLdouble n, GLdouble f); PROC(FenceSync); \ PROC(DeleteSync); \ PROC(WaitSync); \ + PROC(RenderbufferStorageMultisample) \ PROC(BlitFramebuffer); #define FOR_EACH_IMPELLER_EXT_PROC(PROC) \ diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 5ca2f5d47f8a1..71650a2572a46 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -7,7 +7,6 @@ #include #include "GLES3/gl3.h" -#include "flutter/fml/trace_event.h" #include "fml/closure.h" #include "fml/logging.h" #include "impeller/base/validation.h" @@ -127,6 +126,7 @@ 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; @@ -191,8 +191,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { const ReactorGLES& reactor, const std::vector& commands, const std::shared_ptr& tracer) { - TRACE_EVENT0("impeller", "RenderPassGLES::EncodeCommandsInReactor"); - const auto& gl = reactor.GetProcTable(); #ifdef IMPELLER_DEBUG tracer->MarkFrameStart(gl); @@ -490,7 +488,54 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { } } + if (pass_data.resolve_attachment && + pass_data.resolve_attachment != pass_data.color_attachment && + !is_default_fbo) { + // Perform multisample resolve via blit. + // Create and bind a resolve FBO. + GLuint resolve_fbo; + gl.GenFramebuffers(1u, &resolve_fbo); + gl.BindFramebuffer(GL_FRAMEBUFFER, resolve_fbo); + + if (!TextureGLES::Cast(*pass_data.resolve_attachment) + .SetAsFramebufferAttachment( + GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) { + return false; + } + + auto status = gl.CheckFramebufferStatus(GL_FRAMEBUFFER); + if (gl.CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) { + VALIDATION_LOG << "Could not create a complete frambuffer: " + << DebugToFramebufferError(status); + return false; + } + + // Bind MSAA renderbuffer to read framebuffer. + gl.BindFramebuffer(GL_READ_FRAMEBUFFER, fbo); + gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, resolve_fbo); + + RenderPassGLES::ResetGLState(gl); + auto size = pass_data.color_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 + ); + + gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, GL_NONE); + gl.BindFramebuffer(GL_READ_FRAMEBUFFER, GL_NONE); + gl.DeleteFramebuffers(1u, &resolve_fbo); + } + if (gl.DiscardFramebufferEXT.IsAvailable()) { + gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); std::vector attachments; // TODO(130048): discarding stencil or depth on the default fbo causes Angle @@ -547,6 +592,7 @@ 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 = @@ -556,8 +602,9 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { // 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; + pass_data->discard_color_attachment = + pass_data->discard_color_attachment && + !context.GetCapabilities()->SupportsImplicitResolvingMSAA(); } //---------------------------------------------------------------------------- diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 1b4b47f74b373..44130fa3e91c2 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -45,7 +45,8 @@ static bool IsDepthStencilFormat(PixelFormat format) { } static TextureGLES::Type GetTextureTypeFromDescriptor( - const TextureDescriptor& desc) { + const TextureDescriptor& desc, + bool supports_implict_msaa) { const auto usage = static_cast(desc.usage); const auto render_target = TextureUsage::kRenderTarget; const auto is_msaa = desc.sample_count == SampleCount::kCount4; @@ -53,7 +54,9 @@ static TextureGLES::Type GetTextureTypeFromDescriptor( return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled : TextureGLES::Type::kRenderBuffer; } - return is_msaa ? TextureGLES::Type::kTextureMultisampled + return is_msaa ? (supports_implict_msaa + ? TextureGLES::Type::kTextureMultisampled + : TextureGLES::Type::kRenderBufferMultisampled) : TextureGLES::Type::kTexture; } @@ -190,7 +193,11 @@ TextureGLES::TextureGLES(std::shared_ptr reactor, std::optional external_handle) : Texture(desc), reactor_(std::move(reactor)), - type_(GetTextureTypeFromDescriptor(GetTextureDescriptor())), + type_( + GetTextureTypeFromDescriptor(GetTextureDescriptor(), + reactor_->GetProcTable() + .GetCapabilities() + ->SupportsImplicitResolvingMSAA())), handle_(external_handle.has_value() ? external_handle.value() : reactor_->CreateHandle(ToHandleType(type_))), @@ -362,7 +369,7 @@ static std::optional ToRenderBufferFormat(PixelFormat format) { switch (format) { case PixelFormat::kB8G8R8A8UNormInt: case PixelFormat::kR8G8B8A8UNormInt: - return GL_RGBA4; + return GL_RGBA8; case PixelFormat::kR32G32B32A32Float: return GL_RGBA32F; case PixelFormat::kR16G16B16A16Float: @@ -445,13 +452,26 @@ void TextureGLES::InitializeContentsIfNecessary() const { { TRACE_EVENT0("impeller", "RenderBufferStorageInitialization"); if (type_ == Type::kRenderBufferMultisampled) { - gl.RenderbufferStorageMultisampleEXT( - GL_RENDERBUFFER, // target - 4, // samples - render_buffer_format.value(), // internal format - size.width, // width - size.height // height - ); + // BEWARE: these functions are not at all equivalent! the extensions + // are from EXT_multisampled_render_to_texture and cannot be used + // with regular GLES 3.0 multisampled renderbuffers/textures. + if (gl.GetCapabilities()->SupportsImplicitResolvingMSAA()) { + gl.RenderbufferStorageMultisampleEXT( + GL_RENDERBUFFER, // target + 4, // samples + render_buffer_format.value(), // internal format + size.width, // width + size.height // height + ); + } else { + gl.RenderbufferStorageMultisample( + GL_RENDERBUFFER, // target + 4, // samples + render_buffer_format.value(), // internal format + size.width, // width + size.height // height + ); + } } else { gl.RenderbufferStorage( GL_RENDERBUFFER, // target From 82c0fce4d901b65876fd84922f8483526ddfbb03 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 19 Nov 2024 11:10:50 -0800 Subject: [PATCH 2/4] Comments and conditional. --- .../backend/gles/capabilities_gles.cc | 4 +-- .../renderer/backend/gles/render_pass_gles.cc | 3 +- .../renderer/backend/gles/texture_gles.cc | 28 +++++++++---------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index 6761eb5395b2b..e6851048d8969 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -125,11 +125,11 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { // We hard-code 4x MSAA, so let's make sure it's supported. GLint value = 0; - gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); + gl.GetIntegerv(GL_MAX_SAMPLES, &value); supports_offscreen_msaa_ = value >= 4; } else if (desc->GetGlVersion().major_version >= 3 && desc->IsES()) { GLint value = 0; - gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value); + gl.GetIntegerv(GL_MAX_SAMPLES, &value); supports_offscreen_msaa_ = value >= 4; } is_es_ = desc->IsES(); diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 71650a2572a46..9f0ecf699cbd6 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -532,10 +532,11 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, GL_NONE); gl.BindFramebuffer(GL_READ_FRAMEBUFFER, GL_NONE); gl.DeleteFramebuffers(1u, &resolve_fbo); + // Rebind the original FBO so that we can discard it below. + gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); } if (gl.DiscardFramebufferEXT.IsAvailable()) { - gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); std::vector attachments; // TODO(130048): discarding stencil or depth on the default fbo causes Angle diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 44130fa3e91c2..6a911aebf0bec 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -457,27 +457,27 @@ void TextureGLES::InitializeContentsIfNecessary() const { // with regular GLES 3.0 multisampled renderbuffers/textures. if (gl.GetCapabilities()->SupportsImplicitResolvingMSAA()) { gl.RenderbufferStorageMultisampleEXT( - GL_RENDERBUFFER, // target - 4, // samples - render_buffer_format.value(), // internal format - size.width, // width - size.height // height + /*target=*/GL_RENDERBUFFER, // + /*samples=*/4, // + /*internal_format=*/render_buffer_format.value(), // + /*width=*/size.width, // + /*height=*/size.height // ); } else { gl.RenderbufferStorageMultisample( - GL_RENDERBUFFER, // target - 4, // samples - render_buffer_format.value(), // internal format - size.width, // width - size.height // height + /*target=*/GL_RENDERBUFFER, // + /*samples=*/4, // + /*internal_format=*/render_buffer_format.value(), // + /*width=*/size.width, // + /*height=*/size.height // ); } } else { gl.RenderbufferStorage( - GL_RENDERBUFFER, // target - render_buffer_format.value(), // internal format - size.width, // width - size.height // height + /*target=*/GL_RENDERBUFFER, // + /*internal_format=*/render_buffer_format.value(), // + /*width=*/size.width, // + /*height=*/size.height // ); } } From 9561d9189f521627039c25eafa4c601c2ef39e42 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 19 Nov 2024 12:17:10 -0800 Subject: [PATCH 3/4] fix conditional. --- impeller/renderer/backend/gles/render_pass_gles.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 9f0ecf699cbd6..8b79f079716f1 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -6,7 +6,6 @@ #include -#include "GLES3/gl3.h" #include "fml/closure.h" #include "fml/logging.h" #include "impeller/base/validation.h" @@ -489,8 +488,9 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { } if (pass_data.resolve_attachment && - pass_data.resolve_attachment != pass_data.color_attachment && + !gl.GetCapabilities()->SupportsImplicitResolvingMSAA() && !is_default_fbo) { + FML_DCHECK(pass_data.resolve_attachment != pass_data.color_attachment); // Perform multisample resolve via blit. // Create and bind a resolve FBO. GLuint resolve_fbo; From 93beb955a2c489396186f3649575d657ebb454f8 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 19 Nov 2024 13:07:39 -0800 Subject: [PATCH 4/4] Update render_pass_gles.cc --- impeller/renderer/backend/gles/render_pass_gles.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 8b79f079716f1..e8d314b2a175e 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -6,6 +6,7 @@ #include +#include "GLES3/gl3.h" #include "fml/closure.h" #include "fml/logging.h" #include "impeller/base/validation.h"