-
Notifications
You must be signed in to change notification settings - Fork 61
renderer: implement ivec_t and the ability to send integer vectors to GLSL #1040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Right now this code is not used, but I stumbled upon this need years ago, and I may use it in the future. My linear light computation code in #1034 currently makes use of it: Though, since that code uses a In all cases, as I said I found it very annoying that we can only send a single integer and not vector integer while we can send single float and float vector, so having this implemented makes the engine easier to deal with when implementing stuff. |
We should add this if/when it is actually used by something. That said, please don't add more stuff to |
I usually agree with this, but here, definitely not. Those are generic types, it's a library. I faced the need of this more than once in the last years, and I always had to rely on alternative solutions. This is even needed when not merging code that use it, because this is needed to prototype implementations and evaluate solutions. That's why I gave the example that maybe some legit usage of
This is doable, though this is a very basic generic type system, and the float vectors are defined in This work is not renderer-specific, it's engine architecture and fundamental type implementation. It just happens that I need this in renderer first because I'm focusing on the renderer right now. We did similar work for float vector types when I worked on the model code. This PR is anothere step on that same road. Last time for models, today for some rendering techniques, but it's generic ground work for the whole engine, not just the renderer. |
If there are many cases in previously written code where it would have been useful, you should be able to find one of those and migrate it? I'm not saying you need to use every length of vector, but at least one of GLUniform2i/GLUniform3i/GLUniform4i/. Anyway
The above code compiles without protest because 'f''s declaration is equivalent to Furthermore if you're going to have a general-purpose integer vector typedef, it should at least be decent enough what size of integer it is using (like You might consider going without a typedef at all and doing it like |
Right, immediately after having posted my comment I remember that what we had converted before was not So right now I looked how to implement a struct vec_t {
operator float() const { return bits; };
float bits;
};
using vec2_t = vec_t[2];
using vec3_t = vec_t[3];
using vec4_t = vec_t[4]; But then I struggle to implement the In the end I would like to replace the inline void VectorCopy( const vec2_t in, vec2_t out )
{
out[ 0 ] = in[ 0 ];
out[ 1 ] = in[ 1 ];
}
inline void VectorCopy( const vec3_t in, vec3_t out )
{
out[ 0 ] = in[ 0 ];
out[ 1 ] = in[ 1 ];
out[ 2 ] = in[ 2 ];
}
inline void VectorCopy( const vec4_t in, vec4_t out )
{
out[ 0 ] = in[ 0 ];
out[ 1 ] = in[ 1 ];
out[ 2 ] = in[ 2 ];
out[ 3 ] = in[ 3 ];
}
#define Vector2Copy VectorCopy
#define Vector4Copy VectorCopy And the same with all the other vector macro (subtraction, addition…). |
(I edited my previous comment as some wording was saying the contrary of what I meant). |
If you must use an array-like type for some reason you can use |
Everytime I faced the need myself I went up with other implementations anyway, and as I said, most of the time the ivec disappeared after multiple iteration in development, but this is luck. For example by implementing a compute in GLSL for every pixel, one may read the canonical documentation for the feature to implement and need an ivec3 for three numbers. This is the first iteration. This is a direct translation from a generic math algorithm to GLSL. Then later it is discovered the result of the computation using the ivec3 can be precomputed once for all in the C++ code, instead of doing it it per pixel. So in the next development iteration, the compute on the ivec3 is done in some C++ file and a single integer for the result is sent to be used by GLSL. The removal of the need for sending the ivec3 then becomes an optimization. It is not guaranteed that any ivec3 computation we may want to do will be exportable to C++ instead in a way only a single integer result is needed. This is just luck if it happened before. And in all cases, this is very annoying to have to think about the optimization at the same time the naive implementation is done. I want to be able to do naive implementation first, to validate they work, then to think about optimization to save the amount of variable we pass to GLSL and to reduce the GLSL code. Many work we merged without using any ivec2/ivec3 were optimized code from which the naive implementation in the first iteration were using ivec2/ivec3, so I had to rely on float vectors as booleans or to copy paste many single-integer variable sending code, just to deleted all of that before submitting the PR when everything was optimized after 3 or 4 optimization passes. One good example I have in mind is some existing code using some |
Ah, I forgot about I'm trying to convert I did that: using vec_t = float;
using vec2_t = std::array<vec_t, 2>;
using vec3_t = std::array<vec_t, 3>;
using vec4_t = std::array<vec_t, 4>; I'm now getting this:
Any idea to solve those cast issues? |
With help from @DolceTriade on the chat, I did this: diff --git a/src/engine/qcommon/q_shared.h b/src/engine/qcommon/q_shared.h
index e27ab1832..a191b3398 100644
--- a/src/engine/qcommon/q_shared.h
+++ b/src/engine/qcommon/q_shared.h
@@ -226,10 +226,9 @@ void Com_Free_Aligned( void *ptr );
*/
using vec_t = float;
- using vec2_t = vec_t[2];
-
- using vec3_t = vec_t[3];
- using vec4_t = vec_t[4];
+ using vec2_t = std::array<vec_t, 2>;
+ using vec3_t = std::array<vec_t, 3>;
+ using vec4_t = std::array<vec_t, 4>;
using axis_t = vec3_t[3];
using matrix3x3_t = vec_t[3 * 3];
@@ -1120,15 +1119,15 @@ inline float DotProduct( const vec3_t x, const vec3_t y )
inline __m128 sseLoadVec3( const vec3_t vec ) {
__m128 v = _mm_load_ss( &vec[ 2 ] );
v = sseSwizzle( v, YYXY );
- v = _mm_loadl_pi( v, (__m64 *)vec );
+ v = _mm_loadl_pi( v, (__m64 *) &vec[0] );
return v;
}
ATTRIBUTE_NO_SANITIZE_ADDRESS inline __m128 sseLoadVec3Unsafe( const vec3_t vec ) {
// Returns garbage in 4th element
- return _mm_loadu_ps( vec );
+ return _mm_loadu_ps( &vec[0] );
}
inline void sseStoreVec3( __m128 in, vec3_t out ) {
- _mm_storel_pi( (__m64 *)out, in );
+ _mm_storel_pi( (__m64 *) &out[0], in );
__m128 v = sseSwizzle( in, ZZZZ );
_mm_store_ss( &out[ 2 ], v );
} This solved the first errors, but then I face many other errors, I may need to port many stray bool CM_PlaneFromPoints( vec4_t plane, const vec3_t a, const vec3_t b, const vec3_t c )
{
- vec3_t d1, d2;
+ vec3_t d1, d2, d3;
VectorSubtract( b, a, d1 );
VectorSubtract( c, a, d2 );
- CrossProduct( d2, d1, plane );
- if ( VectorNormalize( plane ) == 0 )
+ CrossProduct( d2, d1, d3 );
+
+ if ( VectorNormalize( d3 ) == 0 )
{
return false;
}
- plane[ 3 ] = DotProduct( a, plane );
+ plane[ 0 ] = d3[ 0 ];
+ plane[ 1 ] = d3[ 1 ];
+ plane[ 2 ] = d3[ 2 ];
+ plane[ 3 ] = DotProduct( a, d3 );
return true;
} It looks like implementing a type-safe |
Right now we can only send a single integer to GLSL, or float vectors. Meaning that if we need to sends more than one integer, we either have to send multiple variables, or to send integers in floats vectors.
This implements the
ivec_t
type in engine and implements the related classes to sendivec2
,ivec3
andivec4
to GLSL in a similar way than what we already do forvec2
,vec3
andvec4
.