[PATCH 6/8] wined3d: Allow passing a NULL context to wined3d_device_vk_create_bo().

[Zebediah Figura]

On 11/3/21 12:11 PM, Henri Verbeet wrote:
> On Wed, 3 Nov 2021 at 00:37, Zebediah Figura <zfigura at 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.
>