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.