Re: [d3d8] Fix CreateImageSurface implementation (with testcase)

2008-06-21 Thread H. Verbeet
2008/6/20 Tobias Jakobi <[EMAIL PROTECTED]>:
> However I think the comment should stay there. I don't think the MSDN
> docs are updated in the near future, at least what D3D9 is concerned
> (now that D3D10 is more and more used). So in case someone is wondering
> why the implementation of wine differs from the behaviour described in
> the MSDN this comment should clarify this.
>
>
> Now should I also add something about the scratch pool type?
>
I don't feel strongly about it, it's just that the comment talks a lot
about what MSDN says or doesn't say, without actually describing what
D3DPOOL_SCRATCH is supposed to mean, while ultimately that's all we
care about.




[d3d8] Fix CreateImageSurface implementation (with testcase)

2008-06-20 Thread Tobias Jakobi
(I think this belongs into wine-devel, sorry for also posting this on 
wine-patches)

Oops, sorry! I corrected the mixed indentation style (must have been the 
copy&paste).

However I think the comment should stay there. I don't think the MSDN 
docs are updated in the near future, at least what D3D9 is concerned 
(now that D3D10 is more and more used). So in case someone is wondering 
why the implementation of wine differs from the behaviour described in 
the MSDN this comment should clarify this.


Now should I also add something about the scratch pool type?


Greets,
Tobias




Re: [d3d8] Fix CreateImageSurface implementation (with testcase)

2008-06-20 Thread H. Verbeet
2008/6/20 Tobias Jakobi <[EMAIL PROTECTED]>:
> Patch fixes a problem with the IDirect3DDevice8::CreateImageSurface
> implementation (wrong pool type of the returned surface object). This fixes
> savegame screenshot bugs (black images instead of game scenes) in both Max
> Payne and Max Payne 2.
>
> Affected bugs:
> http://bugs.winehq.org/show_bug.cgi?id=9775
> http://bugs.winehq.org/show_bug.cgi?id=7801
>
> Test case is included and verifies behaviour on both Windows XP and Vista.
> This is my first patch for wine and also my first post on the wine-patches
> ml, so if I'm doing anything wrong here please tell me and I try to do
> better next time :-)
>
> Greets,
> Tobias Jakobi
>

The change itself looks ok to me, provided the test passes on XP.
Don't mix tabs and spaces though:
> + /*
> + D3DPOOL_DEFAULT = 0
> +D3DPOOL_MANAGED = 1
> +D3DPOOL_SYSTEMMEM = 2
> +D3DPOOL_SCRATCH = 3
> + */

I'm not sure this adds much (not that the original comment did either):
> -/*MSDN: D3DPOOL_SCRATCH will return a surface that has identical 
> characteristics to a surface created by the Microsoft DirectX 8.x method 
> CreateImageSurface.
> +/*MSDN: D3DPOOL_SCRATCH will return a surface that has identical 
> characteristics to a surface created by the Microsoft DirectX 8.x method 
> CreateImageSurface (quote from the DirectX9 documentation).
> +However this is wrong behaviour and conflicting with the explanation 
> from the original DirectX8 documentation. According to DX8 docs it should be 
> D3DPOOL_SYSTEMMEM and NOT D3DPOOL_SCRATCH.
> +For a testcase of the DX8 CreateImageSurface method look for 
> test_image_surface_pool in dlls/d3d8/tests/surface.c

It would probably be better to just describe the behaviour of
D3DPOOL_SCRATCH there.