[PATCH v4 0/4] MR126: Handle map ranges if possible, or emit a warning.
-- v4: vkd3d: Pass an offset and size to d3d12_heap_unmap() in d3d12_resource_WriteToSubresource(). vkd3d: Call vkFlushMappedMemoryRanges() when a heap remains mapped after an unmapping request. vkd3d: Pass an offset and size to d3d12_heap_map() in d3d12_resource_ReadFromSubresource(). vkd3d: Call vkInvalidateMappedMemoryRanges() when a mapping is requested on a mapped heap. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/resource.c | 58 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 8c050cfe..8561e9c7 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -437,8 +437,28 @@ struct d3d12_heap *unsafe_impl_from_ID3D12Heap(ID3D12Heap *iface) return impl_from_ID3D12Heap(iface); } -static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, - struct d3d12_resource *resource, void **data) +static void d3d12_heap_update_mapping_locked(struct d3d12_heap *heap, uint64_t offset, uint64_t size) +{ + const struct vkd3d_vk_device_procs *vk_procs; + struct d3d12_device *device = heap->device; + VkMappedMemoryRange range; + + if (!size) + return; + + vk_procs = &device->vk_procs; + + range.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE; + range.pNext = NULL; + range.memory = heap->vk_memory; + range.offset = offset; + range.size = size; + + VK_CALL(vkInvalidateMappedMemoryRanges(device->vk_device, 1, &range)); +} + +static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, uint64_t read_offset, + uint64_t read_size, struct d3d12_resource *resource, bool *was_unmapped, void **data) { struct d3d12_device *device = heap->device; HRESULT hr = S_OK; @@ -448,6 +468,12 @@ static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, assert(!resource->map_count || heap->map_ptr); + if (was_unmapped) + *was_unmapped = !heap->map_ptr; + + if (heap->map_ptr) + d3d12_heap_update_mapping_locked(heap, offset + read_offset, read_size); + if (!resource->map_count) { if (!heap->map_ptr) @@ -1223,11 +1249,27 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_GetDevice(ID3D12Resource *iface, return d3d12_device_query_interface(resource->device, iid, device); } +static uint64_t offset_and_size_from_d3d12_range(const D3D12_RANGE *range, uint64_t default_size, uint64_t *size) +{ + if (range) + { + *size = (range->End > range->Begin) ? range->End - range->Begin : 0; + return range->Begin; + } + else + { + *size = default_size; + return 0; + } +} + static HRESULT STDMETHODCALLTYPE d3d12_resource_Map(ID3D12Resource *iface, UINT sub_resource, const D3D12_RANGE *read_range, void **data) { struct d3d12_resource *resource = impl_from_ID3D12Resource(iface); unsigned int sub_resource_count; + uint64_t read_offset, read_size; + bool was_unmapped; HRESULT hr; TRACE("iface %p, sub_resource %u, read_range %p, data %p.\n", @@ -1259,11 +1301,15 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_Map(ID3D12Resource *iface, UINT return E_NOTIMPL; } - WARN("Ignoring read range %p.\n", read_range); + read_offset = offset_and_size_from_d3d12_range(read_range, resource->desc.Width, &read_size); - if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, resource, data))) + if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, read_offset, read_size, resource, + &was_unmapped, data))) WARN("Failed to map resource %p, hr %#x.\n", resource, hr); + if (was_unmapped && read_range) + WARN("Ignored read range %p.\n", read_range); + if (data) TRACE("Returning pointer %p.\n", *data); @@ -1381,7 +1427,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_WriteToSubresource(ID3D12Resourc TRACE("Offset %#"PRIx64", size %#"PRIx64", row pitch %#"PRIx64", depth pitch %#"PRIx64".\n", vk_layout.offset, vk_layout.size, vk_layout.rowPitch, vk_layout.depthPitch); - if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, resource, (void **)&dst_data))) + if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, 0, 0, resource, NULL, (void **)&dst_data))) { WARN("Failed to map resource %p, hr %#x.\n", resource, hr); return hr; @@ -1469,7 +1515,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour TRACE("Offset %#"PRIx64", size %#"PRIx64", row pitch %#"PRIx64", depth pitch %#"PRIx64".\n", vk_layout.offset, vk_layout.size, vk_layout.rowPitch, vk_layout.depthPitch); - if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, resource, (void **)&src_data))) + if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, 0, 0, resource, NULL, (void **)&src_data))) { WARN("Failed to map resource %p, hr %#x.\n", resource, hr); return hr; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/resource.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 8561e9c7..20701fe3 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1453,6 +1453,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour const struct vkd3d_vk_device_procs *vk_procs; VkImageSubresource vk_sub_resource; const struct vkd3d_format *format; + uint64_t read_offset, read_size; VkSubresourceLayout vk_layout; struct d3d12_device *device; uint8_t *src_data; @@ -1515,14 +1516,20 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour TRACE("Offset %#"PRIx64", size %#"PRIx64", row pitch %#"PRIx64", depth pitch %#"PRIx64".\n", vk_layout.offset, vk_layout.size, vk_layout.rowPitch, vk_layout.depthPitch); - if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, 0, 0, resource, NULL, (void **)&src_data))) + read_offset = vkd3d_format_get_data_offset(format, vk_layout.rowPitch, + vk_layout.depthPitch, src_box->left, src_box->top, src_box->front); + read_size = vkd3d_format_get_data_offset(format, vk_layout.rowPitch, + vk_layout.depthPitch, src_box->right, src_box->bottom, src_box->back) - read_offset; + read_offset += vk_layout.offset; + + if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, read_offset, read_size, + resource, NULL, (void **)&src_data))) { WARN("Failed to map resource %p, hr %#x.\n", resource, hr); return hr; } - src_data += vk_layout.offset + vkd3d_format_get_data_offset(format, vk_layout.rowPitch, - vk_layout.depthPitch, src_box->left, src_box->top, src_box->front); + src_data += read_offset; vkd3d_format_copy_data(format, src_data, vk_layout.rowPitch, vk_layout.depthPitch, dst_data, dst_row_pitch, dst_slice_pitch, src_box->right - src_box->left, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/resource.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 20701fe3..15776a85 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -437,7 +437,7 @@ struct d3d12_heap *unsafe_impl_from_ID3D12Heap(ID3D12Heap *iface) return impl_from_ID3D12Heap(iface); } -static void d3d12_heap_update_mapping_locked(struct d3d12_heap *heap, uint64_t offset, uint64_t size) +static void d3d12_heap_update_mapping_locked(struct d3d12_heap *heap, uint64_t offset, uint64_t size, bool written) { const struct vkd3d_vk_device_procs *vk_procs; struct d3d12_device *device = heap->device; @@ -454,7 +454,10 @@ static void d3d12_heap_update_mapping_locked(struct d3d12_heap *heap, uint64_t o range.offset = offset; range.size = size; - VK_CALL(vkInvalidateMappedMemoryRanges(device->vk_device, 1, &range)); + if (written) + VK_CALL(vkFlushMappedMemoryRanges(device->vk_device, 1, &range)); + else + VK_CALL(vkInvalidateMappedMemoryRanges(device->vk_device, 1, &range)); } static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, uint64_t read_offset, @@ -472,7 +475,7 @@ static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, uint64_t *was_unmapped = !heap->map_ptr; if (heap->map_ptr) - d3d12_heap_update_mapping_locked(heap, offset + read_offset, read_size); + d3d12_heap_update_mapping_locked(heap, offset + read_offset, read_size, false); if (!resource->map_count) { @@ -517,9 +520,11 @@ static HRESULT d3d12_heap_map(struct d3d12_heap *heap, uint64_t offset, uint64_t return hr; } -static void d3d12_heap_unmap(struct d3d12_heap *heap, struct d3d12_resource *resource) +static bool d3d12_heap_unmap(struct d3d12_heap *heap, uint64_t written_offset, uint64_t written_size, + struct d3d12_resource *resource) { struct d3d12_device *device = heap->device; + bool is_unmapped; vkd3d_mutex_lock(&heap->mutex); @@ -551,7 +556,12 @@ static void d3d12_heap_unmap(struct d3d12_heap *heap, struct d3d12_resource *res } done: + if (!(is_unmapped = !heap->map_ptr)) + d3d12_heap_update_mapping_locked(heap, resource->heap_offset + written_offset, written_size, true); + vkd3d_mutex_unlock(&heap->mutex); + + return is_unmapped; } static HRESULT validate_heap_desc(const D3D12_HEAP_DESC *desc, const struct d3d12_resource *resource) @@ -1320,6 +1330,7 @@ static void STDMETHODCALLTYPE d3d12_resource_Unmap(ID3D12Resource *iface, UINT s const D3D12_RANGE *written_range) { struct d3d12_resource *resource = impl_from_ID3D12Resource(iface); + uint64_t written_offset, written_size; unsigned int sub_resource_count; TRACE("iface %p, sub_resource %u, written_range %p.\n", @@ -1332,9 +1343,11 @@ static void STDMETHODCALLTYPE d3d12_resource_Unmap(ID3D12Resource *iface, UINT s return; } - WARN("Ignoring written range %p.\n", written_range); + /* As long as Map() is only implemented for buffers we can use the resource width here. */ + written_offset = offset_and_size_from_d3d12_range(written_range, resource->desc.Width, &written_size); - d3d12_heap_unmap(resource->heap, resource); + if (d3d12_heap_unmap(resource->heap, written_offset, written_size, resource) && written_range) + WARN("Ignored written range %p.\n", written_range); } static D3D12_RESOURCE_DESC * STDMETHODCALLTYPE d3d12_resource_GetDesc(ID3D12Resource *iface, @@ -1440,7 +1453,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_WriteToSubresource(ID3D12Resourc dst_data, vk_layout.rowPitch, vk_layout.depthPitch, dst_box->right - dst_box->left, dst_box->bottom - dst_box->top, dst_box->back - dst_box->front); - d3d12_heap_unmap(resource->heap, resource); + d3d12_heap_unmap(resource->heap, 0, 0, resource); return S_OK; } @@ -1535,7 +1548,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour dst_data, dst_row_pitch, dst_slice_pitch, src_box->right - src_box->left, src_box->bottom - src_box->top, src_box->back - src_box->front); - d3d12_heap_unmap(resource->heap, resource); + d3d12_heap_unmap(resource->heap, 0, 0, resource); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/resource.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 15776a85..38171797 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1376,6 +1376,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_WriteToSubresource(ID3D12Resourc { struct d3d12_resource *resource = impl_from_ID3D12Resource(iface); const struct vkd3d_vk_device_procs *vk_procs; + uint64_t written_offset, written_size; VkImageSubresource vk_sub_resource; const struct vkd3d_format *format; VkSubresourceLayout vk_layout; @@ -1446,14 +1447,18 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_WriteToSubresource(ID3D12Resourc return hr; } - dst_data += vk_layout.offset + vkd3d_format_get_data_offset(format, vk_layout.rowPitch, + written_offset = vkd3d_format_get_data_offset(format, vk_layout.rowPitch, vk_layout.depthPitch, dst_box->left, dst_box->top, dst_box->front); + written_size = vkd3d_format_get_data_offset(format, vk_layout.rowPitch, + vk_layout.depthPitch, dst_box->right, dst_box->bottom, dst_box->back) - written_offset; + written_offset += vk_layout.offset; + dst_data += written_offset; vkd3d_format_copy_data(format, src_data, src_row_pitch, src_slice_pitch, dst_data, vk_layout.rowPitch, vk_layout.depthPitch, dst_box->right - dst_box->left, dst_box->bottom - dst_box->top, dst_box->back - dst_box->front); - d3d12_heap_unmap(resource->heap, 0, 0, resource); + d3d12_heap_unmap(resource->heap, written_offset, written_size, resource); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126
Giovanni Mascellani (@giomasce) commented about libs/vkd3d/resource.c:
assert(!resource->map_count || heap->map_ptr);
+ if (was_unmapped) + *was_unmapped = !heap->map_ptr; + + if (heap->map_ptr) + d3d12_heap_update_mapping_locked(heap, offset + read_offset, read_size);
Reading again the Vulkan specs, I think you need to always call `d3d12_heap_update_mapping_locked()`, also when the memory just got mapped. The docs for `vkInvalidateMappedMemoryRanges()` say that "Mapping non-coherent memory does not implicitly invalidate that memory" (and I guess we're dealing with non-coherent memory; if not, we don't need the whole invalidation business at all). And the same thing for flushing, which I think must happen also when you're going to unmap. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126#note_27149
Giovanni Mascellani (@giomasce) commented about libs/vkd3d/resource.c:
return E_NOTIMPL; }
- WARN("Ignoring read range %p.\n", read_range); + read_offset = offset_and_size_from_d3d12_range(read_range, resource->desc.Width, &read_size);
- if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, resource, data))) + if (FAILED(hr = d3d12_heap_map(resource->heap, resource->heap_offset, read_offset, read_size, resource, + &was_unmapped, data))) WARN("Failed to map resource %p, hr %#x.\n", resource, hr);
+ if (was_unmapped && read_range) + WARN("Ignored read range %p.\n", read_range);
I think this can be dropped, since as I wrote above we need to invalidate also when freshly mapping. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/126#note_27150
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)