Why is `shader.sort` a float instead of a `shaderType_t` enum?

[illwieckz]

While I worked on #682 I discovered shader.sort is a float.

Here is an example of patch turning it to a shaderSort_t instead of a float: 6aa28b0...illwieckz/shader-sort-no-float

But I don't know if we can do this.

Everywhere the code is setting values it is setting shaderSort_t enum values, except in one place in tr_shader.cpp when reading .shader files. It allows the mapper to not only set keywords like opaque or decal but also numbers, and the number is parsed as float:

Daemon/src/engine/renderer/tr_shader.cpp

Line 3602 in 6aa28b0

shader.sort = atof( token );

[…]
	else if ( !Q_stricmp( token, "nearest" ) )
	{
		shader.sort = shaderSort_t::SS_NEAREST;
	}
	else if ( !Q_stricmp( token, "postProcess" ) )
	{
		shader.sort = shaderSort_t::SS_POST_PROCESS;
	}
	else
	{
		shader.sort = (shaderSort_t) atof( token );
	}

Parsing this and storing this as a float not only means all comparisons are done as floats, but also every comparison between this variable and a shaderSort_t enum value like shaderSort_t::SS_NEAREST requires extra code, comparing shader.sort with Util:ordinal(shaderSort_t::SS_NEAREST).

This shader.sort = atof(token) is already present in original Quake 3 code: https://github.com/id-Software/Quake-III-Arena/blob/dbe4ddb10315479fc00086f08e25d968b4b43c49/code/renderer/tr_shader.c#L1303-L1305

I've noticed a .shader files from Quake 3 that sets two time a numeric value instead of using a keyword:

scripts/sfx.shader
215:      sort 5
228:       sort 6

According to the enum here: https://github.com/id-Software/Quake-III-Arena/blob/master/code/renderer/tr_local.h#L113-L138

The 5 and 6 values are just another way to write seeThrough or banner, maybe those numbers are a relic of a very old Quake3 version before those were properly named.

This shader.sort = atof(token) can also be found in Wolfenstein: Enemy Territory: https://github.com/id-Software/Enemy-Territory/blob/40342a9e3690cb5b627a433d4d5cbf30e3c57698/src/renderer/tr_shader.c#L1292-L1294

I would only one .shader file using numeric values in Wolfenstein: Enemy Territory files:

scripts/assault_rock.shader
20:	sort 16
44:	sort 16

The 16 value is for SS_NEAREST with a comment about “blood blobs”: https://github.com/id-Software/Enemy-Territory/blob/40342a9e3690cb5b627a433d4d5cbf30e3c57698/src/renderer/tr_local.h#L131

Since there is no keyword for that value in the .shader parser, setting this int is a kind of workaround for an incomplete parser.

You'll notice those are all integers, not floats, and parsing integers and doing shader.sort = atoi(token) would work.

But Quake3 code is parsing floats since more than 20 years, is there a good reason for that?

Both XreaL and ET:XreaL also do shader.sort = atof(token), this code is there since first commit of all those repositories and was never modified since:

https://sourceforge.net/p/xreal/svn/298/tree/trunk/xreal/code/renderer/tr_shader.c#l1304

https://sourceforge.net/p/xreal/ET-XreaL/ci/058289f3c9b016c331e1c364c128ecd1a11ad311/tree/src/renderer/tr_shader.c#l1293

Dæmon does as well and this code was never modified.

I haven't found a sort with a numeric value in default Tremulous .shader file. In all Tremulous maps formerly listed in betaserv.tk (more than 3000 .shader files) and handful of maps sets integer values, sometime they set sort 6 (which is decal) like in station15-r1.shader, some also set sort 16, in a similar way to Wolfenstein: Enemy Territory.

At the time I write this, I only found one file setting float values, that's one of us, an Unvanquished file, the mysterious binary.shader:
https://github.com/UnvanquishedAssets/unvanquished_src.dpkdir/blob/master/scripts/binary.shader

In this file, all the sort invocations use numeric values, and all are float values, they are 1537 invocations with float values.

All those values are between 15 and 16:

scripts/binary.shader
3:	sort 15.5004
14:	sort 15.5005
[…]
16111:	sort 15.6532
16121:	sort 15.6533
16131:	sort 15.5

Between 15 and 16 means they are sorted between what we have in our code being named SS_BLEND0 (with a comment saying “regular transparency and filters”) and SS_BLEND1 (with a comment saying “generally only used for additive type effects”): https://github.com/UnvanquishedAssets/unvanquished_src.dpkdir/blob/40459674e4c7daa03045a8a3b763925dd8d26294/scripts/binary.shader

Also, all of them having a different float value means they are all sorted in a way every one of them is sorted before and after another one.

This binary.shader file is there since the first commit in Unvanquished repository: https://github.com/Unvanquished/Unvanquished/blob/98b3a1480522dbd90660ee4c6bc91f6d80885286/main/scripts/binary.shader

This code is used there in cg_main.cpp : https://github.com/Unvanquished/Unvanquished/blob/bd8a6aa8dc0e7c3d0cf81d4ffd8785879f551a98/src/cgame/cg_main.cpp#L851-L861

	cgs.media.binaryAlpha1Shader = trap_R_RegisterShader("gfx/binary/alpha1", RSF_DEFAULT);

	for ( i = 0; i < NUM_BINARY_SHADERS; ++i )
	{
		cgs.media.binaryShaders[ i ].f1 = trap_R_RegisterShader(va("gfx/binary/%03i_F1", i), RSF_DEFAULT);
		cgs.media.binaryShaders[ i ].f2 = trap_R_RegisterShader(va("gfx/binary/%03i_F2", i), RSF_DEFAULT);
		cgs.media.binaryShaders[ i ].f3 = trap_R_RegisterShader(va("gfx/binary/%03i_F3", i), RSF_DEFAULT);
		cgs.media.binaryShaders[ i ].b1 = trap_R_RegisterShader(va("gfx/binary/%03i_B1", i), RSF_DEFAULT);
		cgs.media.binaryShaders[ i ].b2 = trap_R_RegisterShader(va("gfx/binary/%03i_B2", i), RSF_DEFAULT);
		cgs.media.binaryShaders[ i ].b3 = trap_R_RegisterShader(va("gfx/binary/%03i_B3", i), RSF_DEFAULT);
	}

It was imported in first Unvanquished commit as a code from gpp in cg_main.c: https://github.com/Unvanquished/Unvanquished/blob/98b3a1480522dbd90660ee4c6bc91f6d80885286/src/gamelogic/gpp/src/cgame/cg_main.c#L955-L964

The cg_main.c file from base didn't have it: https://github.com/Unvanquished/Unvanquished/blob/98b3a1480522dbd90660ee4c6bc91f6d80885286/src/gamelogic/base/src/cgame/cg_main.c

I had a look at the cg_main.c file from gpp branch of Tremulous repository and there is no mention of such gfx/binary shader: https://github.com/darklegion/tremulous/blob/gpp/src/cgame/cg_main.c

I have no idea what those gfx/binary shaders with float sort values are for.

And then I have no idea why shader.sort is a float instead of a shaderType_t enum.

[illwieckz]

Outside of the specific gfx/binary shader question, I have not found yet a place where the sort values are compared as float against another float.

[VReaperV]

I was wondering what those binary shaders were when looking through the assets... I thought they were some compressed shaders, but apparently not. I'll see if they can just be NUKED then.

[DolceTriade]

The binary shaders are used for buildable ranges right?

[VReaperV]

No, building ranges use gfx/colors/plain.

[VReaperV]

Apparently they're only used by CG_DrawBinaryShadersFinalPhases(), which checks cg_binaryShaderScreenScale. Cvar description: "I don't know". Amazing.

[VReaperV]

Apparently they're used by https://github.com/Unvanquished/Unvanquished/blob/1fe04368e6395811a55c4d9ef9e617152d9ccd84/src/cgame/cg_drawtools.cpp#L586 as well. Maybe they're used to prevent z-fighting on intersecting building ranges?