Re: d3dx9 [patch 1/2]: Implement D3DXSHEvalDirectionalLight

[Rico Schüller]

On 17.09.2012 00:33, Nozomi Kodama wrote:
> +    { { rout, gout, bout, table, &(table[90]), &(table[180]), 1.01f, 1.02f, 1.03f, },
> ...
> +      { rout, bout, gout, table, &(table[90]), &(table[180]), 1.01f, 1.02f, 1.03f, },
> +      { bout, rout, gout, table, &(table[90]), &(table[180]), 1.01f, 1.02f, 1.03f, },

I think it doesn't make much sense to test this, they are doing 
basically all three the same. The name of the variable doesn't matter. 
Besides we test for (rout, rout, rout), these might be good to test: 
(rout, gout, gout), (rout, rout, bout), (rout, gout, rout).

Maybe the naming of the variables is a bit confusing... I used the 
starting name for the name, hence you could also use the expected value 
name or rename the variables to out1, out2, out3 to not make confusion 
with the expected / started color. Though I'm fine with all cases...
> +            }
> +
> +        startindex += order * order;
> +        }

I would indent the startindex...

Some style comments (I have no strong opinion about that, just like to 
mention it):
The brackets "()" in &(table[90]) could be left out. You may also break 
the longer lines into two. Though, I don't know the actual recommended 
line length, but 200 seems a bit long... I only found this 
http://www.winehq.org/pipermail/wine-devel/2010-September/086996.html .

Cheers
Rico

d3dx9 [patch 1/2]: Implement D3DXSHEvalDirectionalLight

[Henri Verbeet]

On 17 September 2012 09:52, Rico Schüller <kgbricola(a)web.de> wrote:
> longer lines into two. Though, I don't know the actual recommended line
> length, but 200 seems a bit long... I only found this
> http://www.winehq.org/pipermail/wine-devel/2010-September/086996.html .

Yeah, 100 is safe, but it's not a hard limit. Personally I use 120 in wined3d.