From 127d3d1b3290434abf96d3e6a9c0fe1893fa6566 Mon Sep 17 00:00:00 2001 From: JerryAMD Date: Wed, 8 Jan 2025 13:59:56 -0500 Subject: [PATCH 1/2] CommandList Reset at Invalid CommandAllocator The content of the commandlist should be discarded when detect an invalid commandallocator --- .../encode/custom_dx12_wrapper_commands.h | 13 ++++++-- framework/encode/d3d12_capture_manager.cpp | 31 +++++++++++++------ framework/encode/d3d12_capture_manager.h | 7 ++++- framework/encode/dx12_object_wrapper_info.h | 14 ++++----- framework/encode/dx12_state_tracker.cpp | 11 ++++++- framework/encode/dx12_state_tracker.h | 6 +++- framework/encode/dx12_state_writer.cpp | 16 ++++++---- 7 files changed, 70 insertions(+), 28 deletions(-) diff --git a/framework/encode/custom_dx12_wrapper_commands.h b/framework/encode/custom_dx12_wrapper_commands.h index 6ff13b3693..fe8a296f17 100644 --- a/framework/encode/custom_dx12_wrapper_commands.h +++ b/framework/encode/custom_dx12_wrapper_commands.h @@ -1,6 +1,6 @@ /* ** Copyright (c) 2021 LunarG, Inc. -** Copyright (c) 2021-2023 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2021-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -492,6 +492,16 @@ struct CustomWrapperPostCall +struct CustomWrapperPostCall +{ + template + static void Dispatch(D3D12CaptureManager* manager, Args... args) + { + manager->PostProcess_ID3D12GraphicsCommandList_Reset(args...); + } +}; + template <> struct CustomWrapperPostCall { @@ -814,7 +824,6 @@ struct CustomWrapperPostCallTrackCommandList_Reset(list_wrapper, pAllocator, pInitialState); + } +} + void D3D12CaptureManager::PostProcess_ID3D12GraphicsCommandList4_BuildRaytracingAccelerationStructure( ID3D12GraphicsCommandList4_Wrapper* list_wrapper, const D3D12_BUILD_RAYTRACING_ACCELERATION_STRUCTURE_DESC* desc, @@ -3368,11 +3379,11 @@ bool D3D12CaptureManager::TrimDrawCalls_ID3D12CommandQueue_ExecuteCommandLists( if (target_info->find_target_draw_call_count != (trim_draw_calls.draw_call_indices.last - trim_draw_calls.draw_call_indices.first + 1)) { - GFXRECON_LOG_WARNING( - "CAPTURE_DRAW_CALLS didn't find the enough draw call count(%d). The indices(%d-%d) might be out of range.", - target_info->find_target_draw_call_count, - trim_draw_calls.draw_call_indices.first, - trim_draw_calls.draw_call_indices.last); + GFXRECON_LOG_WARNING("CAPTURE_DRAW_CALLS didn't find the enough draw call count(%d). The indices(%d-%d) " + "might be out of range.", + target_info->find_target_draw_call_count, + trim_draw_calls.draw_call_indices.first, + trim_draw_calls.draw_call_indices.last); } if (target_info->target_bundle_commandlist_info) @@ -3678,10 +3689,10 @@ D3D12CaptureManager::GetCommandListsForTrimDrawCalls(ID3D12CommandList_Wrapper* case graphics::dx12::Dx12DumpResourcePos::kDrawCall: if (trim_draw_calls.draw_call_indices.first != trim_draw_calls.draw_call_indices.last) { - GFXRECON_LOG_FATAL( - "The target draw call is a ExecuteBundle. The draw call indices must be not a range(%d-%d).", - trim_draw_calls.draw_call_indices.first, - trim_draw_calls.draw_call_indices.last); + GFXRECON_LOG_FATAL("The target draw call is a ExecuteBundle. The draw call indices must be not " + "a range(%d-%d).", + trim_draw_calls.draw_call_indices.first, + trim_draw_calls.draw_call_indices.last); GFXRECON_ASSERT(trim_draw_calls.draw_call_indices.first == trim_draw_calls.draw_call_indices.last); } diff --git a/framework/encode/d3d12_capture_manager.h b/framework/encode/d3d12_capture_manager.h index 1cd7663f71..afe7178841 100644 --- a/framework/encode/d3d12_capture_manager.h +++ b/framework/encode/d3d12_capture_manager.h @@ -1,7 +1,7 @@ /* ** Copyright (c) 2018-2020 Valve Corporation ** Copyright (c) 2018-2021 LunarG, Inc. -** Copyright (c) 2019-2024 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2019-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -497,6 +497,11 @@ class D3D12CaptureManager : public ApiCaptureManager UINT num_barriers, const D3D12_RESOURCE_BARRIER* barriers); + void PostProcess_ID3D12GraphicsCommandList_Reset(ID3D12CommandList_Wrapper* list_wrapper, + HRESULT result, + ID3D12CommandAllocator* pAllocator, + ID3D12PipelineState* pInitialState); + void PostProcess_ID3D12GraphicsCommandList4_BuildRaytracingAccelerationStructure( ID3D12GraphicsCommandList4_Wrapper* list_wrapper, const D3D12_BUILD_RAYTRACING_ACCELERATION_STRUCTURE_DESC* desc, diff --git a/framework/encode/dx12_object_wrapper_info.h b/framework/encode/dx12_object_wrapper_info.h index a6139e8e7d..4f0496bfa5 100644 --- a/framework/encode/dx12_object_wrapper_info.h +++ b/framework/encode/dx12_object_wrapper_info.h @@ -1,6 +1,6 @@ /* ** Copyright (c) 2021 LunarG, Inc. -** Copyright (c) 2022-2024 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2022-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -309,8 +309,7 @@ struct AccelerationStructureBuildTrackingObjects graphics::dx12::ID3D12ResourceComPtr _resource, graphics::dx12::ID3D12CommandAllocatorComPtr _post_build_copy_cmd_allocator, graphics::dx12::ID3D12GraphicsCommandList4ComPtr _post_build_copy_cmd_list) : - resource(_resource), - post_build_copy_cmd_allocator(_post_build_copy_cmd_allocator), + resource(_resource), post_build_copy_cmd_allocator(_post_build_copy_cmd_allocator), post_build_copy_cmd_list(_post_build_copy_cmd_list) {} @@ -477,11 +476,12 @@ struct ID3D12CommandListInfo : public DxWrapperInfo D3D12_COMMAND_LIST_TYPE command_list_type{}; // Track command list dependencies. - format::HandleId create_command_allocator_id{ format::kNullHandleId }; + format::HandleId create_command_allocator_id{ format::kNullHandleId }; std::shared_ptr create_command_allocator_info; - std::unordered_set command_objects[D3D12GraphicsCommandObjectType::NumObjectTypes]; - std::unordered_set command_cpu_descriptor_handles; - std::unordered_set command_gpu_virtual_addresses; + format::HandleId reset_command_allocator_id{ format::kNullHandleId }; + std::unordered_set command_objects[D3D12GraphicsCommandObjectType::NumObjectTypes]; + std::unordered_set command_cpu_descriptor_handles; + std::unordered_set command_gpu_virtual_addresses; // Record for future. It's not used for now. std::unordered_set command_gpu_descriptor_handles; diff --git a/framework/encode/dx12_state_tracker.cpp b/framework/encode/dx12_state_tracker.cpp index 3e8220cba7..a6a6d19e2e 100644 --- a/framework/encode/dx12_state_tracker.cpp +++ b/framework/encode/dx12_state_tracker.cpp @@ -1,6 +1,6 @@ /* ** Copyright (c) 2021 LunarG, Inc. -** Copyright (c) 2022-2024 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2022-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -200,6 +200,14 @@ void Dx12StateTracker::TrackResourceBarriers(ID3D12CommandList_Wrapper* list_ } } +void Dx12StateTracker::TrackCommandList_Reset(ID3D12CommandList_Wrapper* list_wrapper, + ID3D12CommandAllocator* pAllocator, + ID3D12PipelineState* pInitialState) +{ + auto allocator = reinterpret_cast(pAllocator); + list_wrapper->GetObjectInfo()->reset_command_allocator_id = allocator->GetCaptureId(); +} + void Dx12StateTracker::TrackExecuteCommandLists(ID3D12CommandQueue_Wrapper* queue_wrapper, UINT num_lists, ID3D12CommandList* const* lists) @@ -457,6 +465,7 @@ void Dx12StateTracker::TrackCommandListCreation(ID3D12CommandList_Wrapper* list_ { auto cmd_alloc_wrapper = reinterpret_cast(pCommandAllocator); list_info->create_command_allocator_id = cmd_alloc_wrapper->GetCaptureId(); + list_info->reset_command_allocator_id = list_info->create_command_allocator_id; list_info->create_command_allocator_info = cmd_alloc_wrapper->GetObjectInfo(); } } diff --git a/framework/encode/dx12_state_tracker.h b/framework/encode/dx12_state_tracker.h index e1b38789a3..eef2489154 100644 --- a/framework/encode/dx12_state_tracker.h +++ b/framework/encode/dx12_state_tracker.h @@ -1,6 +1,6 @@ /* ** Copyright (c) 2021 LunarG, Inc. -** Copyright (c) 2022-2024 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2022-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -143,6 +143,10 @@ class Dx12StateTracker UINT num_barriers, const D3D12_RESOURCE_BARRIER* barriers); + void TrackCommandList_Reset(ID3D12CommandList_Wrapper* list_wrapper, + ID3D12CommandAllocator* pAllocator, + ID3D12PipelineState* pInitialState); + void TrackExecuteCommandLists(ID3D12CommandQueue_Wrapper* queue_wrapper, UINT num_lists, ID3D12CommandList* const* lists); diff --git a/framework/encode/dx12_state_writer.cpp b/framework/encode/dx12_state_writer.cpp index ef3a4f3b7f..ecb987c8f2 100644 --- a/framework/encode/dx12_state_writer.cpp +++ b/framework/encode/dx12_state_writer.cpp @@ -1,6 +1,6 @@ /* ** Copyright (c) 2021 LunarG, Inc. -** Copyright (c) 2022-2024 Advanced Micro Devices, Inc. All rights reserved. +** Copyright (c) 2022-2025 Advanced Micro Devices, Inc. All rights reserved. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and associated documentation files (the "Software"), @@ -38,8 +38,7 @@ Dx12StateWriter::Dx12StateWriter(util::FileOutputStream* output_stream, util::Compressor* compressor, format::ThreadId thread_id, util::FileOutputStream* asset_file_stream) : - output_stream_(output_stream), - compressor_(compressor), thread_id_(thread_id), encoder_(¶meter_stream_) + output_stream_(output_stream), compressor_(compressor), thread_id_(thread_id), encoder_(¶meter_stream_) { assert(output_stream != nullptr); } @@ -797,7 +796,7 @@ void Dx12StateWriter::WriteResourceSnapshot(graphics::Dx12ResourceDataUtil* reso { // Needs swapchain's queue to write its buffer. auto swapchain_info = resource_info->swapchain_wrapper->GetObjectInfo(); - queue = swapchain_info->command_queue; + queue = swapchain_info->command_queue; } // Read the data from the resource. HRESULT result = resource_data_util->ReadFromResource(resource, @@ -1089,8 +1088,13 @@ void Dx12StateWriter::WriteCommandListCommands(const ID3D12CommandList_Wrapper* size_t data_size = list_info->command_data.GetDataSize(); const uint8_t* data = list_info->command_data.GetData(); - // TODO: Don't write any commands, including the Reset or Close commands, if the command allocator used in the most - // recent Reset command no longer exists. + bool invalid_command_allocator = + (list_info->reset_command_allocator_id != format::kNullHandleId) && + (state_table.GetID3D12CommandAllocator_Wrapper(list_info->reset_command_allocator_id) == nullptr); + if (invalid_command_allocator == true) + { + return; + } while (offset < data_size) { const size_t* parameter_size = reinterpret_cast(&data[offset]); From 959ff18d8bf5b3219fab592941bc9ae9fd567e86 Mon Sep 17 00:00:00 2001 From: JerryAMD Date: Wed, 15 Jan 2025 13:19:20 -0500 Subject: [PATCH 2/2] Follow up review --- framework/encode/dx12_state_writer.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/framework/encode/dx12_state_writer.cpp b/framework/encode/dx12_state_writer.cpp index ecb987c8f2..ee62c4ecf4 100644 --- a/framework/encode/dx12_state_writer.cpp +++ b/framework/encode/dx12_state_writer.cpp @@ -1081,13 +1081,6 @@ void Dx12StateWriter::WriteCommandListCommands(const ID3D12CommandList_Wrapper* { auto list_info = list_wrapper->GetObjectInfo(); - bool write_commands = CheckCommandListObjects(list_info.get(), state_table); - - // Write each of the commands that was recorded for the command buffer. - size_t offset = 0; - size_t data_size = list_info->command_data.GetDataSize(); - const uint8_t* data = list_info->command_data.GetData(); - bool invalid_command_allocator = (list_info->reset_command_allocator_id != format::kNullHandleId) && (state_table.GetID3D12CommandAllocator_Wrapper(list_info->reset_command_allocator_id) == nullptr); @@ -1095,6 +1088,14 @@ void Dx12StateWriter::WriteCommandListCommands(const ID3D12CommandList_Wrapper* { return; } + + bool write_commands = CheckCommandListObjects(list_info.get(), state_table); + + // Write each of the commands that was recorded for the command buffer. + size_t offset = 0; + size_t data_size = list_info->command_data.GetDataSize(); + const uint8_t* data = list_info->command_data.GetData(); + while (offset < data_size) { const size_t* parameter_size = reinterpret_cast(&data[offset]);