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

[Impeller] cleanup and test vk image usage flags. #51301

Merged
merged 4 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 5 additions & 19 deletions impeller/renderer/backend/vulkan/allocator_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ int32_t AllocatorVK::FindMemoryTypeIndex(
return type_index;
}

static constexpr vk::ImageUsageFlags ToVKImageUsageFlags(
vk::ImageUsageFlags AllocatorVK::ToVKImageUsageFlags(
PixelFormat format,
TextureUsageMask usage,
StorageMode mode,
Expand All @@ -219,30 +219,16 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags(
vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment;
} else {
vk_usage |= vk::ImageUsageFlagBits::eColorAttachment;
vk_usage |= vk::ImageUsageFlagBits::eInputAttachment;
}
vk_usage |= vk::ImageUsageFlagBits::eInputAttachment;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't attach depth/stencil, only the color textures (actually, only the MSAA one, maybe I can fix that too).

}

if (usage & TextureUsage::kShaderRead) {
vk_usage |= vk::ImageUsageFlagBits::eSampled;
// Device transient images can only be used as attachments. The caller
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed overly defensive code. Since we have validation layers we should let this cause problems and have validation fix them (or error here), but not remove the request flags to fix validation

// specified incorrect usage flags and is attempting to read a device
// transient image in a shader. Unset the transient attachment flag. See:
// https://github.com/flutter/flutter/issues/121633
if (mode == StorageMode::kDeviceTransient) {
vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment;
}
}

if (usage & TextureUsage::kShaderWrite) {
vk_usage |= vk::ImageUsageFlagBits::eStorage;
// Device transient images can only be used as attachments. The caller
// specified incorrect usage flags and is attempting to read a device
// transient image in a shader. Unset the transient attachment flag. See:
// https://github.com/flutter/flutter/issues/121633
if (mode == StorageMode::kDeviceTransient) {
vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment;
}
}

if (mode != StorageMode::kDeviceTransient) {
Expand Down Expand Up @@ -314,9 +300,9 @@ class AllocatedTextureSourceVK final : public TextureSourceVK {
image_info.arrayLayers = ToArrayLayerCount(desc.type);
image_info.tiling = vk::ImageTiling::eOptimal;
image_info.initialLayout = vk::ImageLayout::eUndefined;
image_info.usage =
ToVKImageUsageFlags(desc.format, desc.usage, desc.storage_mode,
supports_memoryless_textures);
image_info.usage = AllocatorVK::ToVKImageUsageFlags(
desc.format, desc.usage, desc.storage_mode,
supports_memoryless_textures);
image_info.sharingMode = vk::SharingMode::eExclusive;

VmaAllocationCreateInfo alloc_nfo = {};
Expand Down
7 changes: 7 additions & 0 deletions impeller/renderer/backend/vulkan/allocator_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ class AllocatorVK final : public Allocator {
uint32_t memory_type_bits_requirement,
vk::PhysicalDeviceMemoryProperties& memory_properties);

// Visible for testing.
static vk::ImageUsageFlags ToVKImageUsageFlags(
PixelFormat format,
TextureUsageMask usage,
StorageMode mode,
bool supports_memoryless_textures);

private:
friend class ContextVK;

Expand Down
19 changes: 19 additions & 0 deletions impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,25 @@
namespace impeller {
namespace testing {

TEST(AllocatorVKTest, ToVKImageUsageFlags) {
EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags(
PixelFormat::kR8G8B8A8UNormInt,
static_cast<TextureUsageMask>(TextureUsage::kRenderTarget),
StorageMode::kDeviceTransient,
/*supports_memoryless_textures=*/true),
vk::ImageUsageFlagBits::eInputAttachment |
vk::ImageUsageFlagBits::eColorAttachment |
vk::ImageUsageFlagBits::eTransientAttachment);

EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags(
PixelFormat::kD24UnormS8Uint,
static_cast<TextureUsageMask>(TextureUsage::kRenderTarget),
StorageMode::kDeviceTransient,
/*supports_memoryless_textures=*/true),
vk::ImageUsageFlagBits::eDepthStencilAttachment |
vk::ImageUsageFlagBits::eTransientAttachment);
}

TEST(AllocatorVKTest, MemoryTypeSelectionSingleHeap) {
vk::PhysicalDeviceMemoryProperties properties;
properties.memoryTypeCount = 1;
Expand Down