[PATCH] wined3d: Don't discard buffer data if buffer is not bound.

[Paul Gofman]

On 7/10/20 08:55, Axel Davy wrote:
> On 09/07/2020 13:29, Paul Gofman wrote:
>> On 7/9/20 13:11, Henri Verbeet wrote:
>>> On Thu, 9 Jul 2020 at 13:47, Axel Davy <davyaxel0 at gmail.com> wrote:
>>>> I have another explanation for this behaviour:
>>>>
>>>> For the first DISCARD you try, the buffer backing is busy because
>>>> it is
>>>> doing to be used for rendering not yet finished.
>>>> Thus the DISCARD does actually do something.
>>>>
>>>> Then when you map for reading the buffer it flushes everything and
>>>> rendering has to finish. Thus for the next DISCARD, the driver finds
>>>> that the driver backing is not busy, it decides to keep it.
>>>>
>>>> Definitely you should discard buffer data if the buffer is not
>>>> bound. It
>>>> will happen in a lot of games. A buffer not bound doesn't mean it
>>>> is not
>>>> used by the GPU. What you really want is to not discard when the
>>>> buffer
>>>> is not bound nor used in any pending rendering. That is of course much
>>>> harder to detect. I would bet your application just needs the
>>>> DISCARD to
>>>> do nothing before the buffer is used for rendering for the first time.
>>>>
>>> Yes, what Axel said. I suspect the application is essentially
>>> depending on an optimisation to not bother discarding the buffer if we
>>> wouldn't have to wait to map it. I.e., similar to the redundant
>>> discard filtering we do a bit further down in
>>> buffer_resource_sub_resource_map(), and similar to the Vulkan discard
>>> handling we do in adapter_vk_map_bo_address(). In its current form,
>>> this patch will introduce stalls when mapping buffers that have been
>>> unbound, but are still in use by the GPU.
>> Yes, dropping discard flag for any unbound buffer maps was a bad idea. I
>> don't see a good way to check if buffer is actually used in GL during
>> mapping, so I think I should leave it for now until persistent buffer
>> mapping changes are there.
>>
> One way would be to introduce a frame count and remember the last
> frame the buffer was bound. If the frame count difference with the
> current frame is more than, say, 4, it is definitely idle.
>
> Also mapping for read a buffer without RAM backing would mean all the
> other buffers become idle. 

Yes, sure, there should be some ways to introduce a workaround to make
AION happy in the current buffer management logic, but probably a better
solution is switching to persistent buffer mapping and overall buffer
mapping logic redesign which AFAIK is being worked on and which I guess
will obsolete such solutions. Also, the problem should probably not be
present with the Vulkan backend.