[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 https://github.com/flutter/flutter/issues/157557
This commit is contained in:
Jonah Williams 2024-10-28 13:10:10 -07:00 committed by GitHub
parent 82c9184c2c
commit e357d1899d
6 changed files with 63 additions and 17 deletions

View File

@ -9,6 +9,7 @@
#include <memory>
#include <vector>
#include "fml/mapping.h"
#include "impeller/core/buffer_view.h"
#include "impeller/core/host_buffer.h"
#include "impeller/core/platform.h"
@ -78,8 +79,8 @@ bool ImGui_ImplImpeller_Init(
int width, height;
io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);
auto texture_descriptor = impeller::TextureDescriptor{};
texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible;
impeller::TextureDescriptor texture_descriptor;
texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate;
texture_descriptor.format = impeller::PixelFormat::kR8G8B8A8UNormInt;
texture_descriptor.size = {width, height};
texture_descriptor.mip_count = 1u;
@ -90,8 +91,20 @@ bool ImGui_ImplImpeller_Init(
"Could not allocate ImGui font texture.");
bd->font_texture->SetLabel("ImGui Font Texture");
[[maybe_unused]] bool uploaded = bd->font_texture->SetContents(
pixels, texture_descriptor.GetByteSizeOfBaseMipLevel());
auto command_buffer = context->CreateCommandBuffer();
auto blit_pass = command_buffer->CreateBlitPass();
auto mapping = std::make_shared<fml::NonOwnedMapping>(
reinterpret_cast<const uint8_t*>(pixels),
texture_descriptor.GetByteSizeOfBaseMipLevel());
auto device_buffer =
context->GetResourceAllocator()->CreateBufferWithCopy(*mapping);
blit_pass->AddCopy(impeller::DeviceBuffer::AsBufferView(device_buffer),
bd->font_texture);
blit_pass->EncodeCommands(context->GetResourceAllocator());
[[maybe_unused]] bool uploaded =
context->GetCommandQueue()->Submit({command_buffer}).ok();
IM_ASSERT(uploaded &&
"Could not upload ImGui font texture to device memory.");
}

View File

@ -388,8 +388,8 @@ static std::shared_ptr<Texture> CreateTextureForDecompressedImage(
const std::shared_ptr<Context>& context,
DecompressedImage& decompressed_image,
bool enable_mipmapping) {
auto texture_descriptor = TextureDescriptor{};
texture_descriptor.storage_mode = StorageMode::kHostVisible;
TextureDescriptor texture_descriptor;
texture_descriptor.storage_mode = StorageMode::kDevicePrivate;
texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt;
texture_descriptor.size = decompressed_image.GetSize();
texture_descriptor.mip_count =
@ -463,8 +463,8 @@ std::shared_ptr<Texture> Playground::CreateTextureCubeForFixture(
images[i] = image.value();
}
auto texture_descriptor = TextureDescriptor{};
texture_descriptor.storage_mode = StorageMode::kHostVisible;
TextureDescriptor texture_descriptor;
texture_descriptor.storage_mode = StorageMode::kDevicePrivate;
texture_descriptor.type = TextureType::kTextureCube;
texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt;
texture_descriptor.size = images[0].GetSize();

View File

@ -6,7 +6,9 @@
#include <vector>
#include "impeller/core/formats.h"
#include "impeller/renderer/backend/vulkan/formats_vk.h"
#include "vulkan/vulkan_enums.hpp"
namespace impeller {
@ -31,7 +33,8 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetColorAttachment(
PixelFormat format,
SampleCount sample_count,
LoadAction load_action,
StoreAction store_action) {
StoreAction store_action,
vk::ImageLayout current_layout) {
vk::AttachmentDescription desc;
desc.format = ToVKImageFormat(format);
desc.samples = ToVKSampleCount(sample_count);
@ -39,7 +42,11 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetColorAttachment(
desc.storeOp = ToVKAttachmentStoreOp(store_action, false);
desc.stencilLoadOp = vk::AttachmentLoadOp::eDontCare;
desc.stencilStoreOp = vk::AttachmentStoreOp::eDontCare;
desc.initialLayout = vk::ImageLayout::eUndefined;
if (load_action == LoadAction::kLoad) {
desc.initialLayout = current_layout;
} else {
desc.initialLayout = vk::ImageLayout::eUndefined;
}
desc.finalLayout = vk::ImageLayout::eGeneral;
colors_[index] = desc;

View File

@ -24,11 +24,13 @@ class RenderPassBuilderVK {
RenderPassBuilderVK& operator=(const RenderPassBuilderVK&) = delete;
RenderPassBuilderVK& SetColorAttachment(size_t index,
PixelFormat format,
SampleCount sample_count,
LoadAction load_action,
StoreAction store_action);
RenderPassBuilderVK& SetColorAttachment(
size_t index,
PixelFormat format,
SampleCount sample_count,
LoadAction load_action,
StoreAction store_action,
vk::ImageLayout current_layout = vk::ImageLayout::eUndefined);
RenderPassBuilderVK& SetDepthStencilAttachment(PixelFormat format,
SampleCount sample_count,

View File

@ -27,6 +27,29 @@ TEST(RenderPassBuilder, CreatesRenderPassWithNoDepthStencil) {
EXPECT_FALSE(builder.GetDepthStencil().has_value());
}
TEST(RenderPassBuilder, RenderPassWithLoadOpUsesCurrentLayout) {
RenderPassBuilderVK builder = RenderPassBuilderVK();
auto const context = MockVulkanContextBuilder().Build();
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
SampleCount::kCount1, LoadAction::kLoad,
StoreAction::kStore,
vk::ImageLayout::eColorAttachmentOptimal);
auto render_pass = builder.Build(context->GetDevice());
EXPECT_TRUE(!!render_pass);
auto maybe_color = builder.GetColorAttachments().find(0u);
ASSERT_NE(maybe_color, builder.GetColorAttachments().end());
auto color = maybe_color->second;
EXPECT_EQ(color.initialLayout, vk::ImageLayout::eColorAttachmentOptimal);
EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral);
EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eLoad);
EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eStore);
}
TEST(RenderPassBuilder, CreatesRenderPassWithCombinedDepthStencil) {
RenderPassBuilderVK builder = RenderPassBuilderVK();
auto const context = MockVulkanContextBuilder().Build();
@ -34,7 +57,7 @@ TEST(RenderPassBuilder, CreatesRenderPassWithCombinedDepthStencil) {
// Create a single color attachment with a transient depth stencil.
builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
SampleCount::kCount1, LoadAction::kClear,
StoreAction::kStore);
StoreAction::kStore, vk::ImageLayout::eGeneral);
builder.SetDepthStencilAttachment(PixelFormat::kD24UnormS8Uint,
SampleCount::kCount1, LoadAction::kDontCare,
StoreAction::kDontCare);

View File

@ -89,7 +89,8 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
color.texture->GetTextureDescriptor().format, //
color.texture->GetTextureDescriptor().sample_count, //
color.load_action, //
color.store_action //
color.store_action, //
TextureVK::Cast(*color.texture).GetLayout() //
);
TextureVK::Cast(*color.texture)
.SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral);