[PATCH 3/3] wined3d: Factor out buffer bo_user invalidation.

[Zebediah Figura (she/her)]

On 11/1/21 09:20, Henri Verbeet wrote:
> On Wed, 20 Oct 2021 at 07:28, Zebediah Figura <zfigura at codeweavers.com> wrote:
>> +static void adapter_gl_destroy_bo(struct wined3d_context *context, struct wined3d_bo *bo)
>> +{
>> +    struct wined3d_context_gl *context_gl = wined3d_context_gl(context);
>> +    struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(bo);
>> +
>> +    if (context_gl->c.transform_feedback_active && bo_gl->binding == GL_TRANSFORM_FEEDBACK_BUFFER
>> +            && wined3d_context_is_graphics_state_dirty(&context_gl->c, STATE_STREAM_OUTPUT))
>> +    {
>> +        /* It's illegal to (un)bind GL_TRANSFORM_FEEDBACK_BUFFER while transform
>> +         * feedback is active. Deleting a buffer implicitly unbinds it, so we
>> +         * need to end transform feedback here if this buffer was bound.
>> +         *
>> +         * This should only be possible if STATE_STREAM_OUTPUT is dirty; if we
>> +         * do a draw call before destroying this buffer then the draw call will
>> +         * already rebind the GL target. */
>> +        WARN("Deleting buffer object %p, disabling transform feedback.\n", bo_gl);
>> +        wined3d_context_gl_end_transform_feedback(context_gl);
>> +    }
>> +
> Is there any particular reason this belongs in adapter_gl_destroy_bo()
> instead of wined3d_context_gl_destroy_bo()?
> 
>> +static void adapter_vk_destroy_bo(struct wined3d_context *context, struct wined3d_bo *bo)
>> +{
>> +    struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo);
>> +
>> +    wined3d_context_vk_destroy_bo(wined3d_context_vk(context), bo_vk);
>> +    bo_vk->vk_buffer = VK_NULL_HANDLE;
>> +    bo_vk->memory = NULL;
>> +}
>> +
> Similarly, it seems somewhat arbitrary to clear the "vk_buffer" and
> "memory" fields in adapter_vk_destroy_bo() instead of
> wined3d_context_vk_destroy_bo().

The answer to both of these is "well, wined3d_context_*_destroy_bo() 
didn't already do those things". That could be changed, though.

> 
>> @@ -3370,6 +3380,7 @@ 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);
>> +    void (*adapter_destroy_bo)(struct wined3d_context *context, struct wined3d_bo *bo);
>>       HRESULT (*adapter_create_swapchain)(struct wined3d_device *device,
>>               struct wined3d_swapchain_desc *desc,
>>               struct wined3d_swapchain_state_parent *state_parent, void *parent,
>> @@ -4982,8 +4993,6 @@ struct wined3d_buffer_ops
>>   {
>>       BOOL (*buffer_prepare_location)(struct wined3d_buffer *buffer,
>>               struct wined3d_context *context, unsigned int location);
>> -    void (*buffer_unload_location)(struct wined3d_buffer *buffer,
>> -            struct wined3d_context *context, unsigned int location);
>>       void (*buffer_upload_ranges)(struct wined3d_buffer *buffer, struct wined3d_context *context, const void *data,
>>               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,
> 
> Is that the direction we want to go in? The original idea was to unify
> wined3d_buffer_ops and wined3d_texture_ops, and then merge them into
> wined3d_resource_ops. If there are additional reasons to introduce
> adapter_destroy_bo(), perhaps that's fine though.
> 

I guess not really. I don't think I had a particular reason for this 
commit except to effect some deduplication.

Of course we could do a similar thing with textures, but there are quite 
a few more locations to deal with...

I'll just drop the patch.