Skip to content

Commit e357d18

Browse files
author
Jonah Williams
authored
[Impeller] fix initial layout for loadOp load and incorrect usage of host visible textures. (flutter/engine#56148)
Both changes were required to get playground tests validation free with moltenvk. becuase an initial state of undefined means "I don't care what was in this texture before" but that doesn't make sense if we're setting "kLoad" since that explicitly asks vulkan to load the previous contents. Fixes flutter#157557
1 parent 82c9184 commit e357d18

File tree

6 files changed

+63
-17
lines changed

6 files changed

+63
-17
lines changed

engine/src/flutter/impeller/playground/imgui/imgui_impl_impeller.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <memory>
1010
#include <vector>
1111

12+
#include "fml/mapping.h"
1213
#include "impeller/core/buffer_view.h"
1314
#include "impeller/core/host_buffer.h"
1415
#include "impeller/core/platform.h"
@@ -78,8 +79,8 @@ bool ImGui_ImplImpeller_Init(
7879
int width, height;
7980
io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);
8081

81-
auto texture_descriptor = impeller::TextureDescriptor{};
82-
texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible;
82+
impeller::TextureDescriptor texture_descriptor;
83+
texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate;
8384
texture_descriptor.format = impeller::PixelFormat::kR8G8B8A8UNormInt;
8485
texture_descriptor.size = {width, height};
8586
texture_descriptor.mip_count = 1u;
@@ -90,8 +91,20 @@ bool ImGui_ImplImpeller_Init(
9091
"Could not allocate ImGui font texture.");
9192
bd->font_texture->SetLabel("ImGui Font Texture");
9293

93-
[[maybe_unused]] bool uploaded = bd->font_texture->SetContents(
94-
pixels, texture_descriptor.GetByteSizeOfBaseMipLevel());
94+
auto command_buffer = context->CreateCommandBuffer();
95+
auto blit_pass = command_buffer->CreateBlitPass();
96+
auto mapping = std::make_shared<fml::NonOwnedMapping>(
97+
reinterpret_cast<const uint8_t*>(pixels),
98+
texture_descriptor.GetByteSizeOfBaseMipLevel());
99+
auto device_buffer =
100+
context->GetResourceAllocator()->CreateBufferWithCopy(*mapping);
101+
102+
blit_pass->AddCopy(impeller::DeviceBuffer::AsBufferView(device_buffer),
103+
bd->font_texture);
104+
blit_pass->EncodeCommands(context->GetResourceAllocator());
105+
106+
[[maybe_unused]] bool uploaded =
107+
context->GetCommandQueue()->Submit({command_buffer}).ok();
95108
IM_ASSERT(uploaded &&
96109
"Could not upload ImGui font texture to device memory.");
97110
}

