ddraw: Fix refcounting of palettes

This fixes a crash in a game called "Autobahnverfolgungsjagd Total"
(not in AppDB).

While tracing this bug I noticed that
IWineD3DBaseSurfaceImpl_GetPalette() does not call AddRef when
returning the palette. Is that intentional?

Cheers,
Cihan

[Stefan Dösinger]

Am Sonntag, 16. März 2008 14:44:37 schrieb [e-mail]:
> This fixes a crash in a game called "Autobahnverfolgungsjagd Total"
> (not in AppDB).

I think it would be better to set pal = NULL if there is no wined3d palette 
returned a few lines above the place you're changing.
> While tracing this bug I noticed that
> IWineD3DBaseSurfaceImpl_GetPalette() does not call AddRef when
> returning the palette. Is that intentional?

This looks like it needs a fix

[Cihan Altinay]

Stefan Dösinger wrote:
> Am Sonntag, 16. März 2008 14:44:37 schrieb [e-mail]:
> I think it would be better to set pal = NULL if there is no wined3d palette 
> returned a few lines above the place you're changing.

That's fine with me but it would mean setting it to NULL in every
iteration instead of only when necessary.
> > While tracing this bug I noticed that
> > IWineD3DBaseSurfaceImpl_GetPalette() does not call AddRef when
> > returning the palette. Is that intentional?
>
> This looks like it needs a fix

Ok, I'll add that to the same patch since the problem is related.

Cheers,
Cihan

[Stefan Dösinger]

Am Sonntag, 16. März 2008 17:22:52 schrieb Cihan Altinay:
> Stefan Dösinger wrote:
> > Am Sonntag, 16. März 2008 14:44:37 schrieb [e-mail]:
> > I think it would be better to set pal = NULL if there is no wined3d
> > palette returned a few lines above the place you're changing.
>
> That's fine with me but it would mean setting it to NULL in every
> iteration instead of only when necessary.

I don't think this code is highly performance critical, as it is usually 
called during loading. If you set it to NULL there you'll always set the 
palette that is used rather than relying that the previous iteration left a 
correct palette(maybe NULL) set.
> > > While tracing this bug I noticed that
> > > IWineD3DBaseSurfaceImpl_GetPalette() does not call AddRef when
> > > returning the palette. Is that intentional?
> >
> > This looks like it needs a fix
>
> Ok, I'll add that to the same patch since the problem is related.

If one patch works without the other please send separate patches.

[Alexander Dorofeyev]

Stefan Dösinger wrote:
> Am Sonntag, 16. März 2008 14:44:37 schrieb [e-mail]:
> > This fixes a crash in a game called "Autobahnverfolgungsjagd Total"
> > (not in AppDB).
>
> I think it would be better to set pal = NULL if there is no wined3d palette 
> returned a few lines above the place you're changing.
> > While tracing this bug I noticed that
> > IWineD3DBaseSurfaceImpl_GetPalette() does not call AddRef when
> > returning the palette. Is that intentional?
>
> This looks like it needs a fix

To my knowledge, the way refcounting for IWineD3DPalette currently works, 
refcount is always kept equal to 1 during its lifetime, IDirectDrawPalette owns 
this reference and releases it when it (ddraw palette) is getting destroyed. So, 
the real refcounting is done for ddraw palette and it is omitted in most places 
for wined3d palette (which simply gets destroyed together with ddraw palette). 
Generally, this seems to work ok. If IWineD3DBaseSurfaceImpl_GetPalette is 
changed to call AddRef, then all other places must be fixed too (SetPalette and 
callers of IWineD3DBaseSurfaceImpl_GetPalette, possibly others too) or things 
may break (leaks etc) and become inconsistent. For example, 
IDirectDrawSurfaceImpl_GetPalette does not call Release on interface it gets 
from IWineD3DSurface_GetPalette.