[PATCH 3/3] wined3d: Do not emit an upload for NOOVERWRITE maps on deferred contexts.

[Zebediah Figura (she/her)]

On 9/29/21 11:22, Henri Verbeet wrote:
> On Wed, 29 Sept 2021 at 18:04, Zebediah Figura (she/her)
> <zfigura at codeweavers.com> wrote:
>>
>> On 9/29/21 10:48, Henri Verbeet wrote:
>>> On Tue, 28 Sept 2021 at 23:28, Zebediah Figura <zfigura at codeweavers.com> wrote:
>>>> @@ -2749,6 +2751,7 @@ void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_conte
>>>>        op->box = *box;
>>>>        op->bo.addr.buffer_object = 0;
>>>>        op->bo.addr.addr = data;
>>>> +    op->bo.flags = UPLOAD_BO_UPLOAD_ON_UNMAP;
>>>>        op->row_pitch = row_pitch;
>>>>        op->slice_pitch = slice_pitch;
>>>>
>>> Do we need UPLOAD_BO_UPLOAD_ON_UNMAP here? It seems a little out of
>>> place, and wined3d_cs_exec_update_sub_resource() doesn't appear to use
>>> it.
>>
>> No, I suppose we don't. Thinking too far ahead...
>>
>> Although if we obsolesce UPLOAD_BO_UPLOAD_ON_UNMAP entirely, which is
>> possible, that does mean we can't use it for deferred contexts as you
>> suggest below. (And if we don't, it's mildly weird not to pass it here.)
>>
> Is it? My understanding was that UPLOAD_ON_UNMAP was primarily for
> wined3d_device_context_emit_unmap(), to distinguish the case where we
> got a reference to the original buffer object. (As is the case with
> NOOVERWRITE maps on deferred contexts, but we could conceivably do
> that on immediate contexts as well in some cases.)
> 

My plan had been to introduce a couple of other flags here, one such 
being FLUSH_ON_UNMAP, which would essentially be used for NOOVERWRITE 
maps of non-coherent BOs on the immediate context. That would need to 
get passed all the way to the CS thread. Then you end up with something 
like:


wined3d_cs_exec_update_sub_resource(...)
{
     if (op->bo.flags & UPLOAD_BO_FLUSH_ON_UNMAP)
         ...
     else
         wined3d_buffer_copy_bo_address(...);
}

wined3d_device_context_emit_unmap(...)
{
     struct upload_bo bo;

     if (context->ops->get_upload_bo(..., &bo))
     {
         if (bo.flags & (UPLOAD_BO_FLUSH_ON_UNMAP | 
UPLOAD_BO_UPLOAD_ON_UNMAP))
             wined3d_device_context_upload_bo(..., &bo);
     }
}


Which is both kind of weird, and also less weird than this patch is.

But it's also not even hard to avoid introducing 
UPLOAD_BO_UPLOAD_ON_UNMAP until it's really used, so I might as well 
just restructure my patches a bit accordingly...