Convert vector macros to template functions #1181
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is good because it prevents multiple evaluation of the arguments. As a live example of this bug, the CG_CalculateTimeFrac inside a VectorMA in CG_RenderParticle in Unvanquished/src/cgame/cg_particles.cpp would be evaluated 3 times. Also it may be useful to copy a glm::vec3 returned from a function into a vec3_t without needing a temporary variable.
DolceTriade
reviewed
Jun 7, 2024
Comment on lines
+39
to
+41
| #define DotProduct4(x, y) (( x )[ 0 ] * ( y )[ 0 ] + ( x )[ 1 ] * ( y )[ 1 ] + ( x )[ 2 ] * ( y )[ 2 ] + ( x )[ 3 ] * ( y )[ 3 ] ) | ||
| #define VectorMultiply( a,b,c ) ( ( c )[ 0 ] = ( a )[ 0 ] * ( b )[ 0 ],( c )[ 1 ] = ( a )[ 1 ] * ( b )[ 1 ],( c )[ 2 ] = ( a )[ 2 ] * ( b )[ 2 ] ) | ||
|
|
There was a problem hiding this comment.
Since they were only used in one file, it seemed to me they are not suitable for inclusion in the header included by all files. And with only 1-2 uses in 1 file they should probably be normal functions not templates, but I didn't want to spend time on them.
DolceTriade
approved these changes
Jun 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The point of this is to prevent the arguments from being evaluated multiple times. In the commit message I give an example of this happening in cgame.
Also it can be useful for assigning a temporary GLM vector to a vec3_t. This is one more GLM/vec3_t adaptor we need in addition to GLM4READ and GLM4RW. I was motivated to make this branch upon seeing, in a GLM conversion branch, VectorCopy used to assign a function's GLM return value to a vec3_t (heretofore a bad idea as the function is called 3 times).