tr_shader: fix loose texture lighting, fix #289

[illwieckz]

Before (light mapping/vertex lighting):

After (light mapping/vertex lighting):

Don't mind the weird artifacts visible with vertex lighting, they come from another issue that is not yet tracked down (introduced between 0.50.0 and 0.51.1).

For some reason the loose textures were set to DIFFUSEMAP stageType (that seems correct) while the colorGen was set to CGEN_VERTEX (that seems not correct):

Daemon/src/engine/renderer/tr_shader.cpp

Lines 5464 to 5470 in 390a243

	case shaderType_t::SHADER_3D_STATIC:
	{
	// explicit colors at vertexes
	stages[ 0 ].type = stageType_t::ST_DIFFUSEMAP;
	stages[ 0 ].bundle[ 0 ].image[ 0 ] = image;
	stages[ 0 ].active = true;
	stages[ 0 ].rgbGen = colorGen_t::CGEN_VERTEX;

That seems not correct because the rest of the code sets and expects CGEN_IDENTITY colorGen for textures with DIFFUSEMAP stageType:

Daemon/src/engine/renderer/tr_shader.cpp

Lines 1530 to 1536 in 390a243

	static void ParseDiffuseMap( shaderStage_t *stage, const char **text, const int bundleIndex = TB_DIFFUSEMAP )
	{
	char buffer[ 1024 ] = "";
	stage->active = true;
	stage->type = stageType_t::ST_DIFFUSEMAP;
	stage->rgbGen = colorGen_t::CGEN_IDENTITY;

The issue was reproduced on 0.6.0 release and the wrong colorGen was seen in the very first commit of the repository:

Daemon/src/engine/rendererGL/tr_shader.c

Lines 5558 to 5564 in eb73a11

	case SHADER_3D_STATIC:
	{
	// explicit colors at vertexes
	stages[0].type = ST_DIFFUSEMAP;
	stages[0].bundle[0].image[0] = image;
	stages[0].active = qtrue;
	stages[0].rgbGen = CGEN_VERTEX;

The bug was not visible before because the diffuse map lighting code had blending disabled (that was disabling alpha blending too), the bug was uncovered when alpha blending was enabled for multitexture materials (they use diffuse map lighting code), see #277 (glsl/lightmapping: fix terrain alpha blending for collapsed materials).

[illwieckz]

Related line was introduced 13 years ago, there is high chance the code was already faulty (but the bug was probably already hidden for similar reasons):

https://sourceforge.net/p/xreal/svn/664/

changed lightmap handling
Date: Mon Feb 13 14:50:20 2006 +0000

It's fun to see the faulty line was introduced on 2006-02-13 and fixed on 2020-02-13, for its 14th birthday! 🎂

[illwieckz]

Tremulous 1.1.0 was released on 30th of May 2006, I probably played Tremulous for the first time on 4th of April 2006. So the bug I just fixed in that Tremulous child that is Unvanquished was introduced before I played Tremulous for the first time.

Any way, Tremulous did not have this bug, neither Unvanquished since the bug was dormant because of the bug is living in a feature Tremulous did not have and Unvanquished/Dæmon never enabled before, but that's crazy to think I just fixed a bug that predates the day I played Tremulous for the first time! 😲

[slipher]

LGTM - unless someone who understands this stuff wants to comment, since I don't.

[illwieckz]

I'm not sure to understand everything in that stuff but I understand this:

The loose textures were set to DIFFUSEMAP stageType with CGEN_VERTEX colorGen while the rest of the code sets and expects CGEN_IDENTITY colorGen for textures with DIFFUSEMAP stageType.

So the proposed fix looks to fulfill the need for a consistent rendering of diffuse maps in a way they are all wrongly or correctly rendered. We can then expect them to be rendered the same way either they are described in material files or not. Either the default rendering is wrong or not. it would be done the same.

About “is it the right way to render this” question we can see that those textures (both loose textures and ones described in materials) are now rendered the same way it was rendered before alpha blending feature was enabled on material-described diffuse maps so by enabling the feature and implementing this fix we added an optional feature for material-described diffuse maps without regression for material-described diffuse cases not using the feature, and without regression for non-material-described diffuse maps using the feature or not.