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