From: "Zebediah Figura (she/her)" Subject: Re: [PATCH 1/3] wined3d: Handle discarded buffers in buffer_resource_sub_resource_map(). Message-Id: <2cfd1f6b-913b-b7bb-a4fa-2cf750f180f8@codeweavers.com> Date: Wed, 26 Jan 2022 10:17:06 -0600 In-Reply-To: References: <20220125022039.2005817-1-zfigura@codeweavers.com> On 1/26/22 07:47, Henri Verbeet wrote: > On Tue, 25 Jan 2022 at 03:21, Zebediah Figura 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. > > 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.