Do not try to add transfer src/dst for all buffers
It seems that adding transfer src/dst usages to uniform buffers will
make Chrome less stable. Remove them and rely on memory snapshot to
snapshot buffer contents.
Bug: 342414479
Test: GfxstreamEnd2EndVkSnapshotPipelineTest
Change-Id: Ie9572a7e8f015378f89f106ec628647836513d8e
diff --git a/common/end2end/GfxstreamEnd2EndVkSnapshotBufferTests.cpp b/common/end2end/GfxstreamEnd2EndVkSnapshotBufferTests.cpp
index dbc043a..cc177f7 100644
--- a/common/end2end/GfxstreamEnd2EndVkSnapshotBufferTests.cpp
+++ b/common/end2end/GfxstreamEnd2EndVkSnapshotBufferTests.cpp
@@ -31,7 +31,7 @@
class GfxstreamEnd2EndVkSnapshotBufferTest : public GfxstreamEnd2EndTest {};
-TEST_P(GfxstreamEnd2EndVkSnapshotBufferTest, BufferContent) {
+TEST_P(GfxstreamEnd2EndVkSnapshotBufferTest, DeviceLocalBufferContent) {
static constexpr vkhpp::DeviceSize kSize = 256;
std::vector<uint8_t> srcBufferContent(kSize);
@@ -86,7 +86,9 @@
// Vertex buffer
const vkhpp::BufferCreateInfo vertexBufferCreateInfo = {
.size = static_cast<VkDeviceSize>(kSize),
- .usage = vkhpp::BufferUsageFlagBits::eVertexBuffer,
+ .usage = vkhpp::BufferUsageFlagBits::eVertexBuffer |
+ vkhpp::BufferUsageFlagBits::eTransferSrc |
+ vkhpp::BufferUsageFlagBits::eTransferDst,
.sharingMode = vkhpp::SharingMode::eExclusive,
};
auto vertexBuffer = device->createBufferUnique(vertexBufferCreateInfo).value;
@@ -95,9 +97,9 @@
vkhpp::MemoryRequirements vertexBufferMemoryRequirements{};
device->getBufferMemoryRequirements(*vertexBuffer, &vertexBufferMemoryRequirements);
- const auto vertexBufferMemoryType = utils::getMemoryType(
- physicalDevice, vertexBufferMemoryRequirements,
- vkhpp::MemoryPropertyFlagBits::eHostVisible | vkhpp::MemoryPropertyFlagBits::eHostCoherent);
+ const auto vertexBufferMemoryType =
+ utils::getMemoryType(physicalDevice, vertexBufferMemoryRequirements,
+ vkhpp::MemoryPropertyFlagBits::eDeviceLocal);
// Vertex memory
const vkhpp::MemoryAllocateInfo vertexBufferMemoryAllocateInfo = {
@@ -214,6 +216,69 @@
device->unmapMemory(*readbackBufferMemory);
}
+TEST_P(GfxstreamEnd2EndVkSnapshotBufferTest, HostVisibleBufferContent) {
+ static constexpr vkhpp::DeviceSize kSize = 256;
+
+ std::vector<uint8_t> srcBufferContent(kSize);
+ for (size_t i = 0; i < kSize; i++) {
+ srcBufferContent[i] = static_cast<uint8_t>(i & 0xff);
+ }
+ auto [instance, physicalDevice, device, queue, queueFamilyIndex] =
+ VK_ASSERT(SetUpTypicalVkTestEnvironment());
+
+ // Vertex buffer
+ const vkhpp::BufferCreateInfo uniformBufferCreateInfo = {
+ .size = static_cast<VkDeviceSize>(kSize),
+ .usage = vkhpp::BufferUsageFlagBits::eUniformBuffer,
+ .sharingMode = vkhpp::SharingMode::eExclusive,
+ };
+ auto uniformBuffer = device->createBufferUnique(uniformBufferCreateInfo).value;
+ ASSERT_THAT(uniformBuffer, IsValidHandle());
+
+ vkhpp::MemoryRequirements uniformBufferMemoryRequirements{};
+ device->getBufferMemoryRequirements(*uniformBuffer, &uniformBufferMemoryRequirements);
+
+ const auto uniformBufferMemoryType = utils::getMemoryType(
+ physicalDevice, uniformBufferMemoryRequirements,
+ vkhpp::MemoryPropertyFlagBits::eHostVisible | vkhpp::MemoryPropertyFlagBits::eHostCoherent);
+
+ // Vertex memory
+ const vkhpp::MemoryAllocateInfo uniformBufferMemoryAllocateInfo = {
+ .allocationSize = uniformBufferMemoryRequirements.size,
+ .memoryTypeIndex = uniformBufferMemoryType,
+ };
+ auto uniformBufferMemory = device->allocateMemoryUnique(uniformBufferMemoryAllocateInfo).value;
+ ASSERT_THAT(uniformBufferMemory, IsValidHandle());
+ ASSERT_THAT(device->bindBufferMemory(*uniformBuffer, *uniformBufferMemory, 0), IsVkSuccess());
+
+ // Fill memory content
+ void* mapped = nullptr;
+ auto mapResult =
+ device->mapMemory(*uniformBufferMemory, 0, VK_WHOLE_SIZE, vkhpp::MemoryMapFlags{}, &mapped);
+ ASSERT_THAT(mapResult, IsVkSuccess());
+ ASSERT_THAT(mapped, NotNull());
+
+ auto* bytes = reinterpret_cast<uint8_t*>(mapped);
+ std::memcpy(bytes, srcBufferContent.data(), kSize);
+
+ device->unmapMemory(*uniformBufferMemory);
+
+ // We need to unmap before snapshot due to limitations of the testing framework.
+ SnapshotSaveAndLoad();
+
+ // Verify content
+ mapResult =
+ device->mapMemory(*uniformBufferMemory, 0, VK_WHOLE_SIZE, vkhpp::MemoryMapFlags{}, &mapped);
+ ASSERT_THAT(mapResult, IsVkSuccess());
+ ASSERT_THAT(mapped, NotNull());
+ bytes = reinterpret_cast<uint8_t*>(mapped);
+
+ for (uint32_t i = 0; i < kSize; ++i) {
+ ASSERT_THAT(bytes[i], Eq(srcBufferContent[i]));
+ }
+ device->unmapMemory(*uniformBufferMemory);
+}
+
INSTANTIATE_TEST_CASE_P(GfxstreamEnd2EndTests, GfxstreamEnd2EndVkSnapshotBufferTest,
::testing::ValuesIn({
TestParams{
diff --git a/host/vulkan/VkDecoderGlobalState.cpp b/host/vulkan/VkDecoderGlobalState.cpp
index 5298e60..6d90792 100644
--- a/host/vulkan/VkDecoderGlobalState.cpp
+++ b/host/vulkan/VkDecoderGlobalState.cpp
@@ -1906,8 +1906,19 @@
VkBufferCreateInfo localCreateInfo;
if (snapshotsEnabled()) {
localCreateInfo = *pCreateInfo;
- localCreateInfo.usage |=
- VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT;
+ // Add transfer src bit for potential device local memories.
+ //
+ // There are 3 ways to populate buffer content:
+ // a) use host coherent memory and memory mapping;
+ // b) use transfer_dst and vkcmdcopy* (for device local memories);
+ // c) use storage and compute shaders.
+ //
+ // (a) is covered by memory snapshot. (b) requires an extra vkCmdCopyBuffer
+ // command on snapshot, thuse we need to add transfer_src for (b) so that
+ // they could be loaded back on snapshot save. (c) is still future work.
+ if (localCreateInfo.usage & VK_BUFFER_USAGE_TRANSFER_DST_BIT) {
+ localCreateInfo.usage |= VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
+ }
pCreateInfo = &localCreateInfo;
}
@@ -1917,6 +1928,7 @@
std::lock_guard<std::recursive_mutex> lock(mLock);
auto& bufInfo = mBufferInfo[*pBuffer];
bufInfo.device = device;
+ bufInfo.usage = pCreateInfo->usage;
bufInfo.size = pCreateInfo->size;
*pBuffer = new_boxed_non_dispatchable_VkBuffer(*pBuffer);
}
diff --git a/host/vulkan/VkDecoderInternalStructs.h b/host/vulkan/VkDecoderInternalStructs.h
index eab6656..dc352de 100644
--- a/host/vulkan/VkDecoderInternalStructs.h
+++ b/host/vulkan/VkDecoderInternalStructs.h
@@ -228,6 +228,7 @@
struct BufferInfo {
VkDevice device;
+ VkBufferUsageFlags usage;
VkDeviceMemory memory = 0;
VkDeviceSize memoryOffset = 0;
VkDeviceSize size;
diff --git a/host/vulkan/VkDecoderSnapshotUtils.cpp b/host/vulkan/VkDecoderSnapshotUtils.cpp
index d8510ed..2359fe8 100644
--- a/host/vulkan/VkDecoderSnapshotUtils.cpp
+++ b/host/vulkan/VkDecoderSnapshotUtils.cpp
@@ -475,6 +475,11 @@
void saveBufferContent(android::base::Stream* stream, StateBlock* stateBlock, VkBuffer buffer,
const BufferInfo* bufferInfo) {
+ VkBufferUsageFlags requiredUsages =
+ VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT;
+ if ((bufferInfo->usage & requiredUsages) != requiredUsages) {
+ return;
+ }
VulkanDispatch* dispatch = stateBlock->deviceDispatch;
VkCommandBufferAllocateInfo allocInfo{
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO,
@@ -573,6 +578,11 @@
void loadBufferContent(android::base::Stream* stream, StateBlock* stateBlock, VkBuffer buffer,
const BufferInfo* bufferInfo) {
+ VkBufferUsageFlags requiredUsages =
+ VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT;
+ if ((bufferInfo->usage & requiredUsages) != requiredUsages) {
+ return;
+ }
VulkanDispatch* dispatch = stateBlock->deviceDispatch;
VkCommandBufferAllocateInfo allocInfo{
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO,
