[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 2:46 PM, Henri Verbeet wrote:
> On Wed, 30 Jun 2021 at 18:45, Zebediah Figura (she/her)
> <zfigura at codeweavers.com> wrote:
>> On 6/30/21 7:14 AM, Henri Verbeet wrote:
>>> 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...
>>
> Well, wined3d_device_context_unmap() seems conceptually similar enough
> to wined3d_deferred_context_unmap(). I.e., we could implement
> wined3d_deferred_context_unmap() like this:
> 
>      ...
>      if (!wined3d_deferred_context_get_upload_bo(...))
>          return E_NOTIMPL;
> 
>      wined3d_resource_get_sub_resource_map_pitch(...);
>      wined3d_device_context_upload_bo(...);
> 
>      return WINED3D_OK;
> 
> which doesn't seem terrible, and then just call ops->unmap() in
> wined3d_device_context_unmap() like we do now. Of course,
> wined3d_cs_unmap() would be different, but it would also retrieve the
> upload bo in a different way.
> 
> Alternatively, we could do something like this in
> wined3d_device_context_unmap():
> 
>      ...
>      if (context->ops->get_upload_bo(...))
>      {
>          ...
>          wined3d_device_context_upload_bo(...);
>          return WINED3D_OK;
>      }
>      ...
>      return wined3d_device_context_emit_unmap(...);

Where I'm guessing wined3d_device_context_emit_unmap() contains pretty 
much the entirety of wined3d_cs_unmap()?

That had occurred to me as well, although I didn't implement it. I 
probably would leave it for a separate patch, though I'm not opposed to 
rolling it into this one.

> which should also work, and may be preferable, because we could
> implement wined3d_device_context_map() and
> wined3d_device_context_update_sub_resource() in a similar way. I.e., I
> think we can make this work with either
> prepare_upload_bo()/get_upload_bo() or
> update_sub_resource()/map()/unmap() in the device context ops, but we
> probably don't need both sets of ops.

I'd agree the second approach outlined in this email looks clearer. 
That, or to just use update_sub_resource/map/unmap. I don't have a 
preference between those two, so I'll leave that decision to the 
maintainer ;-)