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

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

A few more comments, with one day delay:

> +static float max_abs_diff_vec2(D3DXVECTOR2 *v1, D3DXVECTOR2 *v2)
> +{
> +    float diff_x = fabs(v1->x - v2->x);
> +    float diff_y = fabs(v1->y - v2->y);
> +
> +    return fmax(diff_x, diff_y);
> +}
fabs and fmax return doubles, probably use the float functions fabsf and fmaxf. gcc doesn't bother about the difference, but msvc generates a precision loss warning.

Duplicating the code for 16 and 32 bit indices looks a bit ugly. Maybe you can use inline functions to read and write values from the proper buffer? The other possibility, which the ddraw blitting code uses is to write a sort of template as a macro and then generate both versions, but I don't like that idea.

Wrt the D3DDECLTYPE epsilon, it may be more efficient to scale the epsilon and calculate the diff and comparison as UBYTEs rather than floats. You'll need the diff functions for the other types like *BYTE*, *SHORT* anyway. Also tests would be helpful here, especially how the epsilon is treated in normalized values like D3DCOLOR and UBYTE4N and how it is treated in non-normalized values like UBYTE4.

Wrt the usage/index fields, what happens when a combination that isn't supported by the fixed function pipeline is used, e.g. NORMAL3 or POSITION5? Those are valid in a vertex declaration and can be used with shaders.

In the test:
> +    int vertex_size_normal = sizeof(struct vertex_normal);
> +    int vertex_size_blendweight = sizeof(struct vertex_blendweight);
> ...
You could make those unsigned.

I'm not quite done reading the test yet, I'll send another mail if I find more.


Think about caching pointreps and adjacency?
Am 24.07.2011 um 11:57 schrieb Michael Mc Donnell:

> On Sun, Jul 24, 2011 at 12:18 AM, Stefan Dösinger
> <stefandoesinger at gmx.at> wrote:
>> I just started reading this, I'll write more once I am done. Just a quick
>> question: What do you mean with "built in function" in the comment above
>> color_to_vector?
> 
> I wanted to ask if there was a DirectX function or macro that can
> convert a D3DCOLOR, which is four bytes in the range 0-255, to four
> floats in the range 0-1 or a D3DXVECTOR4?
There's none I know of. I vaguely remember that there was a macro to construct D3DCOLORs, but that's not too helpful here.

> Reading this again I think color_to_vector should probably accept a
> D3DCOLOR instead of "BYTE color[4]"?
Yes, it would be cleaner. Also keep UBYTE4N in mind, which is pretty much the same as D3DCOLOR for your purposes. It just has a different component ordering(bgra in D3DCOLOR vs rgba in UBYTE4N if I'm not mistaken) when you're using it for drawing. As far as I understand WeldVertices it doesn't compare two attributes of different types, bit if you do you have to watch out there.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)

iQIcBAEBAgAGBQJOLXfpAAoJEN0/YqbEcdMwWWsP/j0Ht34moNt0o4s+cAhiTjk8
1jL4elu9v2SBikvg22W6qjP6SuC+tanFAa0Wv+lsRP9PRBhlT8xkQFC/qdxveAxJ
2tRnWeqa7FJ/0SuOvsD0sTwnhBbPkO9qnv/Y5aBXw6a8O/d1xaNXLXk9rZzo0Aob
dkyc9DCCPdf8+ke4dwm9GAvdu1O/y2sQgYENqUe7PrXwRgzeRcf3yZ7pMfdoEiyD
A7yS74bywQyiO/mUvI/s/9sViAYD8KL/cRP5wRiz1T9QISnzCyxqW+5stuoXgHZS
TVEREOdlMKvwpl24T37Aa6A614ESwlETHa9PUeo8C8900xb6Vmf3pcRsDIkTNyDo
Jwk0VpxJlZ0Md2LBnyR4GdWhUzaPQx4r6yEtd74JOeZlWxxd8fYmrpHEYTIy3WeI
JGsLNIe12oXVtCGxAgpQhYwtiiBgmrQx8rZyhfF+jewUKfQOcRCacn5uYMQLJTQ+
d2g7jsN4Y4BvCCt1RCFRwwWeuoZdAeo5IDApAJBNKhIOqiRORnV2Z4eDDIG1ng1F
MtCatSwaeGsLus0ZZDbpJSVkmVkeaPYO9bYxf0NmAKk7tJDYJRT4Anbl/G2IVjKe
RRyQVwICk9HS42AKqgt8ti3kh8khQ0/h3hdhlDZ67GEI+tdzPUVtpADGWKTzRS9f
BrcWDV0x7reDLEIOLfR4
=hkUU
-----END PGP SIGNATURE-----