Re: [v9 05/11] ddraw: Do refcounting only through wined3d_texture_{inc, dec}ref().

[Stefan Dösinger]

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

Am 2015-11-19 um 20:34 schrieb Riccardo Bortolato:
> @@ -532,10 +529,9 @@ static void ddraw_surface_cleanup(struct ddraw_surface *surface)
> ...
> + wined3d_texture_decref(surface->wined3d_texture);
> + if (surface->sub_resource_idx == 0)
> wined3d_texture_decref(surface->wined3d_texture);
> If I follow this correctly the unconditionally released reference is the
> one added by ddraw_surface_init, and the one that's released if
> sub_resource_idx == 0 is the initial reference that the texture is
> returned with from wined3d_texture_create. In this case I think it would
> be better to release the idx == 0 reference after calling
> wined3d_surface_create and retrieving the ddraw parent. Please correct
> me if I am wrong...

(This would also remove a non-obvious non-problem in the flip code if
one of the involved surfaces has a non-zero index and the other one has
a zero index and holds an extra reference to its wined3d texture. In
practice it's a non-issue because swapchains never have mipmaps and the
code swaps both the wined3d texture and the sub resource index)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWURGCAAoJEN0/YqbEcdMwXVQP/AqWEh17bjHO5KjWJ7eMdhvM
xS0GCfUK4xcr9i5Xr0AHmOHzhd2NB3vkmYaqQyn6VTXzNbjoYf2uIlTlm9bkhLJi
xfDYjedsx8lcwrCjPUEuwrmam7mKOMNsTq4NZ6IClGeVaQBEYgC/Q9vJ3HgwZYz+
EdW0UNbed0TJaxuaymWVK9SyOUQcMYA90vNvxJNWV/A78jxIktOEGqAEpmIRTEyi
ZzS6iNVQCXc5c90X1ZFeh31g+yX+ZuHLOQ4mlL8dsu5B+dtnb9Mou2FBkcOW51u7
shHbkNGK49sgkmSLKbZaAYFO4+cMWbJpFAqMMbikBZT6+8ATHE+r72USBrc/myya
nnaYlyJuVIZB022zzLxSMcNsyyxMVUZDF+//KfE/VdZ7NfIh8h0gnh/PW7y0i9H9
dYMU/l87+SlVVJCyX4BZYJr1/U3WT2GHJB0eyWHUikIF5hUL5PntbMUwNAVFEM6y
B4QGSE5sfay6DQ6U+FNTfUN35A7KKn4RfFgs6M3wAIh6dXKAcivLbSaPNdRhBGoK
xSR9wA2IN19mkAGdf5VqFOR33AG4VUaKy6q9+DViac7QgGB9eCS4VGBqtbof5/K4
elRJ+jzj4C+1GdQUOYpl77CaKxwRescNP7sOiRyJGxOHOOQ/oP0MhRYRG9deak+W
BhwM31xUd2UpEzECH5EK
=a+jA
-----END PGP SIGNATURE-----

[v9 05/11] ddraw: Do refcounting only through wined3d_texture_{inc, dec}ref().

[Riccardo Bortolato]

2015-11-22 1:51 GMT+01:00 Stefan Dösinger <stefandoesinger(a)gmail.com>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Am 2015-11-19 um 20:34 schrieb Riccardo Bortolato:
> > @@ -532,10 +529,9 @@ static void ddraw_surface_cleanup(struct ddraw_surface *surface)
> > ...
> > + wined3d_texture_decref(surface->wined3d_texture);
> > + if (surface->sub_resource_idx == 0)
> > wined3d_texture_decref(surface->wined3d_texture);
> > If I follow this correctly the unconditionally released reference is the
> > one added by ddraw_surface_init, and the one that's released if
> > sub_resource_idx == 0 is the initial reference that the texture is
> > returned with from wined3d_texture_create. In this case I think it would
> > be better to release the idx == 0 reference after calling
> > wined3d_surface_create and retrieving the ddraw parent. Please correct
> > me if I am wrong...

