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

Commit 6f941c9

Browse files
Reverts "[Impeller] support GLES 3.0 MSAA without extension. (#56705)" (#56741)
Reverts: #56705 Initiated by: jonahwilliams Reason for reverting: goldens occassionally fail to render anything. Original PR Author: jonahwilliams Reviewed By: {gaaclarke} This change reverts the following previous change: Adds multisampling support for GLES devices without GL_EXT_multisampled_render_to_texture provided they are at least GLES 3.0 to support mutlisampled render buffers. Fixes flutter/flutter#158360 Fixes flutter/flutter#157951 TBD: should we prefer renderbuffer 3.0 approach over multisample_render_to_texture?
1 parent c348218 commit 6f941c9

5 files changed

Lines changed: 23 additions & 102 deletions

File tree

impeller/display_list/canvas.cc

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,15 +1667,10 @@ bool Canvas::BlitToOnscreen() {
16671667
auto offscreen_target = render_passes_.back()
16681668
.inline_pass_context->GetPassTarget()
16691669
.GetRenderTarget();
1670-
// Unlike other backends the blit to restore the onscreen fails for GLES
1671-
// if the src is a multisample framebuffer, even if we specifically target
1672-
// the non-multisampled resolve of the dst.
1673-
bool is_gles_and_must_skip_blit = renderer_.GetContext()->GetBackendType() ==
1674-
Context::BackendType::kOpenGLES;
1670+
16751671
if (renderer_.GetContext()
16761672
->GetCapabilities()
1677-
->SupportsTextureToTextureBlits() &&
1678-
!is_gles_and_must_skip_blit) {
1673+
->SupportsTextureToTextureBlits()) {
16791674
auto blit_pass = command_buffer->CreateBlitPass();
16801675
blit_pass->AddCopy(offscreen_target.GetRenderTargetTexture(),
16811676
render_target_.GetRenderTargetTexture());

impeller/renderer/backend/gles/capabilities_gles.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,7 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) {
125125

126126
// We hard-code 4x MSAA, so let's make sure it's supported.
127127
GLint value = 0;
128-
gl.GetIntegerv(GL_MAX_SAMPLES, &value);
129-
supports_offscreen_msaa_ = value >= 4;
130-
} else if (desc->GetGlVersion().major_version >= 3 && desc->IsES()) {
131-
GLint value = 0;
132-
gl.GetIntegerv(GL_MAX_SAMPLES, &value);
128+
gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value);
133129
supports_offscreen_msaa_ = value >= 4;
134130
}
135131
is_es_ = desc->IsES();

impeller/renderer/backend/gles/proc_table_gles.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,6 @@ void(glDepthRange)(GLdouble n, GLdouble f);
242242
PROC(FenceSync); \
243243
PROC(DeleteSync); \
244244
PROC(WaitSync); \
245-
PROC(RenderbufferStorageMultisample) \
246245
PROC(BlitFramebuffer);
247246

248247
#define FOR_EACH_IMPELLER_EXT_PROC(PROC) \

impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <cstdint>
88

99
#include "GLES3/gl3.h"
10+
#include "flutter/fml/trace_event.h"
1011
#include "fml/closure.h"
1112
#include "fml/logging.h"
1213
#include "impeller/base/validation.h"
@@ -126,7 +127,6 @@ struct RenderPassData {
126127
Scalar clear_depth = 1.0;
127128

128129
std::shared_ptr<Texture> color_attachment;
129-
std::shared_ptr<Texture> resolve_attachment;
130130
std::shared_ptr<Texture> depth_attachment;
131131
std::shared_ptr<Texture> stencil_attachment;
132132

@@ -191,6 +191,8 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
191191
const ReactorGLES& reactor,
192192
const std::vector<Command>& commands,
193193
const std::shared_ptr<GPUTracerGLES>& tracer) {
194+
TRACE_EVENT0("impeller", "RenderPassGLES::EncodeCommandsInReactor");
195+
194196
const auto& gl = reactor.GetProcTable();
195197
#ifdef IMPELLER_DEBUG
196198
tracer->MarkFrameStart(gl);
@@ -488,55 +490,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
488490
}
489491
}
490492

