tests: Immediately transition resources after a resource readback.
The resource could be destructed before the command list left open is executed; instead, we immediately perform the transition.
Merge request reports
Activity
reset_command_list(test_context->list, test_context->allocator); + transition_resource_state(test_context->list, resource->resource, D3D12_RESOURCE_STATE_COPY_SOURCE, state); + hr = ID3D12GraphicsCommandList_Close(test_context->list); + assert_that(hr == S_OK, "Failed to close command list, hr %#x.\n", hr); + exec_command_list(test_context->queue, test_context->list); + wait_queue_idle(test_context->device, test_context->queue); + reset_command_list(test_context->list, test_context->allocator);
We're probably not overly concerned with the performance of the shader runner, but this doesn't seem ideal. It also seems like there might be a similar issue in d3d12_runner_create_resource(). Is there any chance we could instead do something like flushing the current command list in d3d12_runner_destroy_resource() if needed?
Is there any chance we could instead do something like flushing the current command list in d3d12_runner_destroy_resource() if needed?
In principle yes, of course, but I'm not sure it's worth the effort. Notice that we're already doing something similar for dispatch and draw operations. If we decide that we never want to execute and flush the command list until we need to get back data from the GPU, we'd have to track all the operations that are outstanding at any given moment so we can reply them when flushing as you propose. Or we'd have to keep many lists around and choose which one to fire and when, and possibly also add synchronization between them. My feeling is that both are overkill for a test program which we'd rather try to keep simple and explicit.
It also seems like there might be a similar issue in d3d12_runner_create_resource().
Good point, I'll add a patch for that too.
It just occurred to me that maybe a better alternative would be to make
get_resource_readback_with_command_list()
andupload_texture_data()
aware of the states the resources they touch are at the beginning and should be at the end. So they can directly care about doing the required transitions in the command list they already execute.
Is there any chance we could instead do something like flushing the current command list in d3d12_runner_destroy_resource() if needed?
In principle yes, of course, but I'm not sure it's worth the effort. Notice that we're already doing something similar for dispatch and draw operations. If we decide that we never want to execute and flush the command list until we need to get back data from the GPU, we'd have to track all the operations that are outstanding at any given moment so we can reply them when flushing as you propose. Or we'd have to keep many lists around and choose which one to fire and when, and possibly also add synchronization between them. My feeling is that both are overkill for a test program which we'd rather try to keep simple and explicit.
What I had in mind would be to set something like "runner->pending_list = GRAPHICS | COMPUTE;" at the end of things like d3d12_runner_dispatch()/d3d12_runner_draw()/d3d12_runner_get_resource_readback(), and then flush as appropriate; it wouldn't need to be terribly complicated.
But yes, keeping track of the resource state might ultimately be nicer.
But yes, keeping track of the resource state might ultimately be nicer.
Not sure if what I did is what you intended: what do you think of the version I just pushed?
In the future more users could switch to the new
*_with_states()
functions and spare some boilerplate. In theory with this solution it is possible that some GPU time is wasted transitioning resources that will never be used again, but that looks like something we can afford. At least no new synchronization points should be added, I think.
But yes, keeping track of the resource state might ultimately be nicer.
Not sure if what I did is what you intended: what do you think of the version I just pushed?
I think that works, yeah. An alternative could be to store the "current" resource state in struct d3d12_resource, and then transition to the desired state as needed before draws, dispatches, readbacks, etc.
added 35 commits
-
e30433ab...58967018 - 32 commits from branch
wine:master
- ca7fa0c0 - tests: Immediately transition resources after readback in the shader runner.
- 46b7fccf - tests: Immediately transition textures after creation in the shader runner.
- c691ad88 - tests: Immediately transition buffers after creation in the shader runner.
Toggle commit list-
e30433ab...58967018 - 32 commits from branch
mentioned in merge request !366 (merged)