[PATCH v4 0/4] MR126: Handle map ranges if possible, or emit a warning.

[Conor McCarthy (＠cmccarthy)]

--
  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

[PATCH v4 1/4] vkd3d: Call vkInvalidateMappedMemoryRanges() when a mapping is requested on a mapped heap.

[Conor McCarthy]

From: Conor McCarthy

---
 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;

[PATCH v4 2/4] vkd3d: Pass an offset and size to d3d12_heap_map() in d3d12_resource_ReadFromSubresource().

[Conor McCarthy]

From: Conor McCarthy

---
 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,

[PATCH v4 3/4] vkd3d: Call vkFlushMappedMemoryRanges() when a heap remains mapped after an unmapping request.

[Conor McCarthy]

From: Conor McCarthy

---
 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;
 }

[PATCH v4 4/4] vkd3d: Pass an offset and size to d3d12_heap_unmap() in d3d12_resource_WriteToSubresource().

[Conor McCarthy]

From: Conor McCarthy

---
 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;
 }

[Giovanni Mascellani (＠giomasce)]

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.

[Giovanni Mascellani (＠giomasce)]

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.