Portable Acceleration Structures #1955

fabian-lunarg · 2025-01-14T09:25:08Z

worked mostly on decode::VulkanAddressReplacer,
finally enabling portable replay of raytracing pipelines, when using RebindAllocator (-m rebind).

work on Acceleration Structures (AS):
-> tracking, meta-commands, copies, query-pool handling and rebuild
-> rebuild AS from original input-buffers when trimming.
-> check AS- and scratch-buffer sizes, replace resources with shadow objects where required

belongs to #1526

ci-tester-lunarg · 2025-01-14T09:25:11Z

CI gfxreconstruct build queued with queue ID 344454.

ci-tester-lunarg · 2025-01-14T09:30:01Z

CI gfxreconstruct build # 5784 running.

ci-tester-lunarg · 2025-01-14T09:38:07Z

CI gfxreconstruct build queued with queue ID 344470.

ci-tester-lunarg · 2025-01-14T09:42:12Z

CI gfxreconstruct build queued with queue ID 344472.

ci-tester-lunarg · 2025-01-14T09:59:05Z

CI gfxreconstruct build # 5785 running.

ci-tester-lunarg · 2025-01-14T10:32:20Z

CI gfxreconstruct build # 5785 failed.

ci-tester-lunarg · 2025-01-14T11:36:20Z

CI gfxreconstruct build queued with queue ID 344525.

ci-tester-lunarg · 2025-01-14T11:55:24Z

CI gfxreconstruct build # 5787 running.

ci-tester-lunarg · 2025-01-14T12:27:00Z

CI gfxreconstruct build # 5787 failed.

…d warn

…king here

- Add internal command/queue-assets, manage lifetime

ci-tester-lunarg · 2025-01-14T13:07:50Z

CI gfxreconstruct build queued with queue ID 344604.

ci-tester-lunarg · 2025-01-14T13:39:04Z

CI gfxreconstruct build # 5789 running.

- cleanup obsolete warnings

ci-tester-lunarg · 2025-01-14T14:08:48Z

CI gfxreconstruct build queued with queue ID 344693.

ci-tester-lunarg · 2025-01-14T14:14:00Z

CI gfxreconstruct build # 5790 running.

ci-tester-lunarg · 2025-01-14T14:46:56Z

CI gfxreconstruct build # 5790 failed.

framework/decode/vulkan_address_replacer.h

framework/decode/vulkan_address_replacer.cpp

antonio-lunarg · 2025-01-14T15:32:11Z

framework/decode/vulkan_address_replacer.cpp

