Re: [PATCH] wined3d: Set WINED3D_BUFFER_DOUBLEBUFFER for managed buffers as well.

[Stefan Dösinger]

> Am 08.12.2015 um 20:42 schrieb Henri Verbeet <hverbeet(a)codeweavers.com>:
>
> -    if (device->create_parms.flags & WINED3DCREATE_SOFTWARE_VERTEXPROCESSING)
> +    if (device->create_parms.flags & WINED3DCREATE_SOFTWARE_VERTEXPROCESSING || pool == WINED3D_POOL_MANAGED)
> This also helps bugs like 29878, or the Crysis bug that was I think incorrectly closed.

However, the reason why I've never made this seemingly obvious change is that you'll get other applications that crash due to VM exhaustion now. So I'm afraid you'll just trade one kind of breakage for another. The proper fix is probably somewhere in GL_ARB_buffer_storage, but that is too complicated for 1.8.

One app that needed direct mapping of managed buffers to run properly was Half Life 2. I think the game changed since then and sets the LAA flag, but there are other games with the same problem.

[PATCH] wined3d: Set WINED3D_BUFFER_DOUBLEBUFFER for managed buffers as well.

[Henri Verbeet]

On 8 December 2015 at 22:53, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
> However, the reason why I've never made this seemingly obvious change is that you'll get other applications that crash due to VM exhaustion now. So I'm afraid you'll just trade one kind of breakage for another. The proper fix is probably somewhere in GL_ARB_buffer_storage, but that is too complicated for 1.8.

Yeah, there's that risk. It seems preferable though, and there are
probably more options for reducing vm usage / fragmentation than for
fixing this bug. I suppose I'm also hoping that applications that need
a lot of vm space know better than to use the managed pool, but
perhaps that's too optimistic.

That said, I'm not particularly attached to fixing this bug before
1.8, and the application in question needs additional patches to even
start. The patch could certainly be deferred if you think the risk
isn't worth it.

[PATCH] wined3d: Set WINED3D_BUFFER_DOUBLEBUFFER for managed buffers as well.

[Stefan Dösinger]

> Am 08.12.2015 um 22:33 schrieb Henri Verbeet <hverbeet(a)gmail.com>:
>
> That said, I'm not particularly attached to fixing this bug before
> 1.8, and the application in question needs additional patches to even
> start. The patch could certainly be deferred if you think the risk
> isn't worth it.
> I'd say defer it, but change this shortly after 1.8. At the time 29878 was filed DOUBLEBUFFER was a no-go because it broke all source engine games, but a lot has changed since then.
> Yeah, there's that risk. It seems preferable though, and there are
> probably more options for reducing vm usage / fragmentation than for
> fixing this bug.
> Agreed.

Personally I want the GL buffer code merged for surfaces, volumes and buffers before dealing with ARB_buffer_storage, but I'm convinced this extension is the path forward on solving a lot of buffer issues we have.

[PATCH] wined3d: Set WINED3D_BUFFER_DOUBLEBUFFER for managed buffers as well.

[Matteo Bruni]

2015-12-08 22:53 GMT+01:00 Stefan Dösinger <stefandoesinger(a)gmail.com>:
> > Am 08.12.2015 um 20:42 schrieb Henri Verbeet <hverbeet(a)codeweavers.com>:
> >
> > -    if (device->create_parms.flags & WINED3DCREATE_SOFTWARE_VERTEXPROCESSING)
> > +    if (device->create_parms.flags & WINED3DCREATE_SOFTWARE_VERTEXPROCESSING || pool == WINED3D_POOL_MANAGED)
> > This also helps bugs like 29878, or the Crysis bug that was I think incorrectly closed.

I guess you mean https://bugs.winehq.org/show_bug.cgi?id=18799, right?
I think we should reopen that one if you recall that Crysis uses the
managed pool for the relevant vertex buffer (which would make a lot of
sense).

WRT VM exhaustion, yeah, this might hurt but overall it still seems a
step in the right direction.
Not sure if it makes sense but we could potentially use
glBufferSubData() in buffer_direct_upload() instead of mapping the
VBO, that might help a bit with VM pressure or at least move the
problem to the GL implementation (also I'm not sure if
glBufferSubData() is still a bad idea for performance with some GL
drivers). Just thinking out loud...

[PATCH] wined3d: Set WINED3D_BUFFER_DOUBLEBUFFER for managed buffers as well.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2015-12-09 um 22:53 schrieb Matteo Bruni:
> also I'm not sure if glBufferSubData() is still a bad idea for
> performance with some GL drivers). Just thinking out loud...
> Nvidia really, really wants glBufferSubData(*). Others prefer
> glMapBufferRange for dynamic buffers because they sync on
> glBufferSubData. For managed buffers neither of that should really matte
> r.

(*) Terms and conditions apply. If you're doing redundant 50 megabytes
uploads then glMapBuffer(50MB) + modifying a handful of bytes only is
better.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWacF/AAoJEN0/YqbEcdMwuFEP/21v1jcXZX1sjzoIiuHfWGAA
z+JwLtj3ixm3AV+BJuVKpr/qgrADCR1F694SB+Am5yvERI3d2c4oFytBzRPAiCyp
xaVAg5pxC4vp/ENE4kJ/XsgtVL32+8/WI6t6hBGFkcaoTMa6cZ0OqPswAWagFkyi
F/jc0357V2gJs2649QGKv5Fms83+ueZtwfwPvbYMrZoys5NzdeWRIFCG6h4YcYAQ
q4JCSvOKtOfGPBVFVIcDmghUV2QJAHvOqezEMJNcFoVms8wqYcxX+crZuWhxfkt4
BxYRu5dHO7zS4b8FNZyBLB7EsR83iSVziwMByEisVP5wC+HUqfPWSUjRNFv7xHON
JJtGXXYQfeI9cUXMp7zmKAr107qAvFJwyMsRygCcciJzvb+wMaI6BUaXKdUZbtp5
QuP/vAuUpWAeTA53QyDsK3zZGtGQsdxFdEeD9iw7gg7fI6snLf8m2nVj661DQBpK
yNpSYiwxJbMZXG/3ameGdwgTnC91T1jYjLlUXtdCyqzaVUVYPwiiu9Fv79Ax/wCY
m2B+qNIxwyyrIH0unrc5Gw4rSXHh4Mn6VeXQt+1mBv1UBlE/Gf3jk+ujnHBxzZp5
5PNhBx3LnEH5Kcd1HqGL+fuGqEPAHkH/MmFa66TgZ663dOHFpS452nEjMfNXorsZ
8FzARp/YPXP8N2FzOCbY
=btQZ
-----END PGP SIGNATURE-----