[PATCH 5/6] wined3d: Pass map flags to wined3d_context_copy_bo_address().

[Zebediah Figura (she/her)]

On 6/18/21 7:24 AM, Henri Verbeet wrote:
> On Fri, 18 Jun 2021 at 06:20, Zebediah Figura <z.figura12 at gmail.com> wrote:
>>   dlls/wined3d/adapter_gl.c      |  6 +++---
>>   dlls/wined3d/adapter_vk.c      | 14 +++++++-------
>>   dlls/wined3d/buffer.c          |  8 ++++++--
>>   dlls/wined3d/context_gl.c      |  2 +-
>>   dlls/wined3d/directx.c         |  4 ++--
>>   dlls/wined3d/view.c            |  2 +-
>>   dlls/wined3d/wined3d_private.h | 12 ++++++------
>>   7 files changed, 26 insertions(+), 22 deletions(-)
>>
> This may be ok, but I'm curious about the reasoning. In particular, is
> there ever a case where you'd want to pass WINED3D_MAP_DISCARD to
> wined3d_context_copy_bo_address(), instead of just using
> wined3d_context_map_bo_address() with WINED3D_MAP_DISCARD?
> 
> And mostly to illustrate the point:
> 
>> @@ -1084,8 +1084,8 @@ static void adapter_vk_copy_bo_address(struct wined3d_context *context,
>>
>>           staging.buffer_object = (uintptr_t)&staging_bo;
>>           staging.addr = NULL;
>> -        adapter_vk_copy_bo_address(context, &staging, src, size);
>> -        adapter_vk_copy_bo_address(context, dst, &staging, size);
>> +        adapter_vk_copy_bo_address(context, &staging, src, size, WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD);
>> +        adapter_vk_copy_bo_address(context, dst, &staging, size, map_flags);
>>
> We just created the staging bo, so it's idle by definition.
> 
>> @@ -1104,8 +1104,8 @@ static void adapter_vk_copy_bo_address(struct wined3d_context *context,
>>
>>           staging.buffer_object = (uintptr_t)&staging_bo;
>>           staging.addr = NULL;
>> -        adapter_vk_copy_bo_address(context, &staging, src, size);
>> -        adapter_vk_copy_bo_address(context, dst, &staging, size);
>> +        adapter_vk_copy_bo_address(context, &staging, src, size, WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD);
>> +        adapter_vk_copy_bo_address(context, dst, &staging, size, map_flags);
>>
> Likewise.
> 
>> @@ -1012,13 +1012,17 @@ static HRESULT buffer_resource_sub_resource_unmap(struct wined3d_resource *resou
>>   void wined3d_buffer_copy_bo_address(struct wined3d_buffer *dst_buffer, struct wined3d_context *context,
>>           unsigned int dst_offset, const struct wined3d_const_bo_address *src_addr, unsigned int size)
>>   {
>> +    DWORD map_flags = WINED3D_MAP_WRITE;
>>       struct wined3d_bo_address dst_addr;
>>       DWORD dst_location;
>>
>> +    if (!dst_offset && size == dst_buffer->resource.size)
>> +        map_flags |= WINED3D_MAP_DISCARD;
>> +
>>       dst_location = wined3d_buffer_get_memory(dst_buffer, context, &dst_addr);
>>       dst_addr.addr += dst_offset;
>>
>> -    wined3d_context_copy_bo_address(context, &dst_addr, (const struct wined3d_bo_address *)src_addr, size);
>> +    wined3d_context_copy_bo_address(context, &dst_addr, (const struct wined3d_bo_address *)src_addr, size, map_flags);
>>       wined3d_buffer_invalidate_range(dst_buffer, ~dst_location, dst_offset, size);
>>   }
>>
> Sure, but if that's worth doing, we may as well do it in the
> wined3d_context_copy_bo_address() implementations.

This is the case I care about mostly. I didn't actually bother to check 
whether we had access to the BO size, but evidently we do. I'll alter 
the implementation accordingly.

> 
>> @@ -1526,7 +1530,7 @@ static void wined3d_buffer_vk_upload_ranges(struct wined3d_buffer *buffer, struc
>>
>>               src.addr = (uint8_t *)data + range->offset - data_offset;
>>               dst.addr = (void *)(uintptr_t)range->offset;
>> -            wined3d_context_copy_bo_address(context, &dst, &src, range->size);
>> +            wined3d_context_copy_bo_address(context, &dst, &src, range->size, flags);
>>           }
>>
> "flags" never contains DISCARD here if "dst" is mappable, and if it's
> not, it doesn't matter.
> 
>> @@ -1597,7 +1597,7 @@ void wined3d_unordered_access_view_set_counter(struct wined3d_unordered_access_v
>>       dst.buffer_object = view->counter_bo;
>>       dst.addr = NULL;
>>
>> -    wined3d_context_copy_bo_address(context, &dst, &src, sizeof(uint32_t));
>> +    wined3d_context_copy_bo_address(context, &dst, &src, sizeof(uint32_t), WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD);
>>
>>       context_release(context);
>>   }
> Sure, but like further above, this could be caught in the
> wined3d_context_copy_bo_address() implementations.
>