Skip to content

Commit

Permalink
Lighting: remove pre-multiplication with material colors
Browse files Browse the repository at this point in the history
All these multiplications are already done by the TEV in
setup_render_scene(). The only exception to this is specular color, but
multiplying it with the light color is wrong anyway (we produce a weaker
light) and, most importantly, this light is already combined with the
material's ambient color by GX (see the previous commit's explanation).
  • Loading branch information
mardy authored and WinterMute committed May 5, 2024
1 parent cb795a0 commit 3f4dafb
Showing 1 changed file with 4 additions and 9 deletions.
13 changes: 4 additions & 9 deletions src/gc_gl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1912,18 +1912,12 @@ static LightMasks prepare_lighting()
if (gx_ambient) {
// Multiply the light color by the material color and set as light color
GXColor amb_col = gxcol_new_fv(glparamstate.lighting.lights[i].ambient_color);
if (!glparamstate.lighting.color_material_enabled) {
gxcol_mulfv(&amb_col, glparamstate.lighting.matambient);
}
GX_InitLightColor(gx_ambient, amb_col);
GX_InitLightPosv(gx_ambient, &glparamstate.lighting.lights[i].position[0]);
}

if (gx_diffuse) {
GXColor diff_col = gxcol_new_fv(glparamstate.lighting.lights[i].diffuse_color);
if (!glparamstate.lighting.color_material_enabled) {
gxcol_mulfv(&diff_col, glparamstate.lighting.matdiffuse);
}
GX_InitLightColor(gx_diffuse, diff_col);
GX_InitLightPosv(gx_diffuse, &glparamstate.lighting.lights[i].position[0]);
}
Expand All @@ -1939,9 +1933,6 @@ static LightMasks prepare_lighting()
}
if (gx_specular) {
GXColor spec_col = gxcol_new_fv(glparamstate.lighting.lights[i].specular_color);
if (!glparamstate.lighting.color_material_enabled) {
gxcol_mulfv(&spec_col, glparamstate.lighting.matspecular);
}

/* We need to compute the normals of the direction */
float normal[3] = {
Expand Down Expand Up @@ -2120,6 +2111,10 @@ static void setup_render_stages(int texen)
dcol = gxcol_new_fv(glparamstate.lighting.matdiffuse);
}

/* We would like to find a way to put matspecular into
* GX_SetChanMatColor(GX_COLOR0A0), since that's the color that GX
* combines with the specular light. But we also need this register
* for the ambient color, which is arguably more important. */
GX_SetChanMatColor(GX_COLOR0A0, acol);
GX_SetChanMatColor(GX_COLOR1A1, dcol);
}
Expand Down

0 comments on commit 3f4dafb

Please sign in to comment.