Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sparse image and buffer support #1877

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

bradgrantham-lunarg
Copy link
Contributor

The current trim handling for buffers and images does not support sparse buffers or sparse images. It only allows one image or buffer binding to a single memory range of a single memory object. As a result, it cannot manage sparse resources, which has led to missing asset issues in trim trace for some titles that utilize sparse resources. This commit introduces support for sparse buffers and sparse images (using opaque memory binding only), effectively resolving the missing asset issue.

Originally by Ming Zheng [email protected]

The current trim handling for buffers and images does not support sparse
buffers or sparse images. It only allows one image or buffer binding to
a single memory range of a single memory object. As a result, it cannot
manage sparse resources, which has led to missing asset issues in trim
trace for some titles that utilize sparse resources. This commit
introduces support for sparse buffers and sparse images (using opaque
memory binding only), effectively resolving the missing asset issue.

Originally by Ming Zheng <[email protected]>
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 305787.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5350 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5350 passed.

{
if (IsCaptureModeTrack() && (result == VK_SUCCESS) && (pCreateInfo != nullptr))
{
assert(state_tracker_ != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

assert -> GFXRECON_ASSERT

@@ -816,6 +920,50 @@ class VulkanCaptureManager : public ApiCaptureManager
}
}

void PostProcess_vkCreateBuffer(VkResult result,
VkDevice device,
const VkBufferCreateInfo* pCreateInfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want snake case here?

{
if (IsCaptureModeTrack() && (result == VK_SUCCESS) && (pCreateInfo != nullptr))
{
assert(state_tracker_ != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -1380,6 +1528,7 @@ class VulkanCaptureManager : public ApiCaptureManager
std::unique_ptr<VulkanStateTracker> state_tracker_;
HardwareBufferMap hardware_buffers_;
std::mutex deferred_operation_mutex;
std::mutex sparse_resource_mutex;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving the comment in PostProcess_vkQueueBindSparse here?

// In default mode, the capture manager uses a shared mutex to capture every API function. As a result,
// multiple threads may access the sparse resource maps concurrently. Therefore, we use a dedicated mutex
// for write access to these maps.

@@ -1361,7 +1361,7 @@ void VulkanStateWriter::ProcessBufferMemory(const vulkan_wrappers::DeviceWrapper
const uint8_t* bytes = nullptr;
std::vector<uint8_t> data;

assert((buffer_wrapper != nullptr) && (memory_wrapper != nullptr));
assert(buffer_wrapper != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, and a few lines down

const vulkan_wrappers::DeviceWrapper* device_wrapper = wrapper->bind_device;
const VulkanDeviceTable* device_table = &device_wrapper->layer_table;

assert((device_wrapper != nullptr) && (device_table != nullptr));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

encoder_.EncodeEnumValue(VK_SUCCESS);
const vulkan_wrappers::DeviceWrapper* device_wrapper = wrapper->bind_device;
const VulkanDeviceTable* device_table = &device_wrapper->layer_table;
assert((device_wrapper != nullptr) && (device_table != nullptr));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants