Re: [PATCH v2 4/5] wined3d: Use context->ops->prepare_upload_bo() in wined3d_device_context_map() if possible.

[Zebediah Figura (she/her)]

On 6/30/21 7:14 AM, Henri Verbeet wrote:
> On Wed, 30 Jun 2021 at 06:34, Zebediah Figura  wrote:
> > +static bool wined3d_deferred_context_get_upload_bo(struct wined3d_device_context *context,
> > -
> >    struct wined3d_resource *resource, unsigned int sub_resource_idx,
> > -
> >    struct wined3d_box *box, struct wined3d_const_bo_address *address)
> >
> > +{
> > - struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context);
> > - int i = deferred->upload_count;
> > -
> > - while (i--)
> > - {
> > -
> >    struct wined3d_deferred_upload *upload = &deferred->uploads[i];
> > -
> > -
> >    if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx)
> > -
> >    {
> > -
> >        *box = upload->box;
> > -
> >        address->buffer_object = 0;
> > -
> >        address->addr = upload->sysmem;
> > -
> >        return true;
> > -
> >    }
> > - }
> > -
> > - return false;
> >
> > +}
> > -
>
> "SIZE_T i;", or at least "unsigned int i;"

Oh, right, "while (i--)" does work with unsigned integers. Reverse 
iteration is annoying :-/
> > HRESULT CDECL wined3d_device_context_unmap(struct wined3d_device_context *context,
> >           struct wined3d_resource *resource, unsigned int sub_resource_idx)
> >   {
> > - struct wined3d_const_bo_address addr;
> > - struct wined3d_box box;
> > -
> > TRACE("context %p, resource %p, sub_resource_idx %u.\n", context, resource, sub_resource_idx);
> >
> > - return context->ops->unmap(context, resource, sub_resource_idx);
> > - if (context->ops->get_upload_bo(context, resource, sub_resource_idx, &box, &addr))
> > - {
> > -
> >    unsigned int row_pitch, slice_pitch;
> > -
> > -
> >    wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch);
> > -
> >    wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &addr, row_pitch, slice_pitch);
> > -
> >    return WINED3D_OK;
> > - }
> > - else
> > - {
> > -
> >    return context->ops->unmap(context, resource, sub_resource_idx);
> > - }
> > }
>
> Would we ever use ops->get_upload_bo() and ops->unmap() independently
> of each other? If not, we may as well merge those together.

No, but they're conceptually pretty different, and merging them strikes 
me as ugly...

I mean, the nice way to merge them is to also put the prepare_upload_bo 
bits into map(), but that defeats the purpose of having the helper in 
the first place.