[PATCH 2/2] vkd3d/tests: Enable ReadFromSubresource() read-back test and remove todo.

[Józef Kucia]

On Wed, Jul 31, 2019 at 1:24 PM Conor McCarthy
<cmccarthy at codeweavers.com> wrote:
>
> Signed-off-by: Conor McCarthy <cmccarthy at codeweavers.com>
> ---
>  tests/d3d12.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/tests/d3d12.c b/tests/d3d12.c
> index 6cee005..7308525 100644
> --- a/tests/d3d12.c
> +++ b/tests/d3d12.c
> @@ -30119,7 +30119,7 @@ static void test_read_write_subresource(void)
>      todo ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr);
>
>      hr = ID3D12Resource_ReadFromSubresource(rb_buffer, dst_buffer, row_pitch, slice_pitch, 0, &box);
> -    todo ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr);
> +    ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr);

todo() changes need to be made together with the implementation. The
previous patch introduces test failures.

>
>      ID3D12Resource_Release(rb_buffer);
>
> @@ -30133,7 +30133,7 @@ static void test_read_write_subresource(void)
>      resource_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
>      resource_desc.SampleDesc.Count = 1;
>      resource_desc.SampleDesc.Quality = 0;
> -    resource_desc.Layout = D3D12_TEXTURE_LAYOUT_UNKNOWN;
> +    resource_desc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR;
>      resource_desc.Flags = 0;

I don't like this change. The main point of ReadFromSubresource() is
to read textures with undefined layout on CPU. We should add
additional test cases instead. Also, it should be possible to make
ReadFromSubresource() work with D3D12_TEXTURE_LAYOUT_UNKNOWN in our
implementations for some cases. We can try to use
VK_IMAGE_TILING_LINEAR for textures with custom heaps. This isn't very
nice but Vulkan doesn't allow us to read textures with optimal tiling.
See also VK_IMAGE_TILING_LINEAR restrictions in the Vulkan spec.

> +    /* WriteToSubresource() is not implemented so upload test data */
> +    transition_resource_state(command_list, src_texture,
> +            D3D12_RESOURCE_STATE_COMMON, D3D12_RESOURCE_STATE_COPY_DEST);
> +    texture_data.pData = dst_buffer;
> +    texture_data.RowPitch = row_pitch;
> +    texture_data.SlicePitch = slice_pitch;
> +    upload_texture_data(src_texture, &texture_data, 1, queue, command_list);
> +    reset_command_list(command_list, context.allocator);
> +    transition_resource_state(command_list, src_texture,
> +            D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_COMMON);

This makes a bunch of WriteToSubresource() and
ID3D12Resource_ReadFromSubresource() tests much less interesting.
Please try to extends tests instead and add this as a separate test
case as suggested above.