From: Henri Verbeet <hverbeet@gmail.com>
Subject: Re: [PATCH 1/3] wined3d: Handle discarded buffers in buffer_resource_sub_resource_map().
Message-Id: <CAOsNvwxJwGLAudQ-BhkdOHWWrqk6hd5nga-awtxZ5EJCROfvXA@mail.gmail.com>
Date: Thu, 27 Jan 2022 14:13:05 +0100
In-Reply-To: <2cfd1f6b-913b-b7bb-a4fa-2cf750f180f8@codeweavers.com>
References: <20220125022039.2005817-1-zfigura@codeweavers.com> <CAOsNvwxB_o4LrJjgb0dbFRiZofyKX7_Cwe0OpajCqz2ZDPh2nA@mail.gmail.com> <2cfd1f6b-913b-b7bb-a4fa-2cf750f180f8@codeweavers.com>

On Wed, 26 Jan 2022 at 17:17, Zebediah Figura (she/her)
<zfigura@codeweavers.com> wrote:
>
> On 1/26/22 07:47, Henri Verbeet wrote:
> > On Tue, 25 Jan 2022 at 03:21, Zebediah Figura <zfigura@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().