491-
if (pass_data.resolve_attachment &&
492-
!gl.GetCapabilities()->SupportsImplicitResolvingMSAA() &&
493-
!is_default_fbo) {
494-
FML_DCHECK(pass_data.resolve_attachment != pass_data.color_attachment);
495-
// Perform multisample resolve via blit.
496-
// Create and bind a resolve FBO.
497-
GLuint resolve_fbo;
498-
gl.GenFramebuffers(1u, &resolve_fbo);
499-
gl.BindFramebuffer(GL_FRAMEBUFFER, resolve_fbo);
500-
501-
if (!TextureGLES::Cast(*pass_data.resolve_attachment)
502-
.SetAsFramebufferAttachment(
503-
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
504-
return false;
505-
}
506-
507-
auto status = gl.CheckFramebufferStatus(GL_FRAMEBUFFER);
508-
if (gl.CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) {
509-
VALIDATION_LOG << "Could not create a complete frambuffer: "
510-
<< DebugToFramebufferError(status);
511-
return false;
512-
}
513-
514-
// Bind MSAA renderbuffer to read framebuffer.
515-
gl.BindFramebuffer(GL_READ_FRAMEBUFFER, fbo);
516-
gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, resolve_fbo);
517-
518-
RenderPassGLES::ResetGLState(gl);
519-
auto size = pass_data.color_attachment->GetSize();
520-
521-
gl.BlitFramebuffer(0, // srcX0
522-
0, // srcY0
523-
size.width, // srcX1
524-
size.height, // srcY1
525-
0, // dstX0
526-
0, // dstY0
527-
size.width, // dstX1
528-
size.height, // dstY1
529-
GL_COLOR_BUFFER_BIT, // mask
530-
GL_NEAREST // filter
531-
);
532-
533-
gl.BindFramebuffer(GL_DRAW_FRAMEBUFFER, GL_NONE);
534-
gl.BindFramebuffer(GL_READ_FRAMEBUFFER, GL_NONE);
535-
gl.DeleteFramebuffers(1u, &resolve_fbo);
536-
// Rebind the original FBO so that we can discard it below.
537-
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo);
538-
}
539-
540493
if (gl.DiscardFramebufferEXT.IsAvailable()) {
541494
std::vector<GLenum> attachments;
542495

@@ -594,7 +547,6 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const {
594547
/// Setup color data.
595548
///
596549
pass_data->color_attachment = color0.texture;
597-
pass_data->resolve_attachment = color0.resolve_texture;
598550
pass_data->clear_color = color0.clear_color;
599551
pass_data->clear_color_attachment = CanClearAttachment(color0.load_action);
600552
pass_data->discard_color_attachment =
@@ -604,9 +556,8 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const {
604556
// resolved when we bind the texture to the framebuffer. We don't need to
605557
// discard the attachment when we are done.
606558
if (color0.resolve_texture) {
607-
pass_data->discard_color_attachment =
608-
pass_data->discard_color_attachment &&
609-
!context.GetCapabilities()->SupportsImplicitResolvingMSAA();
559+
FML_DCHECK(context.GetCapabilities()->SupportsImplicitResolvingMSAA());
560+
pass_data->discard_color_attachment = false;
610561
}
611562

612563
//----------------------------------------------------------------------------

impeller/renderer/backend/gles/texture_gles.cc

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,15 @@ static bool IsDepthStencilFormat(PixelFormat format) {
4545
}
4646

4747
static TextureGLES::Type GetTextureTypeFromDescriptor(
48-
const TextureDescriptor& desc,
49-
bool supports_implict_msaa) {
48+
const TextureDescriptor& desc) {
5049
const auto usage = static_cast<TextureUsageMask>(desc.usage);
5150
const auto render_target = TextureUsage::kRenderTarget;
5251
const auto is_msaa = desc.sample_count == SampleCount::kCount4;
5352
if (usage == render_target && IsDepthStencilFormat(desc.format)) {
5453
return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled
5554
: TextureGLES::Type::kRenderBuffer;
5655
}
57-
return is_msaa ? (supports_implict_msaa
58-
? TextureGLES::Type::kTextureMultisampled
59-
: TextureGLES::Type::kRenderBufferMultisampled)
56+
return is_msaa ? TextureGLES::Type::kTextureMultisampled
6057
: TextureGLES::Type::kTexture;
6158
}
6259

@@ -193,11 +190,7 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
193190
std::optional<HandleGLES> external_handle)
194191
: Texture(desc),
195192
reactor_(std::move(reactor)),
196-
type_(
197-
GetTextureTypeFromDescriptor(GetTextureDescriptor(),
198-
reactor_->GetProcTable()
199-
.GetCapabilities()
200-
->SupportsImplicitResolvingMSAA())),
193+
type_(GetTextureTypeFromDescriptor(GetTextureDescriptor())),
201194
handle_(external_handle.has_value()
202195
? external_handle.value()
203196
: reactor_->CreateHandle(ToHandleType(type_))),
@@ -369,7 +362,7 @@ static std::optional<GLenum> ToRenderBufferFormat(PixelFormat format) {
369362
switch (format) {
370363
case PixelFormat::kB8G8R8A8UNormInt:
371364
case PixelFormat::kR8G8B8A8UNormInt:
372-
return GL_RGBA8;
365+
return GL_RGBA4;
373366
case PixelFormat::kR32G32B32A32Float:
374367
return GL_RGBA32F;
375368
case PixelFormat::kR16G16B16A16Float:
@@ -452,32 +445,19 @@ void TextureGLES::InitializeContentsIfNecessary() const {
452445
{
453446
TRACE_EVENT0("impeller", "RenderBufferStorageInitialization");
454447
if (type_ == Type::kRenderBufferMultisampled) {
455-
// BEWARE: these functions are not at all equivalent! the extensions
456-
// are from EXT_multisampled_render_to_texture and cannot be used
457-
// with regular GLES 3.0 multisampled renderbuffers/textures.
458-
if (gl.GetCapabilities()->SupportsImplicitResolvingMSAA()) {
459-
gl.RenderbufferStorageMultisampleEXT(
460-
/*target=*/GL_RENDERBUFFER, //
461-
/*samples=*/4, //
462-
/*internal_format=*/render_buffer_format.value(), //
463-
/*width=*/size.width, //
464-
/*height=*/size.height //
465-
);
466-
} else {
467-
gl.RenderbufferStorageMultisample(
468-
/*target=*/GL_RENDERBUFFER, //
469-
/*samples=*/4, //
470-
/*internal_format=*/render_buffer_format.value(), //
471-
/*width=*/size.width, //
472-
/*height=*/size.height //
473-
);
474-
}
448+
gl.RenderbufferStorageMultisampleEXT(
449+
GL_RENDERBUFFER, // target
450+
4, // samples
451+
render_buffer_format.value(), // internal format
452+
size.width, // width
453+
size.height // height
454+
);
475455
} else {
476456
gl.RenderbufferStorage(
477-
/*target=*/GL_RENDERBUFFER, //
478-
/*internal_format=*/render_buffer_format.value(), //
479-
/*width=*/size.width, //
480-
/*height=*/size.height //
457+
GL_RENDERBUFFER, // target
458+
render_buffer_format.value(), // internal format
459+
size.width, // width
460+
size.height // height
481461
);
482462
}
483463
}

0 commit comments

Comments
 (0)