Re: [PATCH 7/8] ddraw: Fix SetTexture reference count behaviour.

[Stefan Dösinger]

On Monday 08 August 2011 00:26:38 Ričardas Barkauskas wrote:
> +    hr = wined3d_device_get_texture(This->wined3d_device, stage, 
> &wined3d_texture);
> This will cause problems when the application is recording a stateblock. In 
> this case set_texture will apply the texture in the update stateblock and 
> get_texture will return the value from the main stateblock.

I feel kinda sorry for you, but I am afraid fixing this properly will require 
a lot of work, starting with testing d3d7 stateblock behavior wrt refcounting. 
In the worst case fixing this might require re-implementing some of the 
stateblock logic in ddraw.

Also make sure to test SetRenderTarget. In d3d8/9 the render target is not 
recorded in the stateblock, so there's no issue. d3d7 may be different. 
Unfortunately we have zero stateblock tests for d3d7.

There's also SetCurrentViewport that addrefs, but IDirect3DDevice3 doesn't 
support stateblocks, so it is not an issue.

The really crappy thing, and that's where my sorry feeling comes from, is that 
I haven't seen any application yet that uses d3d7's stateblocks. But I'm 
afraid if we want to stop the fix one app, regress another app problem we have 
with ddraw we'll have to do this properly and not hope no app ever runs into 
this.

[PATCH 7/8] ddraw: Fix SetTexture reference count behaviour.

[Henri Verbeet]

On 8 August 2011 01:03, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
> On Monday 08 August 2011 00:26:38 Ričardas Barkauskas wrote:
> > +    hr = wined3d_device_get_texture(This->wined3d_device, stage,
> > &wined3d_texture);
> > This will cause problems when the application is recording a stateblock. In
> > It also violates COM rules wrt. reference counting. Though in
> > fairness, ddraw as an API is probably good at that anyway.

I think the more important question here is why it matters. Does the
application really care about the number returned by AddRef() /
Release(), or is this just about not destroying surfaces that are
still in use? If it's the latter, you can fix this just by fixing
ddraw surface destruction to not actually destroy anything until
ddraw_surface_wined3d_object_destroyed() is called. (See how this is
done in d3d8 and d3d9.)
> The really crappy thing, and that's where my sorry feeling comes from, is that
> I haven't seen any application yet that uses d3d7's stateblocks. But I'm
> afraid if we want to stop the fix one app, regress another app problem we have
> with ddraw we'll have to do this properly and not hope no app ever runs into
> this.

I agree that ddraw needs more and better tests, though stateblocks
wouldn't be on top of my list. To a lesser extent this is also true of
d3d8. If anyone has some spare time (haha), porting tests that are
currently d3d9-only to d3d8 and ddraw would be an useful thing to do.