Skip to content

Commit

Permalink
Reverts "[Impeller] Switch from transient stencil-only to depth+stenc…
Browse files Browse the repository at this point in the history 
…il buffer." (flutter#49832)

Reverts flutter#47987
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
Part of flutter/flutter#138460.

In preparation for [draw order optimization](flutter/flutter#114402) and [StC](flutter/flutter#123671).

Use a transient depth+stencil texture instead of a stencil-only texture. Doing this in isolation to detect/weed out any HAL bugs with handling the attachment.
  • Loading branch information
@auto-submit
auto-submit[bot] authored Jan 17, 2024
1 parent 6a5fb8f commit c167107
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 119 deletions.
2 changes: 0 additions & 2 deletions impeller/aiks/aiks_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3557,8 +3557,6 @@ TEST_P(AiksTest, GaussianBlurWithoutDecalSupport) {
.WillRepeatedly(::testing::Return(false));
FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultColorFormat);
FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultStencilFormat);
FLT_FORWARD(mock_capabilities, old_capabilities,
GetDefaultDepthStencilFormat);
FLT_FORWARD(mock_capabilities, old_capabilities, SupportsOffscreenMSAA);
FLT_FORWARD(mock_capabilities, old_capabilities,
SupportsImplicitResolvingMSAA);
Expand Down
5 changes: 2 additions & 3 deletions impeller/entity/entity_pass.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,7 @@ bool EntityPass::Render(ContentContext& renderer,
// If a root stencil was provided by the caller, then verify that it has a
// configuration which can be used to render this pass.
auto stencil_attachment = root_render_target.GetStencilAttachment();
auto depth_attachment = root_render_target.GetDepthAttachment();
if (stencil_attachment.has_value() && depth_attachment.has_value()) {
if (stencil_attachment.has_value()) {
auto stencil_texture = stencil_attachment->texture;
if (!stencil_texture) {
VALIDATION_LOG << "The root RenderTarget must have a stencil texture.";
Expand All @@ -443,7 +442,7 @@ bool EntityPass::Render(ContentContext& renderer,
// Setup a new root stencil with an optimal configuration if one wasn't
// provided by the caller.
else {
root_render_target.SetupDepthStencilAttachments(
root_render_target.SetupStencilAttachment(
*renderer.GetContext(), *renderer.GetRenderTargetCache(),
color0.texture->GetSize(),
renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(),
Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/gles/blit_command_gles.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static std::optional<GLuint> ConfigureFBO(
gl.BindFramebuffer(fbo_type, fbo);

if (!TextureGLES::Cast(*texture).SetAsFramebufferAttachment(
fbo_type, TextureGLES::AttachmentType::kColor0)) {
fbo_type, TextureGLES::AttachmentPoint::kColor0)) {
VALIDATION_LOG << "Could not attach texture to framebuffer.";
DeleteFBO(gl, fbo, fbo_type);
return std::nullopt;
Expand Down
6 changes: 3 additions & 3 deletions impeller/renderer/backend/gles/render_pass_gles.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,20 +186,20 @@ struct RenderPassData {

if (auto color = TextureGLES::Cast(pass_data.color_attachment.get())) {
if (!color->SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kColor0)) {
return false;
}
}

if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) {
if (!depth->SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) {
GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kDepth)) {
return false;
}
}
if (auto stencil = TextureGLES::Cast(pass_data.stencil_attachment.get())) {
if (!stencil->SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) {
GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kStencil)) {
return false;
}
}
Expand Down
71 changes: 22 additions & 49 deletions impeller/renderer/backend/gles/texture_gles.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <optional>
#include <utility>

#include "flutter/fml/logging.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/trace_event.h"
#include "impeller/base/allocation.h"
Expand All @@ -18,37 +17,13 @@

namespace impeller {

static bool IsDepthStencilFormat(PixelFormat format) {
switch (format) {
case PixelFormat::kS8UInt:
case PixelFormat::kD24UnormS8Uint:
case PixelFormat::kD32FloatS8UInt:
return true;
case PixelFormat::kUnknown:
case PixelFormat::kA8UNormInt:
case PixelFormat::kR8UNormInt:
case PixelFormat::kR8G8UNormInt:
case PixelFormat::kR8G8B8A8UNormInt:
case PixelFormat::kR8G8B8A8UNormIntSRGB:
case PixelFormat::kB8G8R8A8UNormInt:
case PixelFormat::kB8G8R8A8UNormIntSRGB:
case PixelFormat::kR32G32B32A32Float:
case PixelFormat::kR16G16B16A16Float:
case PixelFormat::kB10G10R10XR:
case PixelFormat::kB10G10R10XRSRGB:
case PixelFormat::kB10G10R10A10XR:
return false;
}
FML_UNREACHABLE();
}

static TextureGLES::Type GetTextureTypeFromDescriptor(
const TextureDescriptor& desc) {
const auto usage = static_cast<TextureUsageMask>(desc.usage);
const auto render_target =
static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
const auto is_msaa = desc.sample_count == SampleCount::kCount4;
if (usage == render_target && IsDepthStencilFormat(desc.format)) {
if (usage == render_target && desc.format == PixelFormat::kS8UInt) {
return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled
: TextureGLES::Type::kRenderBuffer;
}
Expand Down Expand Up @@ -482,20 +457,19 @@ TextureGLES::Type TextureGLES::GetType() const {
return type_;
}

static GLenum ToAttachmentType(TextureGLES::AttachmentType point) {
static GLenum ToAttachmentPoint(TextureGLES::AttachmentPoint point) {
switch (point) {
case TextureGLES::AttachmentType::kColor0:
case TextureGLES::AttachmentPoint::kColor0:
return GL_COLOR_ATTACHMENT0;
case TextureGLES::AttachmentType::kDepth:
case TextureGLES::AttachmentPoint::kDepth:
return GL_DEPTH_ATTACHMENT;
case TextureGLES::AttachmentType::kStencil:
case TextureGLES::AttachmentPoint::kStencil:
return GL_STENCIL_ATTACHMENT;
}
}

bool TextureGLES::SetAsFramebufferAttachment(
GLenum target,
AttachmentType attachment_type) const {
bool TextureGLES::SetAsFramebufferAttachment(GLenum target,
AttachmentPoint point) const {
if (!IsValid()) {
return false;
}
Expand All @@ -508,30 +482,29 @@ bool TextureGLES::SetAsFramebufferAttachment(

switch (type_) {
case Type::kTexture:
gl.FramebufferTexture2D(target, // target
ToAttachmentType(attachment_type), // attachment
GL_TEXTURE_2D, // textarget
handle.value(), // texture
0 // level
gl.FramebufferTexture2D(target, // target
ToAttachmentPoint(point), // attachment
GL_TEXTURE_2D, // textarget
handle.value(), // texture
0 // level
);
break;
case Type::kTextureMultisampled:
gl.FramebufferTexture2DMultisampleEXT(
target, // target
ToAttachmentType(attachment_type), // attachment
GL_TEXTURE_2D, // textarget
handle.value(), // texture
0, // level
4 // samples
target, // target
ToAttachmentPoint(point), // attachment
GL_TEXTURE_2D, // textarget
handle.value(), // texture
0, // level
4 // samples
);
break;
case Type::kRenderBuffer:
case Type::kRenderBufferMultisampled:
gl.FramebufferRenderbuffer(
target, // target
ToAttachmentType(attachment_type), // attachment
GL_RENDERBUFFER, // render-buffer target
handle.value() // render-buffer
gl.FramebufferRenderbuffer(target, // target
ToAttachmentPoint(point), // attachment
GL_RENDERBUFFER, // render-buffer target
handle.value() // render-buffer
);
break;
}
Expand Down
7 changes: 3 additions & 4 deletions impeller/renderer/backend/gles/texture_gles.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@ class TextureGLES final : public Texture,

[[nodiscard]] bool GenerateMipmap();

enum class AttachmentType {
enum class AttachmentPoint {
kColor0,
kDepth,
kStencil,
};
[[nodiscard]] bool SetAsFramebufferAttachment(
GLenum target,
AttachmentType attachment_type) const;
[[nodiscard]] bool SetAsFramebufferAttachment(GLenum target,
AttachmentPoint point) const;

Type GetType() const;

Expand Down
50 changes: 17 additions & 33 deletions impeller/renderer/render_target.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ RenderTarget RenderTarget::CreateOffscreen(
target.SetColorAttachment(color0, 0u);

if (stencil_attachment_config.has_value()) {
target.SetupDepthStencilAttachments(context, allocator, size, false, label,
stencil_attachment_config.value());
target.SetupStencilAttachment(context, allocator, size, false, label,
stencil_attachment_config.value());
} else {
target.SetStencilAttachment(std::nullopt);
}
Expand Down Expand Up @@ -347,56 +347,43 @@ RenderTarget RenderTarget::CreateOffscreenMSAA(
// Create MSAA stencil texture.

if (stencil_attachment_config.has_value()) {
target.SetupDepthStencilAttachments(context, allocator, size, true, label,
stencil_attachment_config.value());
target.SetupStencilAttachment(context, allocator, size, true, label,
stencil_attachment_config.value());
} else {
target.SetStencilAttachment(std::nullopt);
}

return target;
}

void RenderTarget::SetupDepthStencilAttachments(
void RenderTarget::SetupStencilAttachment(
const Context& context,
RenderTargetAllocator& allocator,
ISize size,
bool msaa,
const std::string& label,
AttachmentConfig stencil_attachment_config) {
TextureDescriptor depth_stencil_texture_desc;
depth_stencil_texture_desc.storage_mode =
stencil_attachment_config.storage_mode;
TextureDescriptor stencil_tex0;
stencil_tex0.storage_mode = stencil_attachment_config.storage_mode;
if (msaa) {
depth_stencil_texture_desc.type = TextureType::kTexture2DMultisample;
depth_stencil_texture_desc.sample_count = SampleCount::kCount4;
stencil_tex0.type = TextureType::kTexture2DMultisample;
stencil_tex0.sample_count = SampleCount::kCount4;
}
depth_stencil_texture_desc.format =
context.GetCapabilities()->GetDefaultDepthStencilFormat();
depth_stencil_texture_desc.size = size;
depth_stencil_texture_desc.usage =
stencil_tex0.format = context.GetCapabilities()->GetDefaultStencilFormat();
stencil_tex0.size = size;
stencil_tex0.usage =
static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);

auto depth_stencil_texture =
allocator.CreateTexture(depth_stencil_texture_desc);
if (!depth_stencil_texture) {
return; // Error messages are handled by `Allocator::CreateTexture`.
}

DepthAttachment depth0;
depth0.load_action = stencil_attachment_config.load_action;
depth0.store_action = stencil_attachment_config.store_action;
depth0.clear_depth = 0u;
depth0.texture = depth_stencil_texture;

StencilAttachment stencil0;
stencil0.load_action = stencil_attachment_config.load_action;
stencil0.store_action = stencil_attachment_config.store_action;
stencil0.clear_stencil = 0u;
stencil0.texture = depth_stencil_texture;
stencil0.texture = allocator.CreateTexture(stencil_tex0);

stencil0.texture->SetLabel(
SPrintF("%s Depth+Stencil Texture", label.c_str()));
SetDepthAttachment(std::move(depth0));
if (!stencil0.texture) {
return; // Error messages are handled by `Allocator::CreateTexture`.
}
stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str()));
SetStencilAttachment(std::move(stencil0));
}

Expand All @@ -416,9 +403,6 @@ size_t RenderTarget::GetTotalAttachmentCount() const {
if (stencil_.has_value()) {
count++;
}
if (depth_.has_value()) {
count++;
}
return count;
}

Expand Down
14 changes: 7 additions & 7 deletions impeller/renderer/render_target.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ class RenderTarget final {

bool IsValid() const;

void SetupDepthStencilAttachments(const Context& context,
RenderTargetAllocator& allocator,
ISize size,
bool msaa,
const std::string& label = "Offscreen",
AttachmentConfig stencil_attachment_config =
kDefaultStencilAttachmentConfig);
void SetupStencilAttachment(const Context& context,
RenderTargetAllocator& allocator,
ISize size,
bool msaa,
const std::string& label = "Offscreen",
AttachmentConfig stencil_attachment_config =
kDefaultStencilAttachmentConfig);

SampleCount GetSampleCount() const;

Expand Down
20 changes: 3 additions & 17 deletions impeller/renderer/renderer_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1161,9 +1161,9 @@ TEST_P(RendererTest, StencilMask) {
stencil_config.storage_mode = StorageMode::kHostVisible;
auto render_target_allocator =
RenderTargetAllocator(context->GetResourceAllocator());
render_target.SetupDepthStencilAttachments(
*context, render_target_allocator,
render_target.GetRenderTargetSize(), true, "stencil", stencil_config);
render_target.SetupStencilAttachment(*context, render_target_allocator,
render_target.GetRenderTargetSize(),
true, "stencil", stencil_config);
// Fill the stencil buffer with an checkerboard pattern.
const auto target_width = render_target.GetRenderTargetSize().width;
const auto target_height = render_target.GetRenderTargetSize().height;
Expand Down Expand Up @@ -1289,20 +1289,6 @@ TEST_P(RendererTest, CanLookupRenderTargetProperties) {
render_target.GetRenderTargetSize());
}

TEST_P(RendererTest,
RenderTargetCreateOffscreenMSAASetsDefaultDepthStencilFormat) {
auto context = GetContext();
auto render_target_cache = std::make_shared<RenderTargetAllocator>(
GetContext()->GetResourceAllocator());

RenderTarget render_target = RenderTarget::CreateOffscreenMSAA(
*context, *render_target_cache, {100, 100}, /*mip_count=*/1);
EXPECT_EQ(render_target.GetDepthAttachment()
->texture->GetTextureDescriptor()
.format,
GetContext()->GetCapabilities()->GetDefaultDepthStencilFormat());
}

} // namespace testing
} // namespace impeller

Expand Down

0 comments on commit c167107

Please sign in to comment.