[PATCH 1/3] wined3d: Handle discarded buffers in buffer_resource_sub_resource_map().

[Henri Verbeet]

On Wed, 26 Jan 2022 at 17:17, Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> On 1/26/22 07:47, Henri Verbeet wrote:
> > On Tue, 25 Jan 2022 at 03:21, Zebediah Figura <zfigura at codeweavers.com> wrote:
> >> @@ -927,10 +928,13 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc
> >>
> >>       count = ++resource->map_count;
> >>
> >> -    if (buffer->buffer_object)
> >> +    context = context_acquire(device, NULL, 0);
> >> +
> >> +    wined3d_buffer_get_memory(buffer, context, &addr);
> >> +
> >> +    if (addr.buffer_object)
> >>       {
> > I think that potentially changes the location we map in some cases.
> > (In particular, when buffer->buffer_object is non-NULL, but
> > WINED3D_LOCATION_BUFFER is not current.) I don't know for sure whether
> > that would cause issues for applications, but this code is sensitive
> > enough that I'd prefer avoiding wined3d_buffer_get_memory() here
> > unless we have a good reason not to.
>
> I guess I see the point here—if we are using a BO in the first place, we
> probably want to actually use the BO instead of reusing sysmem.
>
> Note though that this doesn't seem to preclude
> wined3d_buffer_get_memory(); it just means we want to keep the check for
> buffer->buffer_object instead of addr.buffer_object.
>
I suppose, but I'm not sure there's much of a point in that case.
Conceptually, wined3d_buffer_get_memory() is for cases where we don't
particularly care which location is current.

> > What is the issue exactly? Is it that buffer->buffer_object may not
> > have been created yet for discarded resources?
>
> More generally, no location may yet have been prepared. In particular
> after patch 3/3 no location will be prepared on creation.

It probably makes sense to check for WINED3D_BUFFER_USE_BO instead of
buffer->buffer_object then, and add wined3d_buffer_prepare_location()
calls to paths that don't already call wined3d_buffer_load_location().