engine/src/flutter/impeller/playground/playground.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,8 @@ static std::shared_ptr<Texture> CreateTextureForDecompressedImage(
388388
const std::shared_ptr<Context>& context,
389389
DecompressedImage& decompressed_image,
390390
bool enable_mipmapping) {
391-
auto texture_descriptor = TextureDescriptor{};
392-
texture_descriptor.storage_mode = StorageMode::kHostVisible;
391+
TextureDescriptor texture_descriptor;
392+
texture_descriptor.storage_mode = StorageMode::kDevicePrivate;
393393
texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt;
394394
texture_descriptor.size = decompressed_image.GetSize();
395395
texture_descriptor.mip_count =
@@ -463,8 +463,8 @@ std::shared_ptr<Texture> Playground::CreateTextureCubeForFixture(
463463
images[i] = image.value();
464464
}
465465

466-
auto texture_descriptor = TextureDescriptor{};
467-
texture_descriptor.storage_mode = StorageMode::kHostVisible;
466+
TextureDescriptor texture_descriptor;
467+
texture_descriptor.storage_mode = StorageMode::kDevicePrivate;
468468
texture_descriptor.type = TextureType::kTextureCube;
469469
texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt;
470470
texture_descriptor.size = images[0].GetSize();

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
#include <vector>
88

9+
#include "impeller/core/formats.h"
910
#include "impeller/renderer/backend/vulkan/formats_vk.h"
11+
#include "vulkan/vulkan_enums.hpp"
1012

1113
namespace impeller {
1214

@@ -31,15 +33,20 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetColorAttachment(
3133
PixelFormat format,
3234
SampleCount sample_count,
3335
LoadAction load_action,
34-
StoreAction store_action) {
36+
StoreAction store_action,
37+
vk::ImageLayout current_layout) {
3538
vk::AttachmentDescription desc;
3639
desc.format = ToVKImageFormat(format);
3740
desc.samples = ToVKSampleCount(sample_count);
3841
desc.loadOp = ToVKAttachmentLoadOp(load_action);
3942
desc.storeOp = ToVKAttachmentStoreOp(store_action, false);
4043
desc.stencilLoadOp = vk::AttachmentLoadOp::eDontCare;
4144
desc.stencilStoreOp = vk::AttachmentStoreOp::eDontCare;
42-
desc.initialLayout = vk::ImageLayout::eUndefined;
45+
if (load_action == LoadAction::kLoad) {
46+
desc.initialLayout = current_layout;
47+
} else {
48+
desc.initialLayout = vk::ImageLayout::eUndefined;
49+
}
4350
desc.finalLayout = vk::ImageLayout::eGeneral;
4451
colors_[index] = desc;
4552

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ class RenderPassBuilderVK {
2424

2525
RenderPassBuilderVK& operator=(const RenderPassBuilderVK&) = delete;
2626

27-
RenderPassBuilderVK& SetColorAttachment(size_t index,
28-
PixelFormat format,
29-
SampleCount sample_count,
30-
LoadAction load_action,
31-
StoreAction store_action);
27+
RenderPassBuilderVK& SetColorAttachment(
28+
size_t index,
29+
PixelFormat format,
30+
SampleCount sample_count,
31+
LoadAction load_action,
32+
StoreAction store_action,
33+
vk::ImageLayout current_layout = vk::ImageLayout::eUndefined);
3234

3335
RenderPassBuilderVK& SetDepthStencilAttachment(PixelFormat format,
3436
SampleCount sample_count,

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,37 @@ TEST(RenderPassBuilder, CreatesRenderPassWithNoDepthStencil) {
2727
EXPECT_FALSE(builder.GetDepthStencil().has_value());
2828
}
2929

30+
TEST(RenderPassBuilder, RenderPassWithLoadOpUsesCurrentLayout) {
31+
RenderPassBuilderVK builder = RenderPassBuilderVK();
32+
auto const context = MockVulkanContextBuilder().Build();
33+
34+
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
35+
SampleCount::kCount1, LoadAction::kLoad,
36+
StoreAction::kStore,
37+
vk::ImageLayout::eColorAttachmentOptimal);
38+
39+
auto render_pass = builder.Build(context->GetDevice());
40+
41+
EXPECT_TRUE(!!render_pass);
42+
43+
auto maybe_color = builder.GetColorAttachments().find(0u);
44+
ASSERT_NE(maybe_color, builder.GetColorAttachments().end());
45+
auto color = maybe_color->second;
46+
47+
EXPECT_EQ(color.initialLayout, vk::ImageLayout::eColorAttachmentOptimal);
48+
EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral);
49+
EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eLoad);
50+
EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eStore);
51+
}
52+
3053
TEST(RenderPassBuilder, CreatesRenderPassWithCombinedDepthStencil) {
3154
RenderPassBuilderVK builder = RenderPassBuilderVK();
3255
auto const context = MockVulkanContextBuilder().Build();
3356

3457
// Create a single color attachment with a transient depth stencil.
3558
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
3659
SampleCount::kCount1, LoadAction::kClear,
37-
StoreAction::kStore);
60+
StoreAction::kStore, vk::ImageLayout::eGeneral);
3861
builder.SetDepthStencilAttachment(PixelFormat::kD24UnormS8Uint,
3962
SampleCount::kCount1, LoadAction::kDontCare,
4063
StoreAction::kDontCare);

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
8989
color.texture->GetTextureDescriptor().format, //
9090
color.texture->GetTextureDescriptor().sample_count, //
9191
color.load_action, //
92-
color.store_action //
92+
color.store_action, //
93+
TextureVK::Cast(*color.texture).GetLayout() //
9394
);
9495
TextureVK::Cast(*color.texture)
9596
.SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral);

0 commit comments

Comments
 (0)