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.




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.