d3dx9_36 [try 3]: Implement D3DXSHRotate

Matteo Bruni matteo.mystral at gmail.com
Tue Sep 4 10:10:20 CDT 2012


2012/9/4 Rico Schüller <kgbricola at web.de>:
> On 04.09.2012 13:19, Nozomi Kodama wrote:
> +    FLOAT expected, in[100], out[100],
> ...
> +    for (i = 0; i < 49; i++)
> +        in[i] = i + 1.01f;
>
> I guess this belongs together.
>
> I'd like to make some suggestions, please don't take it personally.
>
> When you got a comment on a patch, read it carefully. Think about it, if it
> makes sens to do the change (just adding what I've said is not always the
> best, it just gives you a hint, that you may improve some things, the list
> is not always complete and different people may have different suggestions)
> and try to improve your patch. After you are done, read your patch. Well
> mistakes happen, especially when you try to get something done really fast.
> (1) Take you some time and read it again and see if you've done that as best
> as possible and if you are happy with the code you've done, then submit it.
> If you've found a bug don't submit, fix it and go to #1.
>
> The earlier a bug is found the cheaper is it to solve it. I hope this helps
> a bit and it hopefully saves you some time to get a patch in the feature
> accepted. Thought I'd like to thank you for your patience and hopefully I
> haven't annoyed you and we'll see some good quality patches in the feature
> from you. :-)
>
> Cheers
> Rico
>

I agree with Rico. It's hard to find issues in your own code, which
makes it all the more important to check it thoroughly.

Still, at least try to be careful with the code style:

+static void RotateX(FLOAT *out, UINT order, FLOAT a, FLOAT *in)

I think it's better to only use lower case for the (private) function
name. Something like "rotatex" or "rotate_x" or whatever.

+    if ( order < 2 )
+    return;

Please indent it correctly.

+    return;
+}

Unneeded.

+FLOAT* WINAPI D3DXSHRotate(FLOAT *out, UINT order, CONST D3DXMATRIX
*matrix, CONST FLOAT *in)

Fix the FLOAT*.

+    return;
+}

Again.

Also, the patch is full of trailing whitespaces, please fix those too.



More information about the wine-devel mailing list