On Wed, Jul 31, 2019 at 1:24 PM Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index e0e1aad..35290f3 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1205,12 +1205,99 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour void *dst_data, UINT dst_row_pitch, UINT dst_slice_pitch, UINT src_sub_resource, const D3D12_BOX *src_box) { - FIXME("iface %p, dst_data %p, dst_row_pitch %u, dst_slice_pitch %u, " - "src_sub_resource %u, src_box %p stub!\n", + struct d3d12_resource *resource = impl_from_ID3D12Resource(iface); + const struct vkd3d_vk_device_procs *vk_procs; + VkImageSubresourceLayers vk_sub_layers; + VkImageSubresource vk_sub_resource; + const struct vkd3d_format *format; + VkSubresourceLayout vk_layout; + struct d3d12_device *device; + void *src_map_ptr; + D3D12_BOX box; + HRESULT hr; + BYTE *dst; + UINT z;
Please use "unsigned int" instead of "INT". We stay away from UINT, VOID and similar defines in d3d code. Prototypes and definitions of D3D API functions are an exception.
+ + TRACE("iface %p, dst_data %p, dst_row_pitch %u, dst_slice_pitch %u, " + "src_sub_resource %u, src_box %p\n",
Missing dot ad the end of TRACE() message.
+ if (src_box) + { + box = *src_box; + } + else + { + box.left = 0; + box.top = 0; + box.front = 0; + box.right = resource->desc.Width; + box.bottom = resource->desc.Height; + box.back = d3d12_resource_desc_get_depth(&resource->desc, 0);
We need to call d3d12_resource_desc_get_depth() with the correct mip-level index.
+ if (!resource->heap) + { + FIXME("Not implemented for this resource type.\n"); + return E_NOTIMPL; + }
We should probably use d3d12_resource_is_cpu_accessible() instead. See d3d12_resource_is_cpu_accessible() for reference.
+ if (resource->desc.Layout != D3D12_TEXTURE_LAYOUT_ROW_MAJOR) + FIXME_ONCE("Layouts other than D3D12_TEXTURE_LAYOUT_ROW_MAJOR are not supported and results are implementation-dependent.\n");
The function needs to exit at this point. It is invalid usage to call vkGetImageSubresourceLayout() for images with VK_IMAGE_TILING_OPTIMAL.
+ vk_image_subresource_layers_from_d3d12(&vk_sub_layers, format, src_sub_resource, resource->desc.MipLevels); + vk_sub_resource.arrayLayer = vk_sub_layers.baseArrayLayer; + vk_sub_resource.mipLevel = vk_sub_layers.mipLevel; + vk_sub_resource.aspectMask = vk_sub_layers.aspectMask;
It doesn't seem beneficial to reuse vk_image_subresource_layers_from_d3d12(). Please simply fill members of vk_sub_resource(): it's less code, it doesn't need a new prototype in the header, it's a bit awkward to fill other structure with vk_image_subresource_layers_from_d3d12() and then copy some fields to vk_sub_resource(). vk_sub_resource.aspectMask = format->vk_aspect_mask; vk_sub_resource.mipLevel = src_sub_resource % resource->desc.MipLevels; vk_sub_resource.baseArrayLayer = src_sub_resource / resource->desc.MipLevels;
+ + VK_CALL(vkGetImageSubresourceLayout(device->vk_device, resource->u.vk_image, &vk_sub_resource, &vk_layout)); + TRACE("offset %#"PRIx64", size %#"PRIx64", rowPitch %#"PRIx64", arrayPitch %#"PRIx64", depthPitch %#"PRIx64".\n", + vk_layout.offset, vk_layout.size, vk_layout.rowPitch, vk_layout.arrayPitch, vk_layout.depthPitch);
Please avoid camelCase in TRACE() messages: "Offset ..., size, row pitch ..., array pitch ..., depth pitch ..."
+ + src_map_ptr = (BYTE*)src_map_ptr + vk_layout.offset; + for (z = box.front; z < box.back; ++z) + { + UINT y;
unsigned int. It could also be moved to other declarations at the top of the function.
+ dst = dst_data + (z - box.front) * dst_slice_pitch;
Are you sure that src_box should be used to offset dst?
+ for (y = box.top; y < box.bottom; y += format->block_height) + { + SIZE_T size = (box.right - box.left) / format->block_width + * format->byte_count * format->block_byte_count;
size_t