GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

[Stefan Dösinger]

Hi,
I'm at school right now, so I had only a very quick look. I'll take a closer 
look later in the evening.

On Tuesday 24 May 2011 14:38:19 Michael Mc Donnell wrote:
> This is my first try at implementing the UpdateSemantics mesh method.
> It works fine here on my local machine, passes the new tests, and
> works with my little interactive demo(not included). I still have a
> few questions though.
> 
> This implementation allocates new heap memory and I suspect that it
> would be faster to re-use the already allocated memory.
As far as I can see this happens only in the tests, where it doesn't really 
matter. Creating and destroying the device is more expensive than the heap 
allocation, so if you care about performance you should reuse your test 
context structure

> Btw should I roll all or some of the patches up into a single patch?
I think you can merge the 3 test patches into one

> Any other comments about the patches in general?

*)
> + * Copyright (C) 2011 Google (Michael Mc Donnell)
Afaik the gsoc conditions say that you keep the copyright to your code :-)

*) In your first test you forgot to check the HeapAlloc result.

*) You could write a test that passes an invalid D3DVERTEXELEMENT9 array to 
UpdateSemantics to see how it handles an invalid declaration.

A few style suggestions:

*) memset(mem, 0, size) is preferred over ZeroMemory(mem, size);

*) In the wined3d code(and its client libs) Henri and I avoid structure 
typedefs. The existing d3dx9 code has a few of them. I'm ok with either way, 
but maybe you and the other d3dx9 devs want to go the wined3d way.

*) In the TestContext structure you can use gotos for error handling, for 
example:

void *ptr1 = NULL, *ptr2 = NULL, *ptr3 = NULL;

ptr1 = HeapAlloc(...);
if(!ptr1) goto err;
ptr2 = HeapAlloc(...);
if(!ptr2) goto err;
ptr3 = HeapAlloc(...);
if(!ptr3) goto err;

return success;

err:
HeapFree(ptr3);
HeapFree(ptr2);
HeapFree(ptr1);
return error;

That avoids ever-growing if (FAILED(hr)) blocks with repeated code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110524/b6fe9147/attachment.pgp>