include: Add d3dx10math.inl.

Matteo Bruni matteo.mystral at gmail.com
Wed Jul 13 14:48:19 CDT 2016


2016-07-04 19:26 GMT+02:00 Andrey Gusev <andrey.goosev at gmail.com>:

+#ifndef __D3DX10MATH_INL__
+#define __D3DX10MATH_INL__

My copy of the file (from the latest DX SDK) uses __D3DXMATH_INL__ as
header guard. I think we should do the same for compatibility.

+/* constructors & operators */

Nitpicking but this comment doesn't seem particularly useful.

+inline D3DXVECTOR2::D3DXVECTOR2(const FLOAT *pf)

Just use plain float.

+    if(!pf) return;

Space before '(', return on a separate line.

+inline D3DXVECTOR2::operator FLOAT* ()
+{
+    return (FLOAT*)&x;

Please put a space between "float" and '*' in the cast. Probably do
the same in the operator definition, although I don't really care in
that regard.

+inline D3DXMATRIX D3DXMATRIX::operator - () const
+{
+    return D3DXMATRIX(-_11, -_12, -_13, -_14,
+                      -_21, -_22, -_23, -_24,
+                      -_31, -_32, -_33, -_34,
+                      -_41, -_42, -_43, -_44);
+}

This is almost identical to the MS's header. Please use 8 space
indentation for line continuations. Probably drop the space between
the "operator" keyword and the operator and between the operator and
the opening parenthesis, in the operator declaration.

+inline D3DXCOLOR::operator UINT () const
+{
+    UINT _r = r >= 1.0f ? 0xff : r <= 0.0f ? 0x00 : (UINT)(r * 255.0f + 0.5f);
+    UINT _g = g >= 1.0f ? 0xff : g <= 0.0f ? 0x00 : (UINT)(g * 255.0f + 0.5f);
+    UINT _b = b >= 1.0f ? 0xff : b <= 0.0f ? 0x00 : (UINT)(b * 255.0f + 0.5f);
+    UINT _a = a >= 1.0f ? 0xff : a <= 0.0f ? 0x00 : (UINT)(a * 255.0f + 0.5f);
+
+    return (_a << 24) | (_r << 16) | (_g << 8) | (_b << 0);
+}

This one is also virtually identical. There seems to be room for
writing a slightly different but equivalent implementation.
FWIW identifiers starting with an underscore are supposed to be
reserved, to some degree. It's generally better to avoid them.

+inline D3DXFLOAT16::D3DXFLOAT16()
+{
+}

These default constructors are in (native) d3dx10math.h. I know that
with d3dx9* headers we did put those in the .inl file but I think that
was a bad idea.

+inline BOOL D3DXFLOAT16::operator == (const D3DXFLOAT16 &f) const
+{

This one is also almost identical, implementation-wise. Now maybe
there isn't that much room for variations here but still, at the very
least replace the "== 0" with a '!'. If you can find other reasonable
ways to differentiate, that's even better.

+inline D3DXCOLOR::D3DXCOLOR(UINT col)
+{
+    const FLOAT f = 1.0f / 255.0f;
+    r = f * (FLOAT)(unsigned char)(col >> 16);
+    g = f * (FLOAT)(unsigned char)(col >>  8);
+    b = f * (FLOAT)(unsigned char)(col >>  0);
+    a = f * (FLOAT)(unsigned char)(col >> 24);
+}

Another one essentially identical. I would write it as something like:

inline D3DXCOLOR::D3DXCOLOR(UINT col)
{
    float inv = 1.0f / 255.0f;

    a = ((col >> 24) & 0xff) * inv;
    r = ((col >> 16) & 0xff) * inv;
    g = ((col >>  8) & 0xff) * inv;
    b = (col & 0xff) * inv;
}

+static inline D3DXCOLOR* D3DXColorLerp(D3DXCOLOR *pout, const
D3DXCOLOR *pc1, const D3DXCOLOR *pc2, FLOAT s)

Don't use those prefixed argument names: out, c1 and c2 work just
fine. Also you want a space between D3DXCOLOR and '*' and no space
between '*' and the function name.

+{
+    if ( !pout || !pc1 || !pc2 ) return NULL;

Please no spaces after the '(' and before the ')'. Return on a separate line.

+    pout->r = (1-s) * (pc1->r) + s *(pc2->r);

You want spaces around the '-' operator and a space after the
multiplication operator. No need for the parentheses around pc1->r and
pc2->r.
Regardless, MS's header uses the "optimized" linear interpolation
formula, with just one multiplication. Your current implementation
will generally give slightly different results (because of the limited
precision of floats) so you should stick to the same formula instead.

This isn't an exhaustive list of issues. Many of the above comments
apply to multiple places over the patch.



More information about the wine-devel mailing list