With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/adapter_vk.c | 10 ++++++---- dlls/wined3d/buffer.c | 3 ++- dlls/wined3d/context_vk.c | 11 +++++------ dlls/wined3d/device.c | 2 +- dlls/wined3d/texture.c | 9 ++++++--- dlls/wined3d/view.c | 6 +++--- dlls/wined3d/wined3d_private.h | 5 +++-- 7 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 02a359c4f07..da1302265ca 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -936,7 +936,7 @@ static void *adapter_vk_map_bo_address(struct wined3d_context *context,
if ((map_flags & WINED3D_MAP_DISCARD) && bo->command_buffer_id > context_vk->completed_command_buffer_id) { - if (wined3d_context_vk_create_bo(context_vk, bo->size, bo->usage, bo->memory_type, &tmp)) + if (wined3d_device_vk_create_bo(device_vk, context_vk, bo->size, bo->usage, bo->memory_type, &tmp)) { bool host_synced = bo->host_synced; list_move_head(&tmp.b.users, &bo->b.users); @@ -1045,6 +1045,7 @@ static void adapter_vk_unmap_bo_address(struct wined3d_context *context, void adapter_vk_copy_bo_address(struct wined3d_context *context, const struct wined3d_bo_address *dst, const struct wined3d_bo_address *src, size_t size) { + struct wined3d_device_vk *device_vk = wined3d_device_vk(context->device); struct wined3d_context_vk *context_vk = wined3d_context_vk(context); const struct wined3d_vk_info *vk_info = context_vk->vk_info; struct wined3d_bo_vk staging_bo, *src_bo, *dst_bo; @@ -1122,7 +1123,7 @@ void adapter_vk_copy_bo_address(struct wined3d_context *context,
if (src_bo && !(src_bo->memory_type & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT)) { - if (!(wined3d_context_vk_create_bo(context_vk, size, VK_BUFFER_USAGE_TRANSFER_DST_BIT, + if (!(wined3d_device_vk_create_bo(device_vk, context_vk, size, VK_BUFFER_USAGE_TRANSFER_DST_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &staging_bo))) { ERR("Failed to create staging bo.\n"); @@ -1142,7 +1143,7 @@ void adapter_vk_copy_bo_address(struct wined3d_context *context, if (dst_bo && (!(dst_bo->memory_type & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) || (!(map_flags & WINED3D_MAP_DISCARD) && dst_bo->command_buffer_id > context_vk->completed_command_buffer_id))) { - if (!(wined3d_context_vk_create_bo(context_vk, size, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, + if (!(wined3d_device_vk_create_bo(device_vk, context_vk, size, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &staging_bo))) { ERR("Failed to create staging bo.\n"); @@ -1691,7 +1692,8 @@ static void adapter_vk_draw_primitive(struct wined3d_device *device, { struct wined3d_bo_vk *bo = &context_vk->vk_so_counter_bo;
- if (!wined3d_context_vk_create_bo(context_vk, ARRAY_SIZE(context_vk->vk_so_counters) * sizeof(uint32_t) * 2, + if (!wined3d_device_vk_create_bo(wined3d_device_vk(device), context_vk, + ARRAY_SIZE(context_vk->vk_so_counters) * sizeof(uint32_t) * 2, VK_BUFFER_USAGE_TRANSFORM_FEEDBACK_COUNTER_BUFFER_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, bo)) ERR("Failed to create counter BO.\n"); for (i = 0; i < ARRAY_SIZE(context_vk->vk_so_counters); ++i) diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 55dcd9e67e8..d4efecb2241 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1433,7 +1433,8 @@ static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buf else if (!(resource->usage & WINED3DUSAGE_DYNAMIC)) memory_type |= VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
- if (!(wined3d_context_vk_create_bo(context_vk, resource->size, usage, memory_type, &buffer_vk->bo))) + if (!(wined3d_device_vk_create_bo(wined3d_device_vk(resource->device), + context_vk, resource->size, usage, memory_type, &buffer_vk->bo))) { WARN("Failed to create Vulkan buffer.\n"); return FALSE; diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index ad7b0a8ba86..63de2f0f052 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -330,7 +330,7 @@ static struct wined3d_allocator_block *wined3d_context_vk_allocate_memory(struct return block; }
-static bool wined3d_context_vk_create_slab_bo(struct wined3d_context_vk *context_vk, +static bool wined3d_context_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { const struct wined3d_adapter_vk *adapter_vk = wined3d_adapter_vk(context_vk->c.device->adapter); @@ -374,7 +374,7 @@ static bool wined3d_context_vk_create_slab_bo(struct wined3d_context_vk *context }
slab->requested_memory_type = memory_type; - if (!wined3d_context_vk_create_bo(context_vk, key.size, usage, memory_type, &slab->bo)) + if (!wined3d_device_vk_create_bo(device_vk, context_vk, key.size, usage, memory_type, &slab->bo)) { ERR("Failed to create slab bo.\n"); heap_free(slab); @@ -424,10 +424,9 @@ static bool wined3d_context_vk_create_slab_bo(struct wined3d_context_vk *context return true; }
-BOOL wined3d_context_vk_create_bo(struct wined3d_context_vk *context_vk, VkDeviceSize size, - VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) +BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, + VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { - struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); const struct wined3d_vk_info *vk_info = context_vk->vk_info; VkMemoryRequirements memory_requirements; struct wined3d_adapter_vk *adapter_vk; @@ -435,7 +434,7 @@ BOOL wined3d_context_vk_create_bo(struct wined3d_context_vk *context_vk, VkDevic unsigned int memory_type_idx; VkResult vr;
- if (wined3d_context_vk_create_slab_bo(context_vk, size, usage, memory_type, bo)) + if (wined3d_context_vk_create_slab_bo(device_vk, context_vk, size, usage, memory_type, bo)) return TRUE;
adapter_vk = wined3d_adapter_vk(device_vk->d.adapter); diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index c5de58c29c9..cdea0ec42cf 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -655,7 +655,7 @@ bool wined3d_device_vk_create_null_resources(struct wined3d_device_vk *device_vk usage = VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT | VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT | VK_BUFFER_USAGE_VERTEX_BUFFER_BIT; memory_type = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; - if (!wined3d_context_vk_create_bo(context_vk, 16, usage, memory_type, &r->bo)) + if (!wined3d_device_vk_create_bo(device_vk, context_vk, 16, usage, memory_type, &r->bo)) return false; VK_CALL(vkCmdFillBuffer(vk_command_buffer, r->bo.vk_buffer, r->bo.buffer_offset, r->bo.size, 0x00000000u)); r->buffer_info.buffer = r->bo.vk_buffer; diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index df42f2d766b..02a5d96346f 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -4647,6 +4647,7 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, unsigned int dst_x, unsigned int dst_y, unsigned int dst_z) { struct wined3d_texture_vk *dst_texture_vk = wined3d_texture_vk(dst_texture); + struct wined3d_device_vk *device_vk = wined3d_device_vk(context->device); struct wined3d_context_vk *context_vk = wined3d_context_vk(context); unsigned int dst_level, dst_row_pitch, dst_slice_pitch; struct wined3d_texture_sub_resource *sub_resource; @@ -4722,7 +4723,7 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context,
if (!(src_bo = (struct wined3d_bo_vk *)src_bo_addr->buffer_object)) { - if (!wined3d_context_vk_create_bo(context_vk, sub_resource->size, + if (!wined3d_device_vk_create_bo(device_vk, context_vk, sub_resource->size, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &staging_bo)) { ERR("Failed to create staging bo.\n"); @@ -4833,6 +4834,7 @@ static void wined3d_texture_vk_download_data(struct wined3d_context *context, unsigned int dst_row_pitch, unsigned int dst_slice_pitch) { struct wined3d_texture_vk *src_texture_vk = wined3d_texture_vk(src_texture); + struct wined3d_device_vk *device_vk = wined3d_device_vk(context->device); struct wined3d_context_vk *context_vk = wined3d_context_vk(context); unsigned int src_level, src_width, src_height, src_depth; struct wined3d_texture_sub_resource *sub_resource; @@ -4919,7 +4921,7 @@ static void wined3d_texture_vk_download_data(struct wined3d_context *context,
if (!(dst_bo = (struct wined3d_bo_vk *)dst_bo_addr->buffer_object)) { - if (!wined3d_context_vk_create_bo(context_vk, sub_resource->size, + if (!wined3d_device_vk_create_bo(device_vk, context_vk, sub_resource->size, VK_BUFFER_USAGE_TRANSFER_DST_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &staging_bo)) { ERR("Failed to create staging bo.\n"); @@ -5191,6 +5193,7 @@ BOOL wined3d_texture_vk_prepare_texture(struct wined3d_texture_vk *texture_vk, static BOOL wined3d_texture_vk_prepare_buffer_object(struct wined3d_texture_vk *texture_vk, unsigned int sub_resource_idx, struct wined3d_context_vk *context_vk) { + struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); struct wined3d_texture_sub_resource *sub_resource; struct wined3d_bo_vk *bo;
@@ -5199,7 +5202,7 @@ static BOOL wined3d_texture_vk_prepare_buffer_object(struct wined3d_texture_vk * if (bo->vk_buffer) return TRUE;
- if (!wined3d_context_vk_create_bo(context_vk, sub_resource->size, + if (!wined3d_device_vk_create_bo(device_vk, context_vk, sub_resource->size, VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, bo)) return FALSE; diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index 374e693b17d..6a88286b861 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -2078,8 +2078,8 @@ void wined3d_unordered_access_view_vk_clear(struct wined3d_unordered_access_view vk_writes[1].descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; vk_writes[1].pBufferInfo = &buffer_info;
- if (!wined3d_context_vk_create_bo(context_vk, sizeof(constants), VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, - VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &constants_bo)) + if (!wined3d_device_vk_create_bo(device_vk, context_vk, sizeof(constants), + VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &constants_bo)) { ERR("Failed to create constants BO.\n"); goto out; @@ -2213,7 +2213,7 @@ static void wined3d_unordered_access_view_vk_cs_init(void *object)
if (desc->flags & (WINED3D_VIEW_BUFFER_COUNTER | WINED3D_VIEW_BUFFER_APPEND)) { - if (!wined3d_context_vk_create_bo(context_vk, sizeof(uint32_t), VK_BUFFER_USAGE_TRANSFER_SRC_BIT + if (!wined3d_device_vk_create_bo(device_vk, context_vk, sizeof(uint32_t), VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, &uav_vk->counter_bo)) { diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index aa8974366a6..b2525110adc 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2642,8 +2642,6 @@ VkCommandBuffer wined3d_context_vk_apply_compute_state(struct wined3d_context_vk VkCommandBuffer wined3d_context_vk_apply_draw_state(struct wined3d_context_vk *context_vk, const struct wined3d_state *state, struct wined3d_buffer_vk *indirect_vk, bool indexed) DECLSPEC_HIDDEN; void wined3d_context_vk_cleanup(struct wined3d_context_vk *context_vk) DECLSPEC_HIDDEN; -BOOL wined3d_context_vk_create_bo(struct wined3d_context_vk *context_vk, VkDeviceSize size, - VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) DECLSPEC_HIDDEN; BOOL wined3d_context_vk_create_image(struct wined3d_context_vk *context_vk, VkImageType vk_image_type, VkImageUsageFlags usage, VkFormat vk_format, unsigned int width, unsigned int height, unsigned int depth, unsigned int sample_count, unsigned int mip_levels, unsigned int layer_count, unsigned int flags, @@ -4094,6 +4092,9 @@ static inline struct wined3d_device_vk *wined3d_device_vk(struct wined3d_device return CONTAINING_RECORD(device, struct wined3d_device_vk, d); }
+BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, + VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, + struct wined3d_bo_vk *bo) DECLSPEC_HIDDEN; bool wined3d_device_vk_create_null_resources(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk) DECLSPEC_HIDDEN; bool wined3d_device_vk_create_null_views(struct wined3d_device_vk *device_vk,
With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/context_vk.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 63de2f0f052..9854ca70ef0 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -306,10 +306,9 @@ VkDeviceMemory wined3d_context_vk_allocate_vram_chunk_memory(struct wined3d_cont return vk_memory; }
-static struct wined3d_allocator_block *wined3d_context_vk_allocate_memory(struct wined3d_context_vk *context_vk, - unsigned int memory_type, VkDeviceSize size, VkDeviceMemory *vk_memory) +static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct wined3d_device_vk *device_vk, + struct wined3d_context_vk *context_vk, unsigned int memory_type, VkDeviceSize size, VkDeviceMemory *vk_memory) { - struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); struct wined3d_allocator *allocator = &device_vk->allocator; struct wined3d_allocator_block *block;
@@ -330,7 +329,7 @@ static struct wined3d_allocator_block *wined3d_context_vk_allocate_memory(struct return block; }
-static bool wined3d_context_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, +static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { const struct wined3d_adapter_vk *adapter_vk = wined3d_adapter_vk(context_vk->c.device->adapter); @@ -434,7 +433,7 @@ BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct win unsigned int memory_type_idx; VkResult vr;
- if (wined3d_context_vk_create_slab_bo(device_vk, context_vk, size, usage, memory_type, bo)) + if (wined3d_device_vk_create_slab_bo(device_vk, context_vk, size, usage, memory_type, bo)) return TRUE;
adapter_vk = wined3d_adapter_vk(device_vk->d.adapter); @@ -464,7 +463,7 @@ BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct win VK_CALL(vkDestroyBuffer(device_vk->vk_device, bo->vk_buffer, NULL)); return FALSE; } - bo->memory = wined3d_context_vk_allocate_memory(context_vk, + bo->memory = wined3d_device_vk_allocate_memory(device_vk, context_vk, memory_type_idx, memory_requirements.size, &bo->vk_memory); if (!bo->vk_memory) { @@ -557,7 +556,7 @@ BOOL wined3d_context_vk_create_image(struct wined3d_context_vk *context_vk, VkIm return FALSE; }
- image->memory = wined3d_context_vk_allocate_memory(context_vk, memory_type_idx, + image->memory = wined3d_device_vk_allocate_memory(device_vk, context_vk, memory_type_idx, memory_requirements.size, &image->vk_memory); if (!image->vk_memory) {
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
-static struct wined3d_allocator_block *wined3d_context_vk_allocate_memory(struct wined3d_context_vk *context_vk,
unsigned int memory_type, VkDeviceSize size, VkDeviceMemory *vk_memory)+static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct wined3d_device_vk *device_vk,
struct wined3d_context_vk *context_vk, unsigned int memory_type, VkDeviceSize size, VkDeviceMemory *vk_memory){
Device functions in device.c.
-static bool wined3d_context_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, +static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) {
This change belong in patch 1/8. (Or possibly even a separate patch.)
In order to allow slab allocation from the client thread.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/adapter_vk.c | 13 +++++++++++++ dlls/wined3d/context_vk.c | 30 +++++++++--------------------- dlls/wined3d/wined3d_private.h | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index da1302265ca..218b7dc74ba 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -397,6 +397,18 @@ static const struct wined3d_allocator_ops wined3d_allocator_vk_ops = .allocator_destroy_chunk = wined3d_allocator_vk_destroy_chunk, };
+static int wined3d_bo_slab_vk_compare(const void *key, const struct wine_rb_entry *entry) +{ + const struct wined3d_bo_slab_vk *slab = WINE_RB_ENTRY_VALUE(entry, const struct wined3d_bo_slab_vk, entry); + const struct wined3d_bo_slab_vk_key *k = key; + + if (k->memory_type != slab->requested_memory_type) + return k->memory_type - slab->requested_memory_type; + if (k->usage != slab->bo.usage) + return k->usage - slab->bo.usage; + return k->size - slab->bo.size; +} + static HRESULT adapter_vk_create_device(struct wined3d *wined3d, const struct wined3d_adapter *adapter, enum wined3d_device_type device_type, HWND focus_window, unsigned int flags, BYTE surface_alignment, const enum wined3d_feature_level *levels, unsigned int level_count, @@ -512,6 +524,7 @@ static HRESULT adapter_vk_create_device(struct wined3d *wined3d, const struct wi hr = E_FAIL; goto fail; } + wine_rb_init(&device_vk->bo_slab_available, wined3d_bo_slab_vk_compare);
if (FAILED(hr = wined3d_device_init(&device_vk->d, wined3d, adapter->ordinal, device_type, focus_window, flags, surface_alignment, levels, level_count, vk_info->supported, device_parent))) diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 9854ca70ef0..a6a2ea8a44b 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -359,7 +359,7 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk key.usage = usage; key.size = 32 * object_size;
- if ((entry = wine_rb_get(&context_vk->bo_slab_available, &key))) + if ((entry = wine_rb_get(&device_vk->bo_slab_available, &key))) { slab = WINE_RB_ENTRY_VALUE(entry, struct wined3d_bo_slab_vk, entry); TRACE("Using existing bo slab %p.\n", slab); @@ -381,7 +381,7 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk } slab->map = ~0u;
- if (wine_rb_put(&context_vk->bo_slab_available, &key, &slab->entry) < 0) + if (wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry) < 0) { ERR("Failed to add slab to available tree.\n"); wined3d_context_vk_destroy_bo(context_vk, &slab->bo); @@ -397,12 +397,12 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk { if (slab->next) { - wine_rb_replace(&context_vk->bo_slab_available, &slab->entry, &slab->next->entry); + wine_rb_replace(&device_vk->bo_slab_available, &slab->entry, &slab->next->entry); slab->next = NULL; } else { - wine_rb_remove(&context_vk->bo_slab_available, &slab->entry); + wine_rb_remove(&device_vk->bo_slab_available, &slab->entry); } }
@@ -705,6 +705,7 @@ void wined3d_context_vk_destroy_allocator_block(struct wined3d_context_vk *conte static void wined3d_bo_slab_vk_free_slice(struct wined3d_bo_slab_vk *slab, SIZE_T idx, struct wined3d_context_vk *context_vk) { + struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); struct wined3d_bo_slab_vk_key key; struct wine_rb_entry *entry;
@@ -716,12 +717,12 @@ static void wined3d_bo_slab_vk_free_slice(struct wined3d_bo_slab_vk *slab, key.usage = slab->bo.usage; key.size = slab->bo.size;
- if ((entry = wine_rb_get(&context_vk->bo_slab_available, &key))) + if ((entry = wine_rb_get(&device_vk->bo_slab_available, &key))) { slab->next = WINE_RB_ENTRY_VALUE(entry, struct wined3d_bo_slab_vk, entry); - wine_rb_replace(&context_vk->bo_slab_available, entry, &slab->entry); + wine_rb_replace(&device_vk->bo_slab_available, entry, &slab->entry); } - else if (wine_rb_put(&context_vk->bo_slab_available, &key, &slab->entry) < 0) + else if (wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry) < 0) { ERR("Unable to return slab %p (map 0x%08x) to available tree.\n", slab, slab->map); } @@ -1458,7 +1459,7 @@ void wined3d_context_vk_cleanup(struct wined3d_context_vk *context_vk) wined3d_context_vk_destroy_query_pools(context_vk, &context_vk->free_timestamp_query_pools); wined3d_context_vk_destroy_query_pools(context_vk, &context_vk->free_pipeline_statistics_query_pools); wined3d_context_vk_destroy_query_pools(context_vk, &context_vk->free_stream_output_statistics_query_pools); - wine_rb_destroy(&context_vk->bo_slab_available, wined3d_context_vk_destroy_bo_slab, context_vk); + wine_rb_destroy(&device_vk->bo_slab_available, wined3d_context_vk_destroy_bo_slab, context_vk); heap_free(context_vk->pending_queries.queries); heap_free(context_vk->submitted.buffers); heap_free(context_vk->retired.objects); @@ -1825,18 +1826,6 @@ static int wined3d_graphics_pipeline_vk_compare(const void *key, const struct wi return 0; }
-static int wined3d_bo_slab_vk_compare(const void *key, const struct wine_rb_entry *entry) -{ - const struct wined3d_bo_slab_vk *slab = WINE_RB_ENTRY_VALUE(entry, const struct wined3d_bo_slab_vk, entry); - const struct wined3d_bo_slab_vk_key *k = key; - - if (k->memory_type != slab->requested_memory_type) - return k->memory_type - slab->requested_memory_type; - if (k->usage != slab->bo.usage) - return k->usage - slab->bo.usage; - return k->size - slab->bo.size; -} - static void wined3d_context_vk_init_graphics_pipeline_key(struct wined3d_context_vk *context_vk) { struct wined3d_graphics_pipeline_key_vk *key; @@ -3508,7 +3497,6 @@ HRESULT wined3d_context_vk_init(struct wined3d_context_vk *context_vk, struct wi wine_rb_init(&context_vk->render_passes, wined3d_render_pass_vk_compare); wine_rb_init(&context_vk->pipeline_layouts, wined3d_pipeline_layout_vk_compare); wine_rb_init(&context_vk->graphics_pipelines, wined3d_graphics_pipeline_vk_compare); - wine_rb_init(&context_vk->bo_slab_available, wined3d_bo_slab_vk_compare);
return WINED3D_OK; } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index b2525110adc..d8b16c0d074 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2622,7 +2622,6 @@ struct wined3d_context_vk struct wine_rb_tree render_passes; struct wine_rb_tree pipeline_layouts; struct wine_rb_tree graphics_pipelines; - struct wine_rb_tree bo_slab_available; };
static inline struct wined3d_context_vk *wined3d_context_vk(struct wined3d_context *context) @@ -4085,6 +4084,7 @@ struct wined3d_device_vk struct wined3d_allocator allocator;
struct wined3d_uav_clear_state_vk uav_clear_state; + struct wine_rb_tree bo_slab_available; };
static inline struct wined3d_device_vk *wined3d_device_vk(struct wined3d_device *device)
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
In order to allow slab allocation from the client thread.
As mentioned in 1/8, this is not necessarily required for the Vulkan backend. It makes sense to move this to the device for GL, but in that case perhaps it also makes sense to move it to struct wined3d_device instead of struct wined3d_device_vk.
@@ -512,6 +524,7 @@ static HRESULT adapter_vk_create_device(struct wined3d *wined3d, const struct wi hr = E_FAIL; goto fail; }
wine_rb_init(&device_vk->bo_slab_available, wined3d_bo_slab_vk_compare);
if (FAILED(hr = wined3d_device_init(&device_vk->d, wined3d, adapter->ordinal, device_type, focus_window, flags, surface_alignment, levels, level_count, vk_info->supported, device_parent)))
This should call wine_rb_destroy() on wined3d_device_init() failure. It turns out not doing that doesn't actually leak anything because wine_rb_init() only does trivial initialisation, but that's an implementation detail and could change.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/buffer.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index d4efecb2241..56cce00301e 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1399,10 +1399,10 @@ HRESULT wined3d_buffer_gl_init(struct wined3d_buffer_gl *buffer_gl, struct wined return wined3d_buffer_init(&buffer_gl->b, device, desc, data, parent, parent_ops, &wined3d_buffer_gl_ops); }
-static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buffer_vk, - struct wined3d_context_vk *context_vk) +static BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk, + struct wined3d_context_vk *context_vk, struct wined3d_bo_vk *bo) { - struct wined3d_resource *resource = &buffer_vk->b.resource; + const struct wined3d_resource *resource = &buffer_vk->b.resource; uint32_t bind_flags = resource->bind_flags; VkMemoryPropertyFlags memory_type; VkBufferUsageFlags usage; @@ -1433,19 +1433,8 @@ static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buf else if (!(resource->usage & WINED3DUSAGE_DYNAMIC)) memory_type |= VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
- if (!(wined3d_device_vk_create_bo(wined3d_device_vk(resource->device), - context_vk, resource->size, usage, memory_type, &buffer_vk->bo))) - { - WARN("Failed to create Vulkan buffer.\n"); - return FALSE; - } - - list_init(&buffer_vk->b.bo_user.entry); - list_add_head(&buffer_vk->bo.b.users, &buffer_vk->b.bo_user.entry); - buffer_vk->b.buffer_object = (uintptr_t)&buffer_vk->bo; - buffer_invalidate_bo_range(&buffer_vk->b, 0, 0); - - return TRUE; + return wined3d_device_vk_create_bo(wined3d_device_vk(resource->device), + context_vk, resource->size, usage, memory_type, bo); }
const VkDescriptorBufferInfo *wined3d_buffer_vk_get_buffer_info(struct wined3d_buffer_vk *buffer_vk) @@ -1464,6 +1453,8 @@ const VkDescriptorBufferInfo *wined3d_buffer_vk_get_buffer_info(struct wined3d_b static BOOL wined3d_buffer_vk_prepare_location(struct wined3d_buffer *buffer, struct wined3d_context *context, unsigned int location) { + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer); + switch (location) { case WINED3D_LOCATION_SYSMEM: @@ -1473,7 +1464,16 @@ static BOOL wined3d_buffer_vk_prepare_location(struct wined3d_buffer *buffer, if (buffer->buffer_object) return TRUE;
- return wined3d_buffer_vk_create_buffer_object(wined3d_buffer_vk(buffer), wined3d_context_vk(context)); + if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, wined3d_context_vk(context), &buffer_vk->bo)) + { + WARN("Failed to create Vulkan buffer.\n"); + return FALSE; + } + + list_add_head(&buffer_vk->bo.b.users, &buffer_vk->b.bo_user.entry); + buffer_vk->b.buffer_object = (uintptr_t)&buffer_vk->bo; + buffer_invalidate_bo_range(&buffer_vk->b, 0, 0); + return TRUE;
default: FIXME("Unhandled location %s.\n", wined3d_debug_location(location));
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
-static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buffer_vk,
struct wined3d_context_vk *context_vk)+static BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk,
struct wined3d_context_vk *context_vk, struct wined3d_bo_vk *bo){
"bo_vk" by convention, although we're not terribly consistent about that everywhere. "BOOL" -> "bool" if we're touching it.
@@ -1433,19 +1433,8 @@ static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buf else if (!(resource->usage & WINED3DUSAGE_DYNAMIC)) memory_type |= VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
- if (!(wined3d_device_vk_create_bo(wined3d_device_vk(resource->device),
context_vk, resource->size, usage, memory_type, &buffer_vk->bo)))- {
WARN("Failed to create Vulkan buffer.\n");return FALSE;- }
- list_init(&buffer_vk->b.bo_user.entry);
- list_add_head(&buffer_vk->bo.b.users, &buffer_vk->b.bo_user.entry);
- buffer_vk->b.buffer_object = (uintptr_t)&buffer_vk->bo;
- buffer_invalidate_bo_range(&buffer_vk->b, 0, 0);
- return TRUE;
- return wined3d_device_vk_create_bo(wined3d_device_vk(resource->device),
context_vk, resource->size, usage, memory_type, bo);}
Is this the best way to handle this particular issue? I gather we're doing this primarily in order to create a bo with the correct "usage" and "memory_type" in patch 8/8. However, we could also achieve that by introducing helpers along the lines of vk_access_mask_from_bind_flags() to determine the correct "usage" and "memory_type", and then just call wined3d_device_vk_create_bo() directly from adapter_vk_alloc_bo() in patch 8/8. We may not necessarily need to pass a resource to adapter_alloc_bo() in that case either.
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
-static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buffer_vk,
struct wined3d_context_vk *context_vk)+static BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk,
struct wined3d_context_vk *context_vk, struct wined3d_bo_vk *bo)"bo_vk" by convention, although we're not terribly consistent about that everywhere. "BOOL" -> "bool" if we're touching it.
@@ -1433,19 +1433,8 @@ static BOOL wined3d_buffer_vk_create_buffer_object(struct wined3d_buffer_vk *buf else if (!(resource->usage & WINED3DUSAGE_DYNAMIC)) memory_type |= VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
- if (!(wined3d_device_vk_create_bo(wined3d_device_vk(resource->device),
context_vk, resource->size, usage, memory_type, &buffer_vk->bo)))- {
WARN("Failed to create Vulkan buffer.\n");return FALSE;- }
- list_init(&buffer_vk->b.bo_user.entry);
- list_add_head(&buffer_vk->bo.b.users, &buffer_vk->b.bo_user.entry);
- buffer_vk->b.buffer_object = (uintptr_t)&buffer_vk->bo;
- buffer_invalidate_bo_range(&buffer_vk->b, 0, 0);
- return TRUE;
- return wined3d_device_vk_create_bo(wined3d_device_vk(resource->device),
context_vk, resource->size, usage, memory_type, bo);Is this the best way to handle this particular issue? I gather we're doing this primarily in order to create a bo with the correct "usage" and "memory_type" in patch 8/8. However, we could also achieve that by introducing helpers along the lines of vk_access_mask_from_bind_flags() to determine the correct "usage" and "memory_type", and then just call wined3d_device_vk_create_bo() directly from adapter_vk_alloc_bo() in patch 8/8. We may not necessarily need to pass a resource to adapter_alloc_bo() in that case either.
Sure, that looks reasonable to me.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/adapter_vk.c | 9 +++++++++ dlls/wined3d/context_vk.c | 31 +++++++++++++++++++++++++++---- dlls/wined3d/wined3d_private.h | 1 + 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 218b7dc74ba..020a70b4ed4 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -517,6 +517,10 @@ static HRESULT adapter_vk_create_device(struct wined3d *wined3d, const struct wi #undef VK_DEVICE_EXT_PFN #undef VK_DEVICE_PFN
+ InitializeCriticalSection(&device_vk->allocator_cs); + if (device_vk->allocator_cs.DebugInfo != (RTL_CRITICAL_SECTION_DEBUG *)-1) + device_vk->allocator_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": wined3d_device_vk.allocator_cs"); + if (!wined3d_allocator_init(&device_vk->allocator, adapter_vk->memory_properties.memoryTypeCount, &wined3d_allocator_vk_ops)) { @@ -551,6 +555,11 @@ static void adapter_vk_destroy_device(struct wined3d_device *device)
wined3d_device_cleanup(&device_vk->d); wined3d_allocator_cleanup(&device_vk->allocator); + + if (device_vk->allocator_cs.DebugInfo != (RTL_CRITICAL_SECTION_DEBUG *)-1) + device_vk->allocator_cs.DebugInfo->Spare[0] = 0; + DeleteCriticalSection(&device_vk->allocator_cs); + VK_CALL(vkDestroyDevice(device_vk->vk_device, NULL)); heap_free(device_vk); } diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index a6a2ea8a44b..21239bd74b9 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -312,23 +312,36 @@ static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct struct wined3d_allocator *allocator = &device_vk->allocator; struct wined3d_allocator_block *block;
+ EnterCriticalSection(&device_vk->allocator_cs); + if (size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2) { *vk_memory = wined3d_context_vk_allocate_vram_chunk_memory(context_vk, memory_type, size); + LeaveCriticalSection(&device_vk->allocator_cs); return NULL; }
if (!(block = wined3d_allocator_allocate(allocator, &context_vk->c, memory_type, size))) { + LeaveCriticalSection(&device_vk->allocator_cs); *vk_memory = VK_NULL_HANDLE; return NULL; }
*vk_memory = wined3d_allocator_chunk_vk(block->chunk)->vk_memory;
+ LeaveCriticalSection(&device_vk->allocator_cs); return block; }
+static void wined3d_device_vk_free_memory(struct wined3d_device_vk *device_vk, struct wined3d_allocator_block *block) +{ + assert(block->chunk->allocator == &device_vk->allocator); + EnterCriticalSection(&device_vk->allocator_cs); + wined3d_allocator_block_free(block); + LeaveCriticalSection(&device_vk->allocator_cs); +} + static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { @@ -359,6 +372,8 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk key.usage = usage; key.size = 32 * object_size;
+ EnterCriticalSection(&device_vk->allocator_cs); + if ((entry = wine_rb_get(&device_vk->bo_slab_available, &key))) { slab = WINE_RB_ENTRY_VALUE(entry, struct wined3d_bo_slab_vk, entry); @@ -368,6 +383,7 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk { if (!(slab = heap_alloc_zero(sizeof(*slab)))) { + LeaveCriticalSection(&device_vk->allocator_cs); ERR("Failed to allocate bo slab.\n"); return false; } @@ -375,6 +391,7 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk slab->requested_memory_type = memory_type; if (!wined3d_device_vk_create_bo(device_vk, context_vk, key.size, usage, memory_type, &slab->bo)) { + LeaveCriticalSection(&device_vk->allocator_cs); ERR("Failed to create slab bo.\n"); heap_free(slab); return false; @@ -406,6 +423,8 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk } }
+ LeaveCriticalSection(&device_vk->allocator_cs); + *bo = slab->bo; bo->memory = NULL; bo->slab = slab; @@ -478,7 +497,7 @@ BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct win { ERR("Failed to bind buffer memory, vr %s.\n", wined3d_debug_vkresult(vr)); if (bo->memory) - wined3d_allocator_block_free(bo->memory); + wined3d_device_vk_free_memory(device_vk, bo->memory); else VK_CALL(vkFreeMemory(device_vk->vk_device, bo->vk_memory, NULL)); VK_CALL(vkDestroyBuffer(device_vk->vk_device, bo->vk_buffer, NULL)); @@ -572,7 +591,7 @@ BOOL wined3d_context_vk_create_image(struct wined3d_context_vk *context_vk, VkIm { VK_CALL(vkDestroyImage(device_vk->vk_device, image->vk_image, NULL)); if (image->memory) - wined3d_allocator_block_free(image->memory); + wined3d_device_vk_free_memory(device_vk, image->memory); else VK_CALL(vkFreeMemory(device_vk->vk_device, image->vk_memory, NULL)); ERR("Failed to bind image memory, vr %s.\n", wined3d_debug_vkresult(vr)); @@ -686,7 +705,7 @@ void wined3d_context_vk_destroy_allocator_block(struct wined3d_context_vk *conte
if (context_vk->completed_command_buffer_id > command_buffer_id) { - wined3d_allocator_block_free(block); + wined3d_device_vk_free_memory(wined3d_device_vk(context_vk->c.device), block); TRACE("Freed block %p.\n", block); return; } @@ -711,6 +730,8 @@ static void wined3d_bo_slab_vk_free_slice(struct wined3d_bo_slab_vk *slab,
TRACE("slab %p, idx %lu, context_vk %p.\n", slab, idx, context_vk);
+ EnterCriticalSection(&device_vk->allocator_cs); + if (!slab->map) { key.memory_type = slab->requested_memory_type; @@ -728,6 +749,8 @@ static void wined3d_bo_slab_vk_free_slice(struct wined3d_bo_slab_vk *slab, } } slab->map |= 1u << idx; + + LeaveCriticalSection(&device_vk->allocator_cs); }
static void wined3d_context_vk_destroy_bo_slab_slice(struct wined3d_context_vk *context_vk, @@ -999,7 +1022,7 @@ static void wined3d_context_vk_cleanup_resources(struct wined3d_context_vk *cont
case WINED3D_RETIRED_ALLOCATOR_BLOCK_VK: TRACE("Destroying block %p.\n", o->u.block); - wined3d_allocator_block_free(o->u.block); + wined3d_device_vk_free_memory(device_vk, o->u.block); break;
case WINED3D_RETIRED_BO_SLAB_SLICE_VK: diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index d8b16c0d074..21b4ef2e103 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4081,6 +4081,7 @@ struct wined3d_device_vk struct wined3d_null_resources_vk null_resources_vk; struct wined3d_null_views_vk null_views_vk;
+ CRITICAL_SECTION allocator_cs; struct wined3d_allocator allocator;
struct wined3d_uav_clear_state_vk uav_clear_state;
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
@@ -312,23 +312,36 @@ static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct struct wined3d_allocator *allocator = &device_vk->allocator; struct wined3d_allocator_block *block;
- EnterCriticalSection(&device_vk->allocator_cs);
We'd typically wrap the EnterCriticalSection() call in something like wined3d_device_vk_allocator_lock(). Mostly because that's a little easier to search for than an EnterCriticalSection() call with a specific critical section in some cases, but I suppose it would also make it slightly easier to replace the critical section with something else, if needed.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/context_vk.c | 19 +++++++------------ dlls/wined3d/utils.c | 3 +++ 2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 21239bd74b9..25c41a49c34 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -314,14 +314,14 @@ static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct
EnterCriticalSection(&device_vk->allocator_cs);
- if (size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2) + if (context_vk && size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2) { *vk_memory = wined3d_context_vk_allocate_vram_chunk_memory(context_vk, memory_type, size); LeaveCriticalSection(&device_vk->allocator_cs); return NULL; }
- if (!(block = wined3d_allocator_allocate(allocator, &context_vk->c, memory_type, size))) + if (!(block = wined3d_allocator_allocate(allocator, context_vk ? &context_vk->c : NULL, memory_type, size))) { LeaveCriticalSection(&device_vk->allocator_cs); *vk_memory = VK_NULL_HANDLE; @@ -345,13 +345,14 @@ static void wined3d_device_vk_free_memory(struct wined3d_device_vk *device_vk, s static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { - const struct wined3d_adapter_vk *adapter_vk = wined3d_adapter_vk(context_vk->c.device->adapter); + const struct wined3d_adapter_vk *adapter_vk = wined3d_adapter_vk(device_vk->d.adapter); const VkPhysicalDeviceLimits *limits = &adapter_vk->device_limits; struct wined3d_bo_slab_vk_key key; struct wined3d_bo_slab_vk *slab; struct wine_rb_entry *entry; size_t object_size, idx; size_t alignment; + int ret;
if (size > WINED3D_ALLOCATOR_MIN_BLOCK_SIZE / 2) return false; @@ -398,14 +399,8 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk } slab->map = ~0u;
- if (wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry) < 0) - { - ERR("Failed to add slab to available tree.\n"); - wined3d_context_vk_destroy_bo(context_vk, &slab->bo); - heap_free(slab); - return false; - } - + ret = wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry); + assert(!ret); TRACE("Created new bo slab %p.\n", slab); }
@@ -445,7 +440,7 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) { - const struct wined3d_vk_info *vk_info = context_vk->vk_info; + const struct wined3d_vk_info *vk_info = &device_vk->vk_info; VkMemoryRequirements memory_requirements; struct wined3d_adapter_vk *adapter_vk; VkBufferCreateInfo create_info; diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index 29326668e07..3746eff270b 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -7408,6 +7408,9 @@ struct wined3d_allocator_block *wined3d_allocator_allocate(struct wined3d_alloca return block; }
+ if (!context) + return NULL; + if (!(chunk = allocator->ops->allocator_create_chunk(allocator, context, memory_type, WINED3D_ALLOCATOR_CHUNK_SIZE))) return NULL;
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
dlls/wined3d/context_vk.c | 19 +++++++------------ dlls/wined3d/utils.c | 3 +++ 2 files changed, 10 insertions(+), 12 deletions(-)
As mentioned in 1/8 and 3/8, this should essentially never happen in the Vulkan backend.
@@ -314,14 +314,14 @@ static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct
EnterCriticalSection(&device_vk->allocator_cs);
- if (size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2)
- if (context_vk && size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2) { *vk_memory = wined3d_context_vk_allocate_vram_chunk_memory(context_vk, memory_type, size); LeaveCriticalSection(&device_vk->allocator_cs); return NULL; }
I suppose this works, but I think we should just enter this block and fail if we have a NULL context here. We're not going to successfully allocate a block larger than WINED3D_ALLOCATOR_CHUNK_SIZE / 2 below either, so there doesn't seem much point in trying.
@@ -398,14 +399,8 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk } slab->map = ~0u;
if (wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry) < 0){ERR("Failed to add slab to available tree.\n");wined3d_context_vk_destroy_bo(context_vk, &slab->bo);heap_free(slab);return false;}
ret = wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry);assert(!ret); TRACE("Created new bo slab %p.\n", slab);
It doesn't seem quite right that we'd be able to create a bo without a context, but would then be unable to destroy it again...
@@ -7408,6 +7408,9 @@ struct wined3d_allocator_block *wined3d_allocator_allocate(struct wined3d_alloca return block; }
- if (!context)
return NULL;- if (!(chunk = allocator->ops->allocator_create_chunk(allocator, context, memory_type, WINED3D_ALLOCATOR_CHUNK_SIZE))) return NULL;
Arguably, we could handle the NULL context in allocator_create_chunk(). It's somewhat moot though; in the Vulkan backend we should never have a NULL context, and in the OpenGL backend we can't do anything particularly useful with it.
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
dlls/wined3d/context_vk.c | 19 +++++++------------ dlls/wined3d/utils.c | 3 +++ 2 files changed, 10 insertions(+), 12 deletions(-)
As mentioned in 1/8 and 3/8, this should essentially never happen in the Vulkan backend.
@@ -314,14 +314,14 @@ static struct wined3d_allocator_block *wined3d_device_vk_allocate_memory(struct
EnterCriticalSection(&device_vk->allocator_cs);
- if (size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2)
- if (context_vk && size > WINED3D_ALLOCATOR_CHUNK_SIZE / 2) { *vk_memory = wined3d_context_vk_allocate_vram_chunk_memory(context_vk, memory_type, size); LeaveCriticalSection(&device_vk->allocator_cs); return NULL; }
I suppose this works, but I think we should just enter this block and fail if we have a NULL context here. We're not going to successfully allocate a block larger than WINED3D_ALLOCATOR_CHUNK_SIZE / 2 below either, so there doesn't seem much point in trying.
Right, that makes sense...
@@ -398,14 +399,8 @@ static bool wined3d_device_vk_create_slab_bo(struct wined3d_device_vk *device_vk } slab->map = ~0u;
if (wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry) < 0){ERR("Failed to add slab to available tree.\n");wined3d_context_vk_destroy_bo(context_vk, &slab->bo);heap_free(slab);return false;}
ret = wine_rb_put(&device_vk->bo_slab_available, &key, &slab->entry);assert(!ret); TRACE("Created new bo slab %p.\n", slab); }It doesn't seem quite right that we'd be able to create a bo without a context, but would then be unable to destroy it again...
No, nor did it seem quite right to me when I wrote this, but wined3d_context_vk_destroy_bo() touches more parts of the wined3d_context than I was confident about moving (plus, wine_rb_put() seems assert-worthy in general...)
I suppose the better solution here is to factor out part of wined3d_context_vk_destroy_bo(), the part that assumes the BO isn't in use by the GPU.
@@ -7408,6 +7408,9 @@ struct wined3d_allocator_block *wined3d_allocator_allocate(struct wined3d_alloca return block; }
- if (!context)
return NULL;if (!(chunk = allocator->ops->allocator_create_chunk(allocator, context, memory_type, WINED3D_ALLOCATOR_CHUNK_SIZE))) return NULL;Arguably, we could handle the NULL context in allocator_create_chunk(). It's somewhat moot though; in the Vulkan backend we should never have a NULL context, and in the OpenGL backend we can't do anything particularly useful with it.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/adapter_vk.c | 4 ++-- dlls/wined3d/buffer.c | 13 ++++++++----- dlls/wined3d/context_vk.c | 8 ++++---- dlls/wined3d/view.c | 14 ++++++++++---- dlls/wined3d/wined3d_private.h | 2 +- 5 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 020a70b4ed4..19763b49267 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -1735,7 +1735,7 @@ static void adapter_vk_draw_primitive(struct wined3d_device *device,
if (parameters->indirect) { - struct wined3d_bo_vk *bo = &indirect_vk->bo; + struct wined3d_bo_vk *bo = (struct wined3d_bo_vk *)indirect_vk->b.buffer_object; uint32_t stride, size;
wined3d_context_vk_reference_bo(context_vk, bo); @@ -1807,7 +1807,7 @@ static void adapter_vk_dispatch_compute(struct wined3d_device *device,
if (parameters->indirect) { - struct wined3d_bo_vk *bo = &indirect_vk->bo; + struct wined3d_bo_vk *bo = (struct wined3d_bo_vk *)indirect_vk->b.buffer_object;
wined3d_context_vk_reference_bo(context_vk, bo); VK_CALL(vkCmdDispatchIndirect(vk_command_buffer, bo->vk_buffer, diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 56cce00301e..99a8683df48 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1439,11 +1439,13 @@ static BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_v
const VkDescriptorBufferInfo *wined3d_buffer_vk_get_buffer_info(struct wined3d_buffer_vk *buffer_vk) { + struct wined3d_bo_vk *bo = (struct wined3d_bo_vk *)buffer_vk->b.buffer_object; + if (buffer_vk->b.bo_user.valid) return &buffer_vk->buffer_info;
- buffer_vk->buffer_info.buffer = buffer_vk->bo.vk_buffer; - buffer_vk->buffer_info.offset = buffer_vk->bo.buffer_offset; + buffer_vk->buffer_info.buffer = bo->vk_buffer; + buffer_vk->buffer_info.offset = bo->buffer_offset; buffer_vk->buffer_info.range = buffer_vk->b.resource.size; buffer_vk->b.bo_user.valid = true;
@@ -1528,7 +1530,7 @@ static void wined3d_buffer_vk_upload_ranges(struct wined3d_buffer *buffer, struc if (!ranges->offset && ranges->size == resource->size) flags |= WINED3D_MAP_DISCARD;
- dst_bo = &wined3d_buffer_vk(buffer)->bo; + dst_bo = (struct wined3d_bo_vk *)buffer->buffer_object; if (!(dst_bo->memory_type & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) || (!(flags & WINED3D_MAP_DISCARD) && dst_bo->command_buffer_id > context_vk->completed_command_buffer_id)) { @@ -1620,6 +1622,7 @@ void wined3d_buffer_vk_barrier(struct wined3d_buffer_vk *buffer_vk,
if (src_bind_mask) { + const struct wined3d_bo_vk *bo = (struct wined3d_bo_vk *)buffer_vk->b.buffer_object; const struct wined3d_vk_info *vk_info = context_vk->vk_info; VkBufferMemoryBarrier vk_barrier;
@@ -1634,8 +1637,8 @@ void wined3d_buffer_vk_barrier(struct wined3d_buffer_vk *buffer_vk, vk_barrier.dstAccessMask = vk_access_mask_from_bind_flags(bind_mask); vk_barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; vk_barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; - vk_barrier.buffer = buffer_vk->bo.vk_buffer; - vk_barrier.offset = buffer_vk->bo.buffer_offset; + vk_barrier.buffer = bo->vk_buffer; + vk_barrier.offset = bo->buffer_offset; vk_barrier.size = buffer_vk->b.resource.size; VK_CALL(vkCmdPipelineBarrier(wined3d_context_vk_get_command_buffer(context_vk), vk_pipeline_stage_mask_from_bind_flags(src_bind_mask), diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 25c41a49c34..cbd108b01f2 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -2473,7 +2473,7 @@ static void wined3d_context_vk_bind_vertex_buffers(struct wined3d_context_vk *co { buffer_vk = wined3d_buffer_vk(buffer); buffer_info = wined3d_buffer_vk_get_buffer_info(buffer_vk); - wined3d_context_vk_reference_bo(context_vk, &buffer_vk->bo); + wined3d_context_vk_reference_bo(context_vk, (struct wined3d_bo_vk *)buffer->buffer_object); buffers[count] = buffer_info->buffer; offsets[count] = buffer_info->offset + stream->offset; ++count; @@ -2512,7 +2512,7 @@ static void wined3d_context_vk_bind_stream_output_buffers(struct wined3d_context { buffer_vk = wined3d_buffer_vk(buffer); buffer_info = wined3d_buffer_vk_get_buffer_info(buffer_vk); - wined3d_context_vk_reference_bo(context_vk, &buffer_vk->bo); + wined3d_context_vk_reference_bo(context_vk, (struct wined3d_bo_vk *)buffer->buffer_object); buffers[count] = buffer_info->buffer; if ((offsets[count] = stream->offset) == ~0u) { @@ -2714,7 +2714,7 @@ static bool wined3d_shader_descriptor_writes_vk_add_cbv_write(struct wined3d_sha if (!wined3d_shader_descriptor_writes_vk_add_write(writes, vk_descriptor_set, binding->binding_idx, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, buffer_info, NULL, NULL)) return false; - wined3d_context_vk_reference_bo(context_vk, &buffer_vk->bo); + wined3d_context_vk_reference_bo(context_vk, (struct wined3d_bo_vk *)buffer->buffer_object); return true; }
@@ -3362,7 +3362,7 @@ VkCommandBuffer wined3d_context_vk_apply_draw_state(struct wined3d_context_vk *c idx_type = VK_INDEX_TYPE_UINT32; buffer_vk = wined3d_buffer_vk(state->index_buffer); buffer_info = wined3d_buffer_vk_get_buffer_info(buffer_vk); - wined3d_context_vk_reference_bo(context_vk, &buffer_vk->bo); + wined3d_context_vk_reference_bo(context_vk, (struct wined3d_bo_vk *)buffer_vk->b.buffer_object); VK_CALL(vkCmdBindIndexBuffer(vk_command_buffer, buffer_info->buffer, buffer_info->offset + state->index_offset, idx_type)); } diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index 6a88286b861..5018899e181 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -672,17 +672,19 @@ static VkBufferView wined3d_view_vk_create_vk_buffer_view(struct wined3d_context struct wined3d_device_vk *device_vk; VkBufferView vk_buffer_view; unsigned int offset, size; + struct wined3d_bo_vk *bo; VkResult vr;
get_buffer_view_range(&buffer_vk->b, desc, &view_format_vk->f, &offset, &size); wined3d_buffer_prepare_location(&buffer_vk->b, &context_vk->c, WINED3D_LOCATION_BUFFER); + bo = (struct wined3d_bo_vk *)buffer_vk->b.buffer_object;
create_info.sType = VK_STRUCTURE_TYPE_BUFFER_VIEW_CREATE_INFO; create_info.pNext = NULL; create_info.flags = 0; - create_info.buffer = buffer_vk->bo.vk_buffer; + create_info.buffer = bo->vk_buffer; create_info.format = view_format_vk->vk_format; - create_info.offset = buffer_vk->bo.buffer_offset + offset; + create_info.offset = bo->buffer_offset + offset; create_info.range = size;
device_vk = wined3d_device_vk(buffer_vk->b.resource.device); @@ -1082,6 +1084,7 @@ static void wined3d_shader_resource_view_vk_cs_init(void *object) VkBufferView vk_buffer_view; uint32_t default_flags = 0; VkImageView vk_image_view; + struct wined3d_bo_vk *bo;
TRACE("srv_vk %p.\n", srv_vk);
@@ -1099,12 +1102,13 @@ static void wined3d_shader_resource_view_vk_cs_init(void *object)
if (!vk_buffer_view) return; + bo = (struct wined3d_bo_vk *)buffer_vk->b.buffer_object;
TRACE("Created buffer view 0x%s.\n", wine_dbgstr_longlong(vk_buffer_view));
srv_vk->view_vk.u.vk_buffer_view = vk_buffer_view; srv_vk->view_vk.bo_user.valid = true; - list_add_head(&buffer_vk->bo.b.users, &srv_vk->view_vk.bo_user.entry); + list_add_head(&bo->b.users, &srv_vk->view_vk.bo_user.entry);
return; } @@ -2204,11 +2208,13 @@ static void wined3d_unordered_access_view_vk_cs_init(void *object)
if ((vk_buffer_view = wined3d_view_vk_create_vk_buffer_view(context_vk, desc, buffer_vk, format_vk))) { + struct wined3d_bo_vk *bo = (struct wined3d_bo_vk *)buffer_vk->b.buffer_object; + TRACE("Created buffer view 0x%s.\n", wine_dbgstr_longlong(vk_buffer_view));
uav_vk->view_vk.u.vk_buffer_view = vk_buffer_view; uav_vk->view_vk.bo_user.valid = true; - list_add_head(&buffer_vk->bo.b.users, &view_vk->bo_user.entry); + list_add_head(&bo->b.users, &view_vk->bo_user.entry); }
if (desc->flags & (WINED3D_VIEW_BUFFER_COUNTER | WINED3D_VIEW_BUFFER_APPEND)) diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 21b4ef2e103..fab1c755a81 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -6289,7 +6289,7 @@ static inline void wined3d_context_vk_reference_resource(const struct wined3d_co struct wined3d_resource *resource) { if (resource->type == WINED3D_RTYPE_BUFFER) - wined3d_context_vk_reference_bo(context_vk, &wined3d_buffer_vk(buffer_from_resource(resource))->bo); + wined3d_context_vk_reference_bo(context_vk, (struct wined3d_bo_vk *)buffer_from_resource(resource)->buffer_object); else wined3d_context_vk_reference_texture(context_vk, wined3d_texture_vk(texture_from_resource(resource))); }
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
dlls/wined3d/adapter_vk.c | 4 ++-- dlls/wined3d/buffer.c | 13 ++++++++----- dlls/wined3d/context_vk.c | 8 ++++---- dlls/wined3d/view.c | 14 ++++++++++---- dlls/wined3d/wined3d_private.h | 2 +- 5 files changed, 25 insertions(+), 16 deletions(-)
Why is that? I gather it's related to handling the wined3d_client_bo_vk structure from patch 8/8, but I don't think it's used enough in this series to get the full picture. In any case, that kind of information is helpful to have in the commit message.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/wined3d/adapter_gl.c | 7 ++++ dlls/wined3d/adapter_vk.c | 57 +++++++++++++++++++++++++++++++++ dlls/wined3d/buffer.c | 58 +++++++++++++++++++++++++++++++++- dlls/wined3d/cs.c | 32 +++++++++++++++---- dlls/wined3d/directx.c | 7 ++++ dlls/wined3d/wined3d_private.h | 19 +++++++++++ 6 files changed, 173 insertions(+), 7 deletions(-)
diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index f1cdbffe718..37f055662ea 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -4604,6 +4604,12 @@ static void adapter_gl_flush_bo_address(struct wined3d_context *context, wined3d_context_gl_flush_bo_address(wined3d_context_gl(context), data, size); }
+static bool adapter_gl_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_bo_address *addr) +{ + return false; +} + static HRESULT adapter_gl_create_swapchain(struct wined3d_device *device, struct wined3d_swapchain_desc *desc, struct wined3d_swapchain_state_parent *state_parent, void *parent, const struct wined3d_parent_ops *parent_ops, struct wined3d_swapchain **swapchain) @@ -5057,6 +5063,7 @@ static const struct wined3d_adapter_ops wined3d_adapter_gl_ops = .adapter_unmap_bo_address = adapter_gl_unmap_bo_address, .adapter_copy_bo_address = adapter_gl_copy_bo_address, .adapter_flush_bo_address = adapter_gl_flush_bo_address, + .adapter_alloc_bo = adapter_gl_alloc_bo, .adapter_create_swapchain = adapter_gl_create_swapchain, .adapter_destroy_swapchain = adapter_gl_destroy_swapchain, .adapter_create_buffer = adapter_gl_create_buffer, diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 19763b49267..002d9e07a00 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -23,6 +23,7 @@ #include "wine/vulkan_driver.h"
WINE_DEFAULT_DEBUG_CHANNEL(d3d); +WINE_DECLARE_DEBUG_CHANNEL(d3d_perf);
static const struct wined3d_state_entry_template misc_state_template_vk[] = { @@ -1205,6 +1206,61 @@ static void adapter_vk_flush_bo_address(struct wined3d_context *context, flush_bo_range(context_vk, bo, (uintptr_t)data->addr, size); }
+struct wined3d_client_bo_vk_map_ctx +{ + struct wined3d_device *device; + struct wined3d_client_bo_vk *client_bo; +}; + +static void wined3d_client_bo_vk_map_cs(void *object) +{ + const struct wined3d_client_bo_vk_map_ctx *ctx = object; + struct wined3d_context_vk *context_vk; + + context_vk = wined3d_context_vk(context_acquire(ctx->device, NULL, 0)); + if (!wined3d_bo_vk_map(&ctx->client_bo->bo, context_vk)) + ERR("Failed to map bo.\n"); + context_release(&context_vk->c); +} + +static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_bo_address *addr) +{ + wined3d_not_from_cs(device->cs); + + if (resource->type == WINED3D_RTYPE_BUFFER) + { + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource)); + struct wined3d_client_bo_vk *client_bo; + + if (!(client_bo = heap_alloc(sizeof(*client_bo)))) + return false; + + if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)) + { + heap_free(client_bo); + return false; + } + + if (!client_bo->bo.b.map_ptr) + { + struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo}; + + WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo, + client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab); + + wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx); + wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP); + } + + addr->buffer_object = (uintptr_t)&client_bo->bo; + addr->addr = NULL; + return true; + } + + return false; +} + static HRESULT adapter_vk_create_swapchain(struct wined3d_device *device, struct wined3d_swapchain_desc *desc, struct wined3d_swapchain_state_parent *state_parent, void *parent, const struct wined3d_parent_ops *parent_ops, struct wined3d_swapchain **swapchain) @@ -1858,6 +1914,7 @@ static const struct wined3d_adapter_ops wined3d_adapter_vk_ops = .adapter_unmap_bo_address = adapter_vk_unmap_bo_address, .adapter_copy_bo_address = adapter_vk_copy_bo_address, .adapter_flush_bo_address = adapter_vk_flush_bo_address, + .adapter_alloc_bo = adapter_vk_alloc_bo, .adapter_create_swapchain = adapter_vk_create_swapchain, .adapter_destroy_swapchain = adapter_vk_destroy_swapchain, .adapter_create_buffer = adapter_vk_create_buffer, diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 99a8683df48..fe7debfd92d 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1061,6 +1061,16 @@ static HRESULT buffer_resource_sub_resource_unmap(struct wined3d_resource *resou return WINED3D_OK; }
+void wined3d_buffer_rename(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) +{ + TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo); + + buffer->buffer_ops->buffer_rename_bo(buffer, context, bo); + buffer->buffer_object = (uintptr_t)bo; + wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER); +} + 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) { @@ -1256,12 +1266,19 @@ static void wined3d_buffer_no3d_download_ranges(struct wined3d_buffer *buffer, s FIXME("Not implemented.\n"); }
+static void wined3d_buffer_no3d_rename_bo(struct wined3d_buffer *buffer, + struct wined3d_context *context, struct wined3d_bo *bo) +{ + FIXME("Not implemented.\n"); +} + static const struct wined3d_buffer_ops wined3d_buffer_no3d_ops = { wined3d_buffer_no3d_prepare_location, wined3d_buffer_no3d_unload_location, wined3d_buffer_no3d_upload_ranges, wined3d_buffer_no3d_download_ranges, + wined3d_buffer_no3d_rename_bo, };
HRESULT wined3d_buffer_no3d_init(struct wined3d_buffer *buffer_no3d, struct wined3d_device *device, @@ -1366,12 +1383,19 @@ static void wined3d_buffer_gl_download_ranges(struct wined3d_buffer *buffer, str checkGLcall("buffer download"); }
+static void wined3d_buffer_gl_rename_bo(struct wined3d_buffer *buffer, + struct wined3d_context *context, struct wined3d_bo *bo) +{ + FIXME("Not implemented.\n"); +} + static const struct wined3d_buffer_ops wined3d_buffer_gl_ops = { wined3d_buffer_gl_prepare_location, wined3d_buffer_gl_unload_location, wined3d_buffer_gl_upload_ranges, wined3d_buffer_gl_download_ranges, + wined3d_buffer_gl_rename_bo, };
HRESULT wined3d_buffer_gl_init(struct wined3d_buffer_gl *buffer_gl, struct wined3d_device *device, @@ -1399,7 +1423,7 @@ HRESULT wined3d_buffer_gl_init(struct wined3d_buffer_gl *buffer_gl, struct wined return wined3d_buffer_init(&buffer_gl->b, device, desc, data, parent, parent_ops, &wined3d_buffer_gl_ops); }
-static BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk, +BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk, struct wined3d_context_vk *context_vk, struct wined3d_bo_vk *bo) { const struct wined3d_resource *resource = &buffer_vk->b.resource; @@ -1568,12 +1592,44 @@ static void wined3d_buffer_vk_download_ranges(struct wined3d_buffer *buffer, str FIXME("Not implemented.\n"); }
+static void wined3d_buffer_vk_rename_bo(struct wined3d_buffer *buffer, + struct wined3d_context *context, struct wined3d_bo *bo) +{ + struct wined3d_bo_vk *prev_bo = (struct wined3d_bo_vk *)buffer->buffer_object; + struct wined3d_context_vk *context_vk = wined3d_context_vk(context); + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer); + struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo); + + if (prev_bo) + { + struct wined3d_bo_user *bo_user; + + LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry) + bo_user->valid = false; + assert(list_empty(&bo_vk->b.users)); + list_move_head(&bo_vk->b.users, &prev_bo->b.users); + + if (prev_bo != &buffer_vk->bo) + { + struct wined3d_client_bo_vk *client_bo = CONTAINING_RECORD(prev_bo, struct wined3d_client_bo_vk, bo); + + wined3d_context_vk_destroy_bo(context_vk, &client_bo->bo); + heap_free(client_bo); + } + } + else + { + list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry); + } +} + static const struct wined3d_buffer_ops wined3d_buffer_vk_ops = { wined3d_buffer_vk_prepare_location, wined3d_buffer_vk_unload_location, wined3d_buffer_vk_upload_ranges, wined3d_buffer_vk_download_ranges, + wined3d_buffer_vk_rename_bo, };
HRESULT wined3d_buffer_vk_init(struct wined3d_buffer_vk *buffer_vk, struct wined3d_device *device, diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index e6ff448b028..6978e6bf3cb 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -2304,26 +2304,32 @@ static void wined3d_cs_exec_callback(struct wined3d_cs *cs, const void *data) op->callback(op->object); }
-static void wined3d_cs_emit_callback(struct wined3d_cs *cs, void (*callback)(void *object), void *object) +static void wined3d_cs_emit_callback(struct wined3d_cs *cs, enum wined3d_cs_queue_id queue_id, + void (*callback)(void *object), void *object) { struct wined3d_cs_callback *op;
- op = wined3d_device_context_require_space(&cs->c, sizeof(*op), WINED3D_CS_QUEUE_DEFAULT); + op = wined3d_device_context_require_space(&cs->c, sizeof(*op), queue_id); op->opcode = WINED3D_CS_OP_CALLBACK; op->callback = callback; op->object = object;
- wined3d_device_context_submit(&cs->c, WINED3D_CS_QUEUE_DEFAULT); + wined3d_device_context_submit(&cs->c, queue_id); }
void wined3d_cs_destroy_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) { - wined3d_cs_emit_callback(cs, callback, object); + wined3d_cs_emit_callback(cs, WINED3D_CS_QUEUE_DEFAULT, callback, object); }
void wined3d_cs_init_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) { - wined3d_cs_emit_callback(cs, callback, object); + wined3d_cs_emit_callback(cs, WINED3D_CS_QUEUE_DEFAULT, callback, object); +} + +void wined3d_cs_map_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) +{ + wined3d_cs_emit_callback(cs, WINED3D_CS_QUEUE_MAP, callback, object); }
static void wined3d_cs_exec_query_issue(struct wined3d_cs *cs, const void *data) @@ -2747,6 +2753,9 @@ static void wined3d_cs_exec_update_sub_resource(struct wined3d_cs *cs, const voi struct wined3d_buffer *buffer = buffer_from_resource(resource); size_t size = box->right - box->left;
+ if (op->bo.flags & UPLOAD_BO_RENAME_ON_UNMAP) + wined3d_buffer_rename(buffer, context, (struct wined3d_bo *)op->bo.addr.buffer_object); + if (op->bo.addr.buffer_object && op->bo.addr.buffer_object == buffer->buffer_object) wined3d_context_flush_bo_address(context, &op->bo.addr, size); else @@ -3121,12 +3130,20 @@ static bool wined3d_cs_map_upload_bo(struct wined3d_device_context *context, str { /* Limit NOOVERWRITE maps to buffers for now; there are too many ways that * a texture can be invalidated to even count. */ - if (wined3d_map_persistent() && resource->type == WINED3D_RTYPE_BUFFER && (flags & WINED3D_MAP_NOOVERWRITE)) + if (wined3d_map_persistent() && resource->type == WINED3D_RTYPE_BUFFER + && (flags & (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE))) { struct wined3d_client_resource *client = &resource->client; + struct wined3d_device *device = context->device; const struct wined3d_bo *bo; uint8_t *map_ptr;
+ if (flags & WINED3D_MAP_DISCARD) + { + if (!device->adapter->adapter_ops->adapter_alloc_bo(device, resource, sub_resource_idx, &client->addr)) + return NULL; + } + bo = (const struct wined3d_bo *)client->addr.buffer_object; map_ptr = bo ? bo->map_ptr : NULL; map_ptr += (uintptr_t)client->addr.addr; @@ -3150,6 +3167,9 @@ static bool wined3d_cs_map_upload_bo(struct wined3d_device_context *context, str } map_desc->data = resource_offset_map_pointer(resource, sub_resource_idx, map_ptr, box);
+ if (flags & WINED3D_MAP_DISCARD) + client->mapped_upload.flags |= UPLOAD_BO_UPLOAD_ON_UNMAP | UPLOAD_BO_RENAME_ON_UNMAP; + client->mapped_box = *box;
TRACE("Returning bo %s, flags %#x.\n", debug_const_bo_address(&client->mapped_upload.addr), diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c index c265bdc8c95..f14f57fd3f4 100644 --- a/dlls/wined3d/directx.c +++ b/dlls/wined3d/directx.c @@ -2805,6 +2805,12 @@ static void adapter_no3d_flush_bo_address(struct wined3d_context *context, { }
+static bool adapter_no3d_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_bo_address *addr) +{ + return false; +} + static HRESULT adapter_no3d_create_swapchain(struct wined3d_device *device, struct wined3d_swapchain_desc *desc, struct wined3d_swapchain_state_parent *state_parent, void *parent, const struct wined3d_parent_ops *parent_ops, struct wined3d_swapchain **swapchain) @@ -3075,6 +3081,7 @@ static const struct wined3d_adapter_ops wined3d_adapter_no3d_ops = .adapter_unmap_bo_address = adapter_no3d_unmap_bo_address, .adapter_copy_bo_address = adapter_no3d_copy_bo_address, .adapter_flush_bo_address = adapter_no3d_flush_bo_address, + .adapter_alloc_bo = adapter_no3d_alloc_bo, .adapter_create_swapchain = adapter_no3d_create_swapchain, .adapter_destroy_swapchain = adapter_no3d_destroy_swapchain, .adapter_create_buffer = adapter_no3d_create_buffer, diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index fab1c755a81..568eebb5ace 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -1639,6 +1639,16 @@ struct wined3d_bo_vk bool host_synced; };
+static inline struct wined3d_bo_vk *wined3d_bo_vk(struct wined3d_bo *bo) +{ + return CONTAINING_RECORD(bo, struct wined3d_bo_vk, b); +} + +struct wined3d_client_bo_vk +{ + struct wined3d_bo_vk bo; +}; + struct wined3d_bo_slab_vk_key { VkMemoryPropertyFlags memory_type; @@ -3335,6 +3345,7 @@ bool wined3d_driver_info_init(struct wined3d_driver_info *driver_info, UINT64 vram_bytes, UINT64 sysmem_bytes) DECLSPEC_HIDDEN;
#define UPLOAD_BO_UPLOAD_ON_UNMAP 0x1 +#define UPLOAD_BO_RENAME_ON_UNMAP 0x2
struct upload_bo { @@ -3367,6 +3378,8 @@ struct wined3d_adapter_ops const struct wined3d_bo_address *dst, const struct wined3d_bo_address *src, size_t size); void (*adapter_flush_bo_address)(struct wined3d_context *context, const struct wined3d_const_bo_address *data, size_t size); + bool (*adapter_alloc_bo)(struct wined3d_device *device, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_bo_address *addr); HRESULT (*adapter_create_swapchain)(struct wined3d_device *device, struct wined3d_swapchain_desc *desc, struct wined3d_swapchain_state_parent *state_parent, void *parent, @@ -4874,6 +4887,7 @@ void wined3d_cs_emit_set_render_state(struct wined3d_cs *cs, void wined3d_cs_emit_unload_resource(struct wined3d_cs *cs, struct wined3d_resource *resource) DECLSPEC_HIDDEN; void wined3d_cs_init_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) DECLSPEC_HIDDEN; +void wined3d_cs_map_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) DECLSPEC_HIDDEN;
static inline void wined3d_cs_finish(struct wined3d_cs *cs, enum wined3d_cs_queue_id queue_id) { @@ -5002,6 +5016,7 @@ struct wined3d_buffer_ops unsigned int data_offset, unsigned int range_count, const struct wined3d_range *ranges); void (*buffer_download_ranges)(struct wined3d_buffer *buffer, struct wined3d_context *context, void *data, unsigned int data_offset, unsigned int range_count, const struct wined3d_range *ranges); + void (*buffer_rename_bo)(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo); };
struct wined3d_buffer @@ -5047,6 +5062,8 @@ BOOL wined3d_buffer_load_location(struct wined3d_buffer *buffer, BYTE *wined3d_buffer_load_sysmem(struct wined3d_buffer *buffer, struct wined3d_context *context) DECLSPEC_HIDDEN; BOOL wined3d_buffer_prepare_location(struct wined3d_buffer *buffer, struct wined3d_context *context, unsigned int location) DECLSPEC_HIDDEN; +void wined3d_buffer_rename(struct wined3d_buffer *buffer, + struct wined3d_context *context, struct wined3d_bo *bo) DECLSPEC_HIDDEN;
HRESULT wined3d_buffer_no3d_init(struct wined3d_buffer *buffer_no3d, struct wined3d_device *device, const struct wined3d_buffer_desc *desc, const struct wined3d_sub_resource_data *data, @@ -5089,6 +5106,8 @@ static inline struct wined3d_buffer_vk *wined3d_buffer_vk(struct wined3d_buffer
void wined3d_buffer_vk_barrier(struct wined3d_buffer_vk *buffer_vk, struct wined3d_context_vk *context_vk, uint32_t bind_mask) DECLSPEC_HIDDEN; +BOOL wined3d_buffer_vk_create_buffer_object(const struct wined3d_buffer_vk *buffer_vk, + struct wined3d_context_vk *context_vk, struct wined3d_bo_vk *bo) DECLSPEC_HIDDEN; const VkDescriptorBufferInfo *wined3d_buffer_vk_get_buffer_info(struct wined3d_buffer_vk *buffer_vk) DECLSPEC_HIDDEN; HRESULT wined3d_buffer_vk_init(struct wined3d_buffer_vk *buffer_vk, struct wined3d_device *device, const struct wined3d_buffer_desc *desc, const struct wined3d_sub_resource_data *data,
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}
wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()... Here too, Vulkan could map from application threads, it just needs to be synchronised. In the general case we'd need to make sure the GPU isn't still using the bo, but that's not an issue for newly allocated bo's.
+void wined3d_buffer_rename(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) +{
- TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo);
- buffer->buffer_ops->buffer_rename_bo(buffer, context, bo);
- buffer->buffer_object = (uintptr_t)bo;
- wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER);
- wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER);
+}
If we're doing this at the wined3d_buffer level, perhaps it would make sense to name this something like wined3d_buffer_set_bo(), wined3d_buffer_assign_bo(), or something along those lines.
+static void wined3d_buffer_vk_rename_bo(struct wined3d_buffer *buffer,
struct wined3d_context *context, struct wined3d_bo *bo)+{
- struct wined3d_bo_vk *prev_bo = (struct wined3d_bo_vk *)buffer->buffer_object;
- struct wined3d_context_vk *context_vk = wined3d_context_vk(context);
- struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer);
- struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo);
- if (prev_bo)
- {
struct wined3d_bo_user *bo_user;LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry)bo_user->valid = false;assert(list_empty(&bo_vk->b.users));list_move_head(&bo_vk->b.users, &prev_bo->b.users);if (prev_bo != &buffer_vk->bo){struct wined3d_client_bo_vk *client_bo = CONTAINING_RECORD(prev_bo, struct wined3d_client_bo_vk, bo);wined3d_context_vk_destroy_bo(context_vk, &client_bo->bo);heap_free(client_bo);}- }
- else
- {
list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry);- }
+}
I admit I didn't try very hard, but it's not immediately clear to me why we wouldn't always assign the new bo to "buffer_vk->bo" here, and mark the previous one for destruction.
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
(Now that I think about it, *is* there any sort of a rule as to what deserves WINED3D_CS_OP_CALLBACK and what doesn't? I know that currently it's just a matter of init and destroy, but it seeems a bit arbitrary on reflection.)
Here too, Vulkan could map from application threads, it just needs to be synchronised. In the general case we'd need to make sure the GPU isn't still using the bo, but that's not an issue for newly allocated bo's.
+void wined3d_buffer_rename(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) +{
- TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo);
- buffer->buffer_ops->buffer_rename_bo(buffer, context, bo);
- buffer->buffer_object = (uintptr_t)bo;
- wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER);
- wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER);
+}
If we're doing this at the wined3d_buffer level, perhaps it would make sense to name this something like wined3d_buffer_set_bo(), wined3d_buffer_assign_bo(), or something along those lines.
+static void wined3d_buffer_vk_rename_bo(struct wined3d_buffer *buffer,
struct wined3d_context *context, struct wined3d_bo *bo)+{
- struct wined3d_bo_vk *prev_bo = (struct wined3d_bo_vk *)buffer->buffer_object;
- struct wined3d_context_vk *context_vk = wined3d_context_vk(context);
- struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer);
- struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo);
- if (prev_bo)
- {
struct wined3d_bo_user *bo_user;LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry)bo_user->valid = false;assert(list_empty(&bo_vk->b.users));list_move_head(&bo_vk->b.users, &prev_bo->b.users);if (prev_bo != &buffer_vk->bo){struct wined3d_client_bo_vk *client_bo = CONTAINING_RECORD(prev_bo, struct wined3d_client_bo_vk, bo);wined3d_context_vk_destroy_bo(context_vk, &client_bo->bo);heap_free(client_bo);}- }
- else
- {
list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry);- }
+}
I admit I didn't try very hard, but it's not immediately clear to me why we wouldn't always assign the new bo to "buffer_vk->bo" here, and mark the previous one for destruction.
Right, the basic reason is that the client thread might still be holding a pointer to "bo_vk" in the wined3d_client_resource structure. I'll add a comment to that effect...
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
(Now that I think about it, *is* there any sort of a rule as to what deserves WINED3D_CS_OP_CALLBACK and what doesn't? I know that currently it's just a matter of init and destroy, but it seeems a bit arbitrary on reflection.)
For the most part, it's things that I don't think should have their own wined3d_cs_op. WINED3D_CS_OP_CALLBACK is a little ugly, but it works well enough for the moment. I suppose we could do something along the lines of the following:
struct wined3d_cs_resource_ops { void (*cs_resource_init)(struct wined3d_cs_resource *resource); void (*cs_resource_cleanup)(struct wined3d_cs_resource *resource); };
struct wined3d_cs_resource { struct wined3d_cs_resource_ops *ops; };
and then
struct wined3d_cs_init_resource { enum wined3d_cs_op opcode; struct wined3d_cs_resource *resource; };
static void wined3d_cs_exec_init_resource(struct wined3d_cs *cs, const void *data) { const struct wined3d_cs_init_resource *op = data;
op->resource->ops->cs_resource_init(op->resource); }
and equivalent cleanup variants. Something like wined3d_sampler_vk would then include a wined3d_cs_resource structure, and call "wined3d_cs_init_object(device->cs, &sampler_vk->cs_resource);". That ends up being largely equivalent to the current scheme, although perhaps a little more structured.
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
(Now that I think about it, *is* there any sort of a rule as to what deserves WINED3D_CS_OP_CALLBACK and what doesn't? I know that currently it's just a matter of init and destroy, but it seeems a bit arbitrary on reflection.)
For the most part, it's things that I don't think should have their own wined3d_cs_op. WINED3D_CS_OP_CALLBACK is a little ugly, but it works well enough for the moment. I suppose we could do something along the lines of the following:
struct wined3d_cs_resource_ops { void (*cs_resource_init)(struct wined3d_cs_resource *resource); void (*cs_resource_cleanup)(struct wined3d_cs_resource *resource); }; struct wined3d_cs_resource { struct wined3d_cs_resource_ops *ops; };and then
struct wined3d_cs_init_resource { enum wined3d_cs_op opcode; struct wined3d_cs_resource *resource; }; static void wined3d_cs_exec_init_resource(struct wined3d_cs *cs,const void *data) { const struct wined3d_cs_init_resource *op = data;
op->resource->ops->cs_resource_init(op->resource); }and equivalent cleanup variants. Something like wined3d_sampler_vk would then include a wined3d_cs_resource structure, and call "wined3d_cs_init_object(device->cs, &sampler_vk->cs_resource);". That ends up being largely equivalent to the current scheme, although perhaps a little more structured.
Seems reasonable.
On Thu, Nov 4, 2021 at 4:45 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
On 11/4/21 11:19 AM, Matteo Bruni wrote:
On Thu, Nov 4, 2021 at 4:45 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
For Vulkan I think it doesn't matter, since we can just map from the client thread. For GL we have to to map from the CS thread, but if we just map the old resource via wined3d_resource_map(), we'll use &wined3d_buffer_gl.bo instead of allocating new memory, which means that the client will continue to have no accessible memory to return for a discard map. Repeat ad infinitum.
Allocating sysmem would help, but my understanding is that expanding the available GPU memory pool would be better.
That's the theoretical problem; I don't remember if it caused problems in practice, but I have a suspicion that it did...
On Thu, 4 Nov 2021 at 17:31, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 11:19 AM, Matteo Bruni wrote:
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
For Vulkan I think it doesn't matter, since we can just map from the client thread. For GL we have to to map from the CS thread, but if we just map the old resource via wined3d_resource_map(), we'll use &wined3d_buffer_gl.bo instead of allocating new memory, which means that the client will continue to have no accessible memory to return for a discard map. Repeat ad infinitum.
Allocating sysmem would help, but my understanding is that expanding the available GPU memory pool would be better.
Yeah. My idea for that, although I never worked it out all the way, would be to keep a mapped bo for uploads on the application side of the CS. Then if that runs low, we'd send a request through the CS to allocate more, but without waiting for that to complete. Ideally that request would then have completed before the original upload space actually runs out. If we did run out though, we'd use a CPU allocation to avoid stalling those requests. I.e., the basic premise being that we'd like to avoid stalling even for those requests.
On Thu, Nov 4, 2021 at 5:49 PM Henri Verbeet hverbeet@gmail.com wrote:
On Thu, 4 Nov 2021 at 17:31, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 11:19 AM, Matteo Bruni wrote:
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
For Vulkan I think it doesn't matter, since we can just map from the client thread. For GL we have to to map from the CS thread, but if we just map the old resource via wined3d_resource_map(), we'll use &wined3d_buffer_gl.bo instead of allocating new memory, which means that the client will continue to have no accessible memory to return for a discard map. Repeat ad infinitum.
Allocating sysmem would help, but my understanding is that expanding the available GPU memory pool would be better.
Yeah. My idea for that, although I never worked it out all the way, would be to keep a mapped bo for uploads on the application side of the CS. Then if that runs low, we'd send a request through the CS to allocate more, but without waiting for that to complete. Ideally that request would then have completed before the original upload space actually runs out. If we did run out though, we'd use a CPU allocation to avoid stalling those requests. I.e., the basic premise being that we'd like to avoid stalling even for those requests.
Yeah, that sounds great in principle.
On Thu, Nov 4, 2021 at 5:31 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 11:19 AM, Matteo Bruni wrote:
On Thu, Nov 4, 2021 at 4:45 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote: > +static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource, > + unsigned int sub_resource_idx, struct wined3d_bo_address *addr) > +{ > + wined3d_not_from_cs(device->cs); > + > + if (resource->type == WINED3D_RTYPE_BUFFER) > + { > + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource)); > + struct wined3d_client_bo_vk *client_bo; > + > + if (!(client_bo = heap_alloc(sizeof(*client_bo)))) > + return false; > + > + if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)) > + { > + heap_free(client_bo); > + return false; > + } > + > + if (!client_bo->bo.b.map_ptr) > + { > + struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo}; > + > + WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo, > + client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab); > + > + wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx); > + wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP); > + } wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
For Vulkan I think it doesn't matter, since we can just map from the client thread. For GL we have to to map from the CS thread, but if we just map the old resource via wined3d_resource_map(), we'll use &wined3d_buffer_gl.bo instead of allocating new memory, which means that the client will continue to have no accessible memory to return for a discard map. Repeat ad infinitum.
Right, I guess the problem is that our "CS api", so to speak, works with wined3d buffers, which match d3d resources. That's really not what we would want in an ideal world.
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
This would mainly be for the wined3d_buffer_vk_create_buffer_object() call in patch 8/8, but in the Vulkan backend, we can always get the context through "device_vk->context_vk", provided we're inside an init_3d()/uninit_3d() pair. I.e., the Vulkan backend can make Vulkan calls from applications threads, it just needs synchronisation. I.e., I don't think the Vulkan backend needs this.
That said, I still think this change is probably the right thing to do, but with a bit of a different justification. We'll likely need a change like this for wined3d_context_gl_create_bo(), and then it makes sense to make wined3d_context_vk_create_bo() a device function as well for consistency.
-static bool wined3d_context_vk_create_slab_bo(struct wined3d_context_vk *context_vk, +static bool wined3d_context_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) {
If we're making this a device function, that should probably be called wined3d_device_vk_create_slab_bo(), and be moved to device.c.
-BOOL wined3d_context_vk_create_bo(struct wined3d_context_vk *context_vk, VkDeviceSize size,
VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo)+BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk,
VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo){
If we're making this a device function, this should be moved to device.c. And if we're touching this, be may as well use the standard "bool" instead of "BOOL".
On 11/3/21 12:10 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
This would mainly be for the wined3d_buffer_vk_create_buffer_object() call in patch 8/8, but in the Vulkan backend, we can always get the context through "device_vk->context_vk", provided we're inside an init_3d()/uninit_3d() pair. I.e., the Vulkan backend can make Vulkan calls from applications threads, it just needs synchronisation. I.e., I don't think the Vulkan backend needs this.
It's true that this patch series is modeled more like the GL backend than it needs to be in general.
I had gotten the impression that we didn't want to touch wined3d_context_vk outside of the CS thread, even though it is thread safe. If that's not the case, all the better, although I'm still more than a little confused about the seemingly arbitrary division between device and context in the Vulkan adapter.
That said, I still think this change is probably the right thing to do, but with a bit of a different justification. We'll likely need a change like this for wined3d_context_gl_create_bo(), and then it makes sense to make wined3d_context_vk_create_bo() a device function as well for consistency.
-static bool wined3d_context_vk_create_slab_bo(struct wined3d_context_vk *context_vk, +static bool wined3d_context_vk_create_slab_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk, VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo) {
If we're making this a device function, that should probably be called wined3d_device_vk_create_slab_bo(), and be moved to device.c.
-BOOL wined3d_context_vk_create_bo(struct wined3d_context_vk *context_vk, VkDeviceSize size,
VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo)+BOOL wined3d_device_vk_create_bo(struct wined3d_device_vk *device_vk, struct wined3d_context_vk *context_vk,
VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags memory_type, struct wined3d_bo_vk *bo)If we're making this a device function, this should be moved to device.c. And if we're touching this, be may as well use the standard "bool" instead of "BOOL".
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:10 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
This would mainly be for the wined3d_buffer_vk_create_buffer_object() call in patch 8/8, but in the Vulkan backend, we can always get the context through "device_vk->context_vk", provided we're inside an init_3d()/uninit_3d() pair. I.e., the Vulkan backend can make Vulkan calls from applications threads, it just needs synchronisation. I.e., I don't think the Vulkan backend needs this.
It's true that this patch series is modeled more like the GL backend than it needs to be in general.
Which, to be clear, is largely fine with me, as long as the justification is something along the lines of "consistency with the GL backend", instead of "required to do maps with a NULL context".
I had gotten the impression that we didn't want to touch wined3d_context_vk outside of the CS thread, even though it is thread safe. If that's not the case, all the better, although I'm still more than a little confused about the seemingly arbitrary division between device and context in the Vulkan adapter.
It's true that it's largely arbitrary; for the purposes of the Vulkan backend, the device and the context could/should be considered to be the same object. (And perhaps significantly, within the Vulkan backend you can get a wined3d_device_vk from a wined3d_context using CONTAINING_RECORD.) If you feel strongly about this being confusing, I'd certainly be fine with just getting rid of wined3d_context_vk, and moving everything to wined3d_device_vk. That said, there are broadly two considerations for things ending up in one or the other structure:
- Things that have direct equivalents in the OpenGL backend generally are in the corresponding structure. E.g., "null_resources_vk" and "null_views_vk" in wined3d_device_vk are equivalents of "dummy_textures" in wined3d_device_gl; "free_*_query_pools" in wined3d_context_vk are equivalents of "free_*_queries" in wined3d_context_gl. - Things that mostly get accessed from context functions are in wined3d_context_vk. E.g., "vk_framebuffer" is in that category. That is itself somewhat influenced by equivalency to things in the GL backend; wined3d_context_vk_apply_draw_state() is an equivalent of context_(gl_)apply_draw_state().
The basic idea was that it would make it easier to find things for people already familiar with the GL backend; it does mean some things are somewhat arbitrary when considering the Vulkan backend on its own.
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:10 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:20, Zebediah Figura zfigura@codeweavers.com wrote:
With the goal of making the context_vk parameter optional, so that we can allocate new BOs from the client thread.
This would mainly be for the wined3d_buffer_vk_create_buffer_object() call in patch 8/8, but in the Vulkan backend, we can always get the context through "device_vk->context_vk", provided we're inside an init_3d()/uninit_3d() pair. I.e., the Vulkan backend can make Vulkan calls from applications threads, it just needs synchronisation. I.e., I don't think the Vulkan backend needs this.
It's true that this patch series is modeled more like the GL backend than it needs to be in general.
Which, to be clear, is largely fine with me, as long as the justification is something along the lines of "consistency with the GL backend", instead of "required to do maps with a NULL context".
I had gotten the impression that we didn't want to touch wined3d_context_vk outside of the CS thread, even though it is thread safe. If that's not the case, all the better, although I'm still more than a little confused about the seemingly arbitrary division between device and context in the Vulkan adapter.
It's true that it's largely arbitrary; for the purposes of the Vulkan backend, the device and the context could/should be considered to be the same object. (And perhaps significantly, within the Vulkan backend you can get a wined3d_device_vk from a wined3d_context using CONTAINING_RECORD.) If you feel strongly about this being confusing, I'd certainly be fine with just getting rid of wined3d_context_vk, and moving everything to wined3d_device_vk. That said, there are broadly two considerations for things ending up in one or the other structure:
- Things that have direct equivalents in the OpenGL backend
generally are in the corresponding structure. E.g., "null_resources_vk" and "null_views_vk" in wined3d_device_vk are equivalents of "dummy_textures" in wined3d_device_gl; "free_*_query_pools" in wined3d_context_vk are equivalents of "free_*_queries" in wined3d_context_gl.
- Things that mostly get accessed from context functions are in
wined3d_context_vk. E.g., "vk_framebuffer" is in that category. That is itself somewhat influenced by equivalency to things in the GL backend; wined3d_context_vk_apply_draw_state() is an equivalent of context_(gl_)apply_draw_state().
The basic idea was that it would make it easier to find things for people already familiar with the GL backend; it does mean some things are somewhat arbitrary when considering the Vulkan backend on its own.
Thanks, those are good considerations.
I would indeed prefer a more functional division, if not strongly per se. I guess I'd be interested to see how helpful the current organization is to someone less familiar with the structure of wined3d...
-
Henri Verbeet -
Matteo Bruni -
Zebediah Figura