-
Notifications
You must be signed in to change notification settings - Fork 126
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
Implement dx12 dump resources if in modifiable state #2001
base: dev
Are you sure you want to change the base?
Conversation
CI gfxreconstruct build queued with queue ID 360014. |
CI gfxreconstruct build # 5978 running. |
CI gfxreconstruct build # 5978 failed. |
451565e
to
c04157d
Compare
CI gfxreconstruct build queued with queue ID 361024. |
CI gfxreconstruct build # 5997 running. |
CI gfxreconstruct build # 5997 passed. |
[--dump-resources-modifiable-state-only ] | ||
[--pbi-all] [--pbis <index1,index2>] <file> | ||
<file> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two <file>
const uint64_t modifiableTransitionStates = | ||
D3D12_RESOURCE_STATE_RENDER_TARGET | D3D12_RESOURCE_STATE_DEPTH_WRITE | D3D12_RESOURCE_STATE_STREAM_OUT | | ||
D3D12_RESOURCE_STATE_COPY_DEST | D3D12_RESOURCE_STATE_RESOLVE_DEST | | ||
D3D12_RESOURCE_STATE_RAYTRACING_ACCELERATION_STRUCTURE | D3D12_RESOURCE_STATE_ALL_SHADER_RESOURCE | | ||
D3D12_RESOURCE_STATE_VIDEO_DECODE_WRITE | D3D12_RESOURCE_STATE_VIDEO_PROCESS_WRITE | | ||
D3D12_RESOURCE_STATE_VIDEO_ENCODE_WRITE; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does modifiableTransitionStates
mean that they are write states? If it's, D3D12_RESOURCE_STATE_UNORDERED_ACCESS
should also be.
https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_resource_states
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D3D12_RESOURCE_STATE_COMMON
is a special state which also may be writeable. A resource state can be promoted from D3D12_RESOURCE_STATE_COMMON
to other resource states--both read or write states--depending on how the resource is used.
if (options_.enable_dump_resources) | ||
{ | ||
GFXRECON_ASSERT(dump_resources_); | ||
if (state.transition.StateAfter & modifiableTransitionStates) | ||
{ | ||
dump_resources_->ModifiableResourceAdd(barriers[i].Transition->pResource); | ||
} | ||
else | ||
{ | ||
dump_resources_->ModifiableResourceRemove(barriers[i].Transition->pResource); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here might not be a good place to record states. command_list->ResourceBarrier
doesn't change the resource states until the command_list
is executed on ExecuteCommandLists
. command_list->ResourceBarrier
could be before the target ExecuteCommandLists
, but its ExecuteCommandLists
is after the target ExecuteCommandLists
. In this case, changing the state doesn't happen before dumping resources.
Getting the correct resource states is complicated. Dx12DumpResources::CopyResourceAsyncQueue
finds the correct resource states for copy resources. You could decide if the resource should be dumped there. But from what I remember, it's not perfect. It triggered validations about wrong states in some cases.
By the way, I feel wrong state validations are not always correct when the order of many ResourceBarrier
and ExecuteCommandLists
is intricate. Sometimes ResourceBarrier
prints the wrong states, but ExecuteCommandLists
doesn't print the wrong states, or sometimes ResourceBarrier
doesn't print the wrong states, but ExecuteCommandLists
prints the wrong states. I feel the validation on ExecuteCommandLists
is more accurate than ResourceBarrier
since I feel ResourceBarrier
isn't a good place to check states.
No description provided.