Hi Stefan,

you mean something like (on top of this patch):

diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index edbc21d..59895d7 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -530,8 +530,6 @@ static void ddraw_surface_cleanup(struct
ddraw_surface *surface)
     if (surface->wined3d_rtv)
         wined3d_rendertarget_view_decref(surface->wined3d_rtv);
     wined3d_texture_decref(surface->wined3d_texture);
-    if (surface->sub_resource_idx == 0)
-        wined3d_texture_decref(surface->wined3d_texture);
 }

 ULONG ddraw_surface_release_iface(struct ddraw_surface *This)
@@ -6103,6 +6101,7 @@ HRESULT ddraw_surface_create(struct ddraw
*ddraw, const DDSURFACEDESC2 *surface_

     resource = wined3d_texture_get_sub_resource(wined3d_texture, 0);
     root = wined3d_resource_get_parent(resource);
+    wined3d_texture_decref(wined3d_texture);
     root->is_complex_root = TRUE;
     texture->root = root;

@@ -6237,6 +6236,7 @@ HRESULT ddraw_surface_create(struct ddraw
*ddraw, const DDSURFACEDESC2 *surface_

             resource = wined3d_texture_get_sub_resource(wined3d_texture, 0);
             last = wined3d_resource_get_parent(resource);
+            wined3d_texture_decref(wined3d_texture);
             texture->root = last;
             mip_desc = &last->surface_desc;


This seems OK (test wise).

Ciao,
Riccardo
> (This would also remove a non-obvious non-problem in the flip code if
> one of the involved surfaces has a non-zero index and the other one has
> a zero index and holds an extra reference to its wined3d texture. In
> practice it's a non-issue because swapchains never have mipmaps and the
> code swaps both the wined3d texture and the sub resource index)
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBCAAGBQJWURGCAAoJEN0/YqbEcdMwXVQP/AqWEh17bjHO5KjWJ7eMdhvM
> xS0GCfUK4xcr9i5Xr0AHmOHzhd2NB3vkmYaqQyn6VTXzNbjoYf2uIlTlm9bkhLJi
> xfDYjedsx8lcwrCjPUEuwrmam7mKOMNsTq4NZ6IClGeVaQBEYgC/Q9vJ3HgwZYz+
> EdW0UNbed0TJaxuaymWVK9SyOUQcMYA90vNvxJNWV/A78jxIktOEGqAEpmIRTEyi
> ZzS6iNVQCXc5c90X1ZFeh31g+yX+ZuHLOQ4mlL8dsu5B+dtnb9Mou2FBkcOW51u7
> shHbkNGK49sgkmSLKbZaAYFO4+cMWbJpFAqMMbikBZT6+8ATHE+r72USBrc/myya
> nnaYlyJuVIZB022zzLxSMcNsyyxMVUZDF+//KfE/VdZ7NfIh8h0gnh/PW7y0i9H9
> dYMU/l87+SlVVJCyX4BZYJr1/U3WT2GHJB0eyWHUikIF5hUL5PntbMUwNAVFEM6y
> B4QGSE5sfay6DQ6U+FNTfUN35A7KKn4RfFgs6M3wAIh6dXKAcivLbSaPNdRhBGoK
> xSR9wA2IN19mkAGdf5VqFOR33AG4VUaKy6q9+DViac7QgGB9eCS4VGBqtbof5/K4
> elRJ+jzj4C+1GdQUOYpl77CaKxwRescNP7sOiRyJGxOHOOQ/oP0MhRYRG9deak+W
> BhwM31xUd2UpEzECH5EK
> =a+jA
> -----END PGP SIGNATURE-----

[v9 05/11] ddraw: Do refcounting only through wined3d_texture_{inc, dec}ref().

[Stefan Dösinger]

> Am 22.11.2015 um 11:37 schrieb Riccardo Bortolato <rikyz619(a)gmail.com>:
>
> you mean something like (on top of this patch):
> Yes, exactly that :-) .