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

Commit f31faf3

Browse files
author
Jonah Williams
authored
[Impeller] use optimal depth attachment, remove useless barrier. (#51723)
We have been transitioning the depth/stencil atttachment to general, even though it can just be in attachment optimal (we never use it as an input attachment).
1 parent c602abd commit f31faf3

File tree

6 files changed

+134
-6
lines changed

6 files changed

+134
-6
lines changed

‎ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc
178178
../../../flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc
179179
../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc
180+
../../../flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc
180181
../../../flutter/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
181182
../../../flutter/impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc
182183
../../../flutter/impeller/renderer/backend/vulkan/swapchain/README.md

‎impeller/renderer/backend/vulkan/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ impeller_component("vulkan_unittests") {
1616
"descriptor_pool_vk_unittests.cc",
1717
"driver_info_vk_unittests.cc",
1818
"fence_waiter_vk_unittests.cc",
19+
"render_pass_builder_vk_unittests.cc",
1920
"render_pass_cache_unittests.cc",
2021
"resource_manager_vk_unittests.cc",
2122
"test/gpu_tracer_unittests.cc",

‎impeller/renderer/backend/vulkan/render_pass_builder_vk.cc

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetDepthStencilAttachment(
6262
desc.storeOp = ToVKAttachmentStoreOp(store_action);
6363
desc.stencilLoadOp = desc.loadOp; // Not separable in Impeller.
6464
desc.stencilStoreOp = desc.storeOp; // Not separable in Impeller.
65-
desc.initialLayout = vk::ImageLayout::eGeneral;
66-
desc.finalLayout = vk::ImageLayout::eGeneral;
65+
desc.initialLayout = vk::ImageLayout::eUndefined;
66+
desc.finalLayout = vk::ImageLayout::eDepthStencilAttachmentOptimal;
6767
depth_stencil_ = desc;
6868
return *this;
6969
}
@@ -80,8 +80,8 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetStencilAttachment(
8080
desc.storeOp = vk::AttachmentStoreOp::eDontCare;
8181
desc.stencilLoadOp = ToVKAttachmentLoadOp(load_action);
8282
desc.stencilStoreOp = ToVKAttachmentStoreOp(store_action);
83-
desc.initialLayout = vk::ImageLayout::eGeneral;
84-
desc.finalLayout = vk::ImageLayout::eGeneral;
83+
desc.initialLayout = vk::ImageLayout::eUndefined;
84+
desc.finalLayout = vk::ImageLayout::eDepthStencilAttachmentOptimal;
8585
depth_stencil_ = desc;
8686
return *this;
8787
}
@@ -184,4 +184,19 @@ void InsertBarrierForInputAttachmentRead(const vk::CommandBuffer& buffer,
184184
);
185185
}
186186

187+
const std::map<size_t, vk::AttachmentDescription>&
188+
RenderPassBuilderVK::GetColorAttachments() const {
189+
return colors_;
190+
}
191+
192+
const std::map<size_t, vk::AttachmentDescription>&
193+
RenderPassBuilderVK::GetResolves() const {
194+
return resolves_;
195+
}
196+
197+
const std::optional<vk::AttachmentDescription>&
198+
RenderPassBuilderVK::GetDepthStencil() const {
199+
return depth_stencil_;
200+
}
201+
187202
} // namespace impeller

‎impeller/renderer/backend/vulkan/render_pass_builder_vk.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ class RenderPassBuilderVK {
4242

4343
vk::UniqueRenderPass Build(const vk::Device& device) const;
4444

45+
// Visible for testing.
46+
const std::map<size_t, vk::AttachmentDescription>& GetColorAttachments()
47+
const;
48+
49+
// Visible for testing.
50+
const std::map<size_t, vk::AttachmentDescription>& GetResolves() const;
51+
52+
// Visible for testing.
53+
const std::optional<vk::AttachmentDescription>& GetDepthStencil() const;
54+
4555
private:
4656
std::map<size_t, vk::AttachmentDescription> colors_;
4757
std::map<size_t, vk::AttachmentDescription> resolves_;

‎impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/testing/testing.h" // IWYU pragma: keep
6+
#include "gtest/gtest.h"
7+
#include "impeller/core/formats.h"
8+
#include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h"
9+
#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
10+
#include "vulkan/vulkan_enums.hpp"
11+
12+
namespace impeller {
13+
namespace testing {
14+
15+
TEST(RenderPassBuilder, CreatesRenderPassWithNoDepthStencil) {
16+
RenderPassBuilderVK builder = RenderPassBuilderVK();
17+
auto const context = MockVulkanContextBuilder().Build();
18+
19+
// Create a single color attachment with a transient depth stencil.
20+
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
21+
SampleCount::kCount1, LoadAction::kClear,
22+
StoreAction::kStore);
23+
24+
auto render_pass = builder.Build(context->GetDevice());
25+
26+
EXPECT_TRUE(!!render_pass);
27+
EXPECT_FALSE(builder.GetDepthStencil().has_value());
28+
}
29+
30+
TEST(RenderPassBuilder, CreatesRenderPassWithCombinedDepthStencil) {
31+
RenderPassBuilderVK builder = RenderPassBuilderVK();
32+
auto const context = MockVulkanContextBuilder().Build();
33+
34+
// Create a single color attachment with a transient depth stencil.
35+
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
36+
SampleCount::kCount1, LoadAction::kClear,
37+
StoreAction::kStore);
38+
builder.SetDepthStencilAttachment(PixelFormat::kD24UnormS8Uint,
39+
SampleCount::kCount1, LoadAction::kDontCare,
40+
StoreAction::kDontCare);
41+
42+
auto render_pass = builder.Build(context->GetDevice());
43+
44+
EXPECT_TRUE(!!render_pass);
45+
46+
auto maybe_color = builder.GetColorAttachments().find(0u);
47+
ASSERT_NE(maybe_color, builder.GetColorAttachments().end());
48+
auto color = maybe_color->second;
49+
50+
EXPECT_EQ(color.initialLayout, vk::ImageLayout::eGeneral);
51+
EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral);
52+
EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eClear);
53+
EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eStore);
54+
55+
auto maybe_depth_stencil = builder.GetDepthStencil();
56+
ASSERT_TRUE(maybe_depth_stencil.has_value());
57+
if (!maybe_depth_stencil.has_value()) {
58+
return;
59+
}
60+
auto depth_stencil = maybe_depth_stencil.value();
61+
62+
EXPECT_EQ(depth_stencil.initialLayout, vk::ImageLayout::eUndefined);
63+
EXPECT_EQ(depth_stencil.finalLayout,
64+
vk::ImageLayout::eDepthStencilAttachmentOptimal);
65+
EXPECT_EQ(depth_stencil.loadOp, vk::AttachmentLoadOp::eDontCare);
66+
EXPECT_EQ(depth_stencil.storeOp, vk::AttachmentStoreOp::eDontCare);
67+
EXPECT_EQ(depth_stencil.stencilLoadOp, vk::AttachmentLoadOp::eDontCare);
68+
EXPECT_EQ(depth_stencil.stencilStoreOp, vk::AttachmentStoreOp::eDontCare);
69+
}
70+
71+
TEST(RenderPassBuilder, CreatesRenderPassWithOnlyStencil) {
72+
RenderPassBuilderVK builder = RenderPassBuilderVK();
73+
auto const context = MockVulkanContextBuilder().Build();
74+
75+
// Create a single color attachment with a transient depth stencil.
76+
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
77+
SampleCount::kCount1, LoadAction::kClear,
78+
StoreAction::kStore);
79+
builder.SetStencilAttachment(PixelFormat::kS8UInt, SampleCount::kCount1,
80+
LoadAction::kDontCare, StoreAction::kDontCare);
81+
82+
auto render_pass = builder.Build(context->GetDevice());
83+
84+
EXPECT_TRUE(!!render_pass);
85+
86+
auto maybe_depth_stencil = builder.GetDepthStencil();
87+
ASSERT_TRUE(maybe_depth_stencil.has_value());
88+
if (!maybe_depth_stencil.has_value()) {
89+
return;
90+
}
91+
auto depth_stencil = maybe_depth_stencil.value();
92+
93+
EXPECT_EQ(depth_stencil.initialLayout, vk::ImageLayout::eUndefined);
94+
EXPECT_EQ(depth_stencil.finalLayout,
95+
vk::ImageLayout::eDepthStencilAttachmentOptimal);
96+
EXPECT_EQ(depth_stencil.loadOp, vk::AttachmentLoadOp::eDontCare);
97+
EXPECT_EQ(depth_stencil.storeOp, vk::AttachmentStoreOp::eDontCare);
98+
EXPECT_EQ(depth_stencil.stencilLoadOp, vk::AttachmentLoadOp::eDontCare);
99+
EXPECT_EQ(depth_stencil.stencilStoreOp, vk::AttachmentStoreOp::eDontCare);
100+
}
101+
102+
} // namespace testing
103+
} // namespace impeller

‎impeller/renderer/backend/vulkan/render_pass_vk.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
113113
depth->load_action, //
114114
depth->store_action //
115115
);
116-
TextureVK::Cast(*depth->texture).SetLayout(barrier);
117116
} else if (auto stencil = render_target_.GetStencilAttachment();
118117
stencil.has_value()) {
119118
builder.SetStencilAttachment(
@@ -122,7 +121,6 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
122121
stencil->load_action, //
123122
stencil->store_action //
124123
);
125-
TextureVK::Cast(*stencil->texture).SetLayout(barrier);
126124
}
127125

128126
if (recycled_renderpass != nullptr) {

0 commit comments

Comments
 (0)