Re: d3dx9_36 [patch 4/5]: Implement D3DXSHEvalConeLight

[Rico Schüller]

On 09.10.2012 09:50, Nozomi Kodama wrote:
> +static void CapIntegrale(FLOAT *out, FLOAT order, FLOAT angle)
> +{

I'd use a lower case name.
> - if (order == 5)
> - return;

Indentation. If you break to a new line, please indent 8 spaces (in the 
case for ok(...)).
> -
>    2.0f * sqrtf(D3DX_PI / 7.0f), 2.0f / 3.0f * sqrtf(D3DX_PI), 2.0f * sqrt (D3DX_PI / 11.0f) };

sqrtf.
> - FLOAT cap[6], clamped_angle, norm, scale, temp[36];

No need for temp/scale? (see below)
> - TRACE("order %u, dir %p, radius %f, red %f, green %f, blue %f, rout %p, gout %p, bout %p\n", order, dir, radius, Rintensity, Gintensity, Bintensity, rout, gout, bout);

Long line. Please break.
> - for (i = 0; i < order ; i++)

Space.
> - if (gout)
> -
>    for (i = 0; i < order; i++)
> -
>    {
> -
>        scale = cap[i] * Gintensity * coeff[i] / norm;
> -
>        for (j = 0; j < 2 * i + 1; j++)
> -
>            gout[i * i + j] = temp[i * i + j] * scale;
> -
>    }
> -
> - if (bout)
> -
>    for (i = 0; i < order; i++)
> -
>    {
> -
>        scale = cap[i] * Bintensity * coeff[i] / norm;
> -
>        for (j = 0; j < 2 * i + 1; j++)
> -
>            bout[i * i + j] = temp[i * i + j] * scale;
> -
>    }

Pretty much the same calculation is done again and again. What happens 
if order is 100? You'll get a access violation. This is also true for 
cap and coeff in my sample here (It needs some tests ... Order may be 
clamped to D3DXSH_MAXORDER ... Does native crash in that case? Should we 
touch the values for order >= D3DXSH_MAXORDER at all?):

D3DXSHEvalDirection(rout, order, dir);

for (i = 0; i < order; i++)
{
     FLOAT scale = cap[i] * coeff[i] / norm;

for (j = 0; j < 2 * i + 1; j++)
     {
         FLOAT temp = rout[i * i + j] * scale;

rout[i * i + j] = temp * Rintensity;
         if (gout) gout[i * i + j] = temp * Gintensity;
         if (bout) bout[i * i + j] = temp * Bintensity;
     }
}

Cheers
Rico