+            else
+            {
+                GFXRECON_LOG_INFO_ONCE(
+                    "VulkanAddressReplacer::ProcessCmdTraceRays: missing buffer_info->replay_address, remap failed")


Do you specify the function because this macro is within a lambda, therefore __FUNCTION__ has weird value? :D

we might not need the log-error there at all. was mostly for debugging, also added a return value to check

purged the log-info

framework/decode/vulkan_address_replacer.cpp

antonio-lunarg · 2025-01-14T16:02:24Z

framework/decode/vulkan_address_replacer.cpp

 bool VulkanAddressReplacer::init_pipeline()
 {
-    if (pipeline_sbt_ != VK_NULL_HANDLE)
+    if (_pipeline_sbt != VK_NULL_HANDLE)
    {
        // assume already initialized


nit: I call these kinds of functions ensure_*. :)

framework/decode/vulkan_device_address_tracker.cpp

antonio-lunarg · 2025-01-14T16:46:08Z

framework/decode/vulkan_rebind_allocator.cpp

@@ -249,7 +249,13 @@ VkResult VulkanRebindAllocator::CreateBuffer(const VkBufferCreateInfo*    create

    if ((create_info != nullptr) && (buffer != nullptr) && (allocator_data != nullptr))
    {
-        result = functions_.create_buffer(device_, create_info, nullptr, buffer);
+        auto aligned_size = [](uint32_t size, uint32_t alignment) -> uint32_t {


There's a similar function in vulkan_address_replacer.cpp. How about moving this in a common header under the graphics namespace?

I chose util/alignment_utils.h and consolidated the alignment and divup helpers there

ci-tester-lunarg · 2025-01-14T17:57:53Z

CI gfxreconstruct build queued with queue ID 345010.

ci-tester-lunarg · 2025-01-14T18:00:04Z

CI gfxreconstruct build # 5796 running.

ci-tester-lunarg · 2025-01-14T18:30:01Z

CI gfxreconstruct build # 5796 failed.

MarkY-LunarG · 2025-01-14T16:55:05Z

framework/decode/vulkan_replay_consumer_base.cpp

+    VkWriteDescriptorSet* in_pDescriptorWrites = p_descriptor_writes->GetPointer();
+    VkCopyDescriptorSet*  in_pDescriptorCopies = p_pescriptor_copies->GetPointer();
+
+    {


Should we check that one or more of the extensions are enabled before we do this? I know right now it will only be for Ray Tracing, and eventually things like device generated commands, but should we not do this work if we don't have to?

good point, will check if we can avoid any unnecessary work

I found we can guard the loop like this:

// bail out if we're not tracking any shadow acceleration-structures if (shadow_as_map_.empty()) { return; }

this will avoid any iteration when the class is not actively replacing stuff

MarkY-LunarG · 2025-01-14T16:57:34Z

framework/decode/vulkan_replay_consumer_base.cpp

-    GFXRECON_ASSERT(allocator != nullptr);
-
-    if (allocator->SupportsOpaqueDeviceAddresses() || !loading_trim_state_)
+    if (loading_trim_state_)


Why only if loading trim state in all 3 of the following functions?

those 3 meta-commands all issue calls to restore state around acceleration-structures. rebuild, (compaction-)copies and querying the compacted size. that's only required for loading the trim state (afterwards the flow is through the Override*Acceleration*() overrides)

MarkY-LunarG · 2025-01-14T19:01:37Z

framework/decode/vulkan_decoder_base.cpp

@@ -634,7 +611,7 @@ void VulkanDecoderBase::DispatchVulkanAccelerationStructuresWritePropertiesMetaC

    std::size_t bytes_read = ValueDecoder::DecodeHandleIdValue(parameter_buffer, sizeof(format::HandleId), &device_id);
    bytes_read += ValueDecoder::DecodeEnumValue(parameter_buffer + bytes_read, sizeof(VkQueryType), &query_type);
-    bytes_read += ValueDecoder::DecodeHandleIdValue(
+    ValueDecoder::DecodeHandleIdValue(


Why not update the bytes read here? This seems wrong.

I can add it back, it's just nobody will check bytes_read after that and clang-tidy remarked something there. will check

clang-tidy's comment was: "The value is never used"

Hmm... Ok. I won't push for it, but I just worry about something being added after this being incorrect. Maybe just add (void)bytes_read; line to make clang-tidy happy?

I share your concern. if anyone appends stuff here I'd rather leave it and ignore clang-tidy (I think it's great, just not here) -> rolled back

MarkY-LunarG · 2025-01-14T19:14:58Z

framework/decode/vulkan_replay_consumer_base.cpp

-    if (omitted_pipeline_cache_data_)
+    // NOTE: as of early 2025, rayTracingPipelineShaderGroupHandleCaptureReplay is not widely supported.
+    // e.g. newest nvidia desktop-drivers do not support this feature
+    if (device_info->property_feature_info.feature_rayTracingPipelineShaderGroupHandleCaptureReplay)


Do we need to make sure this was enabled at vkCreateDevice time? Or does the device_info already do it?

device_info has it all. the feature might have been available during capture, but still missing during replay (e.g. seeing that for rayTracingPipelineShaderGroupHandleCaptureReplay between nvidia(no) <-> radv(yes))

for replay, we do checks in gfxrecon::graphics::VulkanDeviceUtil figuring out if to enable the feature or not.
the information for replay is pulled from VkPhysicalDeviceRayTracingPipelineFeaturesKHR via a vkGetPhysicalDeviceFeatures2 call. active when available both during capture & replay.
some of that logic needed to be touched in this PR (and it might not be the end of story yet).

…tiesKHR

ci-tester-lunarg · 2025-01-15T11:33:19Z

CI gfxreconstruct build queued with queue ID 345554.

ci-tester-lunarg · 2025-01-15T11:35:04Z

CI gfxreconstruct build # 5801 running.

ci-tester-lunarg · 2025-01-15T12:29:50Z

CI gfxreconstruct build # 5801 failed.

…ody else did

ci-tester-lunarg · 2025-01-16T08:18:15Z

CI gfxreconstruct build queued with queue ID 346759.

ci-tester-lunarg · 2025-01-16T08:20:01Z

CI gfxreconstruct build # 5816 running.

ci-tester-lunarg · 2025-01-16T08:47:45Z

CI gfxreconstruct build # 5816 failed.

ci-tester-lunarg · 2025-01-16T09:50:50Z

CI gfxreconstruct build queued with queue ID 346827.

ci-tester-lunarg · 2025-01-16T09:55:02Z

CI gfxreconstruct build # 5818 running.

ci-tester-lunarg · 2025-01-16T10:20:01Z

CI gfxreconstruct build # 5818 passed.

ci-tester-lunarg · 2025-01-16T11:26:56Z

CI gfxreconstruct build queued with queue ID 346892.

ci-tester-lunarg · 2025-01-16T11:30:01Z

CI gfxreconstruct build # 5819 running.

ci-tester-lunarg · 2025-01-16T11:56:31Z

CI gfxreconstruct build # 5819 passed.

…iss it during init

ci-tester-lunarg · 2025-01-16T14:05:38Z

CI gfxreconstruct build queued with queue ID 347107.

fabian-lunarg force-pushed the fabian_portable_acceleration_structures branch from 154df0e to c08661a Compare January 14, 2025 09:38

fabian-lunarg force-pushed the fabian_portable_acceleration_structures branch from c08661a to c810c23 Compare January 14, 2025 09:42

fabian-lunarg requested review from MarkY-LunarG, antonio-lunarg and nickdriscoll-lunarg January 14, 2025 11:36

fabian-lunarg added 15 commits January 14, 2025 14:05

Add utils handle->info-struct to AddressTracker, check build-sizes an…

8ddaca1

…d warn

cleanup refactor, stubs for replacement AS

b4667ad

shadow acceleration-structure creation

9684691

work on shadow-AS cleanup and buffer-alignment

e23d6ae

work on AccelerationStructure replacement. portable replay starts wor…

2c907c1

…king here

Add and integrate ProcessUpdateDescriptorSets

805405a

Process meta-commands (rebuild AS when trimming)

1e74976

Implement ProcessBuildVulkanAccelerationStructuresMetaCommand.

c48e000

- Add internal command/queue-assets, manage lifetime

Add missing tracking of AS scratch-buffers, needed for trimming

4490d47

Drop instance_buffers_data, cleanup, doc

e25bbf0

Seperate AS- and scratch shadow-objects, minor improvements

448f41e

Add VulkanCaptureManager::OverrideCmdCopyAccelerationStructureKHR

01d19b4

work on ProcessVulkanAccelerationStructuresWritePropertiesMetaCommand

1f24751

Overall progress and cleanup. some open things, but slowly finalizing

71260df

bring back opaque shadergroup-handles, you never know

2b1bafa

fabian-lunarg force-pushed the fabian_portable_acceleration_structures branch from c810c23 to 62d9748 Compare January 14, 2025 13:05

Guard address_replacer.ProcessCmdTraceRays(), only for '-m rebind'

e878682

- cleanup obsolete warnings

antonio-lunarg reviewed Jan 14, 2025

View reviewed changes

review comments, cleanup

5fc94a5

MarkY-LunarG reviewed Jan 14, 2025

View reviewed changes

Extend state-tracking for VkPhysicalDeviceAccelerationStructureProper…

58a5de0

…tiesKHR

Fetch VkPhysicalDeviceAccelerationStructurePropertiesKHR, in case nob…

97f2083

…ody else did

Consolidate alignment utils in dedicated header

ec2788e

fabian-lunarg force-pushed the fabian_portable_acceleration_structures branch from 8fd261b to ec2788e Compare January 16, 2025 11:26

Repeat call to SetRaytracingProperties, if necessary. we might have m…

f5d35ab

…iss it during init

Portable Acceleration Structures #1955

Are you sure you want to change the base?

Portable Acceleration Structures #1955

Conversation

fabian-lunarg commented Jan 14, 2025 • edited Loading

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

ci-tester-lunarg commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-lunarg Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-tester-lunarg commented Jan 15, 2025

ci-tester-lunarg commented Jan 15, 2025

ci-tester-lunarg commented Jan 15, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

ci-tester-lunarg commented Jan 16, 2025

fabian-lunarg commented Jan 14, 2025 •

edited

Loading

fabian-lunarg Jan 15, 2025 •

edited

Loading