Implement f16_t, f16vec2_t, f16vec4_t half float types for safety

[illwieckz]

Implement f16_t, f16vec2_t, f16vec4_t half float types for safety, also fix some bugs where stuff should have been converted to or from halfFloat.

Now the way the type is defined, there should be a compilation error when passing a f16_vec4t to a i16_vec4t etc.

---

Original mesage:

It's a very minor fix, I noticed shaderVertex_t.texCoords was defined with i16vec4_t type while being the output of floatToHalf(vec4_t, f16vec4_t).

It worked because f16vec4_t is defined as i16vec4_t.

I caught that by experimenting with different (half-)float vertex formats (alternate code for missing ARB_half_float_vertex):

src/engine/renderer/tr_surface.cpp: In function ‘void Tess_AddSprite(const vec_t*, Color::Color32Bit, float, float)’:
src/engine/renderer/tr_surface.cpp:432:28: error: no matching function for call to ‘floatToHalf(vec4_t, i16vec4_t)’
  432 |                 floatToHalf( texCoord, tess.verts[ ndx + i ].texCoords );
      |                 ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I added an extra commit making more obvious that f16vec4_t is defined as i16vec4_t anyway.

[necessarily-equal]

I've come up with this too while having a look

diff --git a/src/engine/renderer/tr_local.h b/src/engine/renderer/tr_local.h
index b253b431..45e2a76a 100644
--- a/src/engine/renderer/tr_local.h
+++ b/src/engine/renderer/tr_local.h
@@ -44,7 +44,8 @@ using i16vec4_t = int16_t[4];
 using u16vec4_t = uint16_t[4];
 using i16vec2_t = int16_t[2];
 using u16vec2_t = uint16_t[2];
-using f16vec4_t = int16_t[4]; // half float vector
+using f16vec2_t = i16vec2_t; // half float vector
+using f16vec4_t = i16vec4_t; // half float vector
 
 // GL conversion helpers
 static inline float unorm8ToFloat(byte unorm8) {
@@ -3096,7 +3097,7 @@ inline bool checkGLErrors()
 
        void R_CalcTangents( vec3_t tangent, vec3_t binormal,
                             const vec3_t v0, const vec3_t v1, const vec3_t v2,
-                            const i16vec2_t t0, const i16vec2_t t1, const i16vec2_t t2 );
+                            const f16vec2_t t0, const f16vec2_t t1, const f16vec2_t t2 );
 
        /*
         * QTangent representation of tangentspace:
diff --git a/src/engine/renderer/tr_main.cpp b/src/engine/renderer/tr_main.cpp
index 6afcd563..8d466e1c 100644
--- a/src/engine/renderer/tr_main.cpp
+++ b/src/engine/renderer/tr_main.cpp
@@ -102,7 +102,7 @@ void R_CalcTangents( vec3_t tangent, vec3_t binormal,
 
 void R_CalcTangents( vec3_t tangent, vec3_t binormal,
                     const vec3_t v0, const vec3_t v1, const vec3_t v2,
-                    const i16vec2_t t0, const i16vec2_t t1, const i16vec2_t t2 )
+                    const f16vec2_t t0, const f16vec2_t t1, const f16vec2_t t2 )
 {
        vec2_t t0f, t1f, t2f;
 
diff --git a/src/engine/renderer/tr_model_md5.cpp b/src/engine/renderer/tr_model_md5.cpp
index e59652fc..701a0d38 100644
--- a/src/engine/renderer/tr_model_md5.cpp
+++ b/src/engine/renderer/tr_model_md5.cpp
@@ -325,7 +325,7 @@ bool R_LoadMD5( model_t *mod, void *buffer, const char *modName )
                        for (unsigned k = 0; k < 2; k++ )
                        {
                                token = COM_ParseExt2( &buf_p, false );
-                               v->texCoords[ k ] = atof( token );
+                               v->texCoords[ k ] = floatToHalf( atof( token ) );
                        }
 
                        // skip )

I think the last change may be an actual bugfix

[illwieckz]

This one breaks md5 texturing:

diff --git a/src/engine/renderer/tr_model_md5.cpp b/src/engine/renderer/tr_model_md5.cpp
index e59652fc..701a0d38 100644
--- a/src/engine/renderer/tr_model_md5.cpp
+++ b/src/engine/renderer/tr_model_md5.cpp
@@ -325,7 +325,7 @@ bool R_LoadMD5( model_t *mod, void *buffer, const char *modName )
                        for (unsigned k = 0; k < 2; k++ )
                        {
                                token = COM_ParseExt2( &buf_p, false );
-                               v->texCoords[ k ] = atof( token );
+                               v->texCoords[ k ] = floatToHalf( atof( token ) );
                        }
 
                        // skip )

Before:

After:

Edit: probably because of:

Daemon/src/engine/renderer/tr_local.h

Lines 2176 to 2182 in cdc6421

	ALIGNED(16, struct md5Vertex_t
	{
	vec4_t position;
	vec4_t tangent;
	vec4_t binormal;
	vec4_t normal;
	vec2_t texCoords;

[illwieckz]

Note to myself for things that may be done in other PRs after this one:

• IQModel_t can be defined using f16vector<N>_t instead of pointers to f16_t, same for the allocation in R_LoadIQModel.
  Edit: no or not in a straightforward manner, pointers are iterated on the data.
• To support systems without ARB_half_float_vertex, the magic will probably happen in R_CopyVertexData by converting texCoords back to float (plus setting GL_FLOAT anytime GL_HALF_FLOAT is set).

[illwieckz]

Damn, it looks like my patch to convert md5mesh texcoords to half-float as soon as possible breaks alignment…

[illwieckz]

I removed the commit to convert md5mesh texCoords as half-floats as soon as possible because that meant the CPU code (likely used on older and slower hardware) had to do the conversion back to float to do the computations so it would slow down this code path, and it also breaks alignment so that would also slow down this code path more.

It may be possible to convert them to half float as soon as possible when using the GPU code path in a way it may improve the GPU code path but this out of topic of this PR (there is no regression).

I renamed the md5mesh texCoords as texCoordsF to avoid confusion as it is the only one texCoords in code base to not be half-float.

[illwieckz]

So vboData_t::spriteOrientation is never set anywhere (meaning the faulty Vector4Copy is dead code), and it was that way since the field was introduced in b10e8c2. I guess the "autosprite" thing (no idea what it is) has probably been broken at least since then. I guess we should delete the RSF_SPRITE stuff unless somebody has any idea what it is supposed to do.

@slipher note that we have some known bugs related to sprites. Some can be seen in metro map, in catacombs: missing fire sprites, distorted chain sprite…

I would not be surprised if that was the incomplete code for those features. Actually gimhael fixed/completed some other autosprite code for other sprites in metro at this time.

[slipher]

Can this be squashed? Somehow there are 10 commits to do what was not very big as 1 commit...

[illwieckz]

I squashed all the commits that were converting i-types to f-types into one.

[illwieckz]

@DolceTriade any comment? 🙂️

[DolceTriade]

I don' t understand much of it, but it seems ok to me.