Skip to content
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

swizzle support #24

Merged
merged 4 commits into from Jun 6, 2019
Merged

swizzle support #24

merged 4 commits into from Jun 6, 2019

Conversation

recp
Copy link
Owner

@recp recp commented Jan 20, 2018

now as C math library cglm supports swizzling: https://www.khronos.org/opengl/wiki/Data_Type_(GLSL)#Swizzling

glm_vec_swizzle(vec4 v, int mask, vec4 dest)
glm_vec4_swizzle(vec4 v, int mask, vec4 dest)

Predefined masks:

/* vec3 */
GLM_XXX
GLM_YYY
GLM_ZZZ
GLM_ZYX

/* vec4 */
GLM_XXXX
GLM_YYYY
GLM_ZZZZ
GLM_WWWW
GLM_WZYX

Example usage (flip vec4 components):

vec4 vector = {12, 73.7, 0.936, 1.05};
glm_vec4_print(vector, stderr);

glm_vec4_swizzle(vector, GLM_WZYX, vector);
glm_vec4_print(vector, stderr);

/* 
Vector (float4):
	|12.0000   73.7000.  0.9360   1.0500|

Vector (float4):
	|1.0500.  0.9360.  73.7000   12.0000|

 */

This functions may be improved my SIMD instructions

@acoto87
Copy link
Contributor

acoto87 commented Jun 3, 2019

@recp I think this PR looks good, is there anything holding it back?

@recp
Copy link
Owner Author

recp commented Jun 3, 2019

Hi @acoto87

I was wanted to use SIMD shuffle macros/instructions for swizzling, IIRC I couldn't do that at first because SIMD mask must be immediate operand while our mask is variable :(

Maybe using macros could allow to use SIMD shuffle/blend... Maybe we can merge this PR and optimize some operations with macros like:

/* pre optimized swizzle */
#define glm_vec4_swizzle_wzyx(VECTOR) SIMD_here

the existing inline function can help to make swizzle with mask parameter as generic implementation. Should we merge this as it is for now?

@recp recp added this to the v0.6.0 milestone Jun 3, 2019
@acoto87
Copy link
Contributor

acoto87 commented Jun 3, 2019

...because SIMD mask must be immediate operand while our mask is variable :(

I'm not really sure about what do you mean with this.

Do you want to use this function?

__m128 _mm_shuffle_ps (__m128 a, __m128 b, unsigned int imm8)

Is there a problem with an implementation like:

CGLM_INLINE
void
glm_vec4_swizzle(vec4 v, int mask, vec4 dest) {
  vec4 t;

#ifdef __SSE__
  /* use of _mm_shuffle_ps here */
#else
  t[0] = v[(mask & (3 << 0))];
  t[1] = v[(mask & (3 << 2)) >> 2];
  t[2] = v[(mask & (3 << 4)) >> 4];
  t[3] = v[(mask & (3 << 6)) >> 6];
#endif

  glm_vec4_copy(t, dest);
}

I'm genuinely asking, I haven't tried something like this, and it maybe be a problem or it can't be done.

@recp
Copy link
Owner Author

recp commented Jun 4, 2019

Is there a problem with an implementation like:

AFAIK, yes. There is a problem.

__m128 _mm_shuffle_ps (__m128 a, __m128 b, unsigned int imm8)

The last parameter of _mm_shuffle_ps (imm8) is mask which is an immediate value which must be constant or literal. I couldn't pass our mask parameter to _mm_shuffle_ps as imm8. In C a parameter is a variable in its local scope, this is why I used 'variable' is my previous comment.

If you pass glm_vec4_swizzle()'s mask parameter to _mm_shuffle_ps as imm8, you will see that compiler will complain. Maybe there is a compiler option which makes the mask immediate or suppress this compiler error. Because glm_vec4_swizzle is inline function.

@acoto87
Copy link
Contributor

acoto87 commented Jun 5, 2019

This suggest that when the compiler can inline the functions, it will not throw an error: https://godbolt.org/z/NwYhhv
If you remove the inline on the suffle_test function, the compiler do complaint

EDIT: Nevermind, clang and msvc do complaint no matter what, only gcc seems to be less strict with this.

@recp
Copy link
Owner Author

recp commented Jun 5, 2019

Thank you for this test and sharing that, I tried for gcc 4.9.4, it also complained. It seems gcc >= 5 allows this, interesting. (FWIW, my main dev environment is macOS, clang)

Maybe we can optimize this for gcc 5+ later. But the problem is that gcc 10, 11... may not allow this... we should read/follow gcc's documentation to go forward if we want...

If the existing swizzle implementation is ok, then I'll merge it later? This is the best we can do for now, I guess.

To use SIMD, as I said above, we could apply optimizations to specific swizzle operations (most common ones):

#define glm_vec4_swizzle_wzyx(VECTOR) SIMD_here // reverse
#define glm_vec4_swizzle_xxxx(VECTOR) SIMD_here
#define glm_vec4_swizzle_yyyy(VECTOR) SIMD_here
...

// Or short versions which indicate swizzling? (I think I liked this):
#define glm_vec4_wzyx(VECTOR) SIMD_here // reverse
#define glm_vec4_xxxx(VECTOR) SIMD_here
#define glm_vec4_yyyy(VECTOR) SIMD_here

in this way, maybe we can make all compilers happy.

@acoto87
Copy link
Contributor

acoto87 commented Jun 5, 2019

Yeah, I think the default implementation should be good for now, and see if later can be optimized.
I think the macros (or maybe inline functions?) for the most common swizzle operations should be fine.
The short versions should be clear enough.

@recp
Copy link
Owner Author

recp commented Jun 5, 2019

or maybe inline functions?

Sorry, I was tried to make glm_vec4_swizzle(); macro to allow mask to be passed to SIMD, this is why I stuck with macro. Since we don't need a mask parameter for these, we can use inline functions 👍

this is why I stuck with macro

Let's postpone this too.

I'll merge this asap.

@coveralls
Copy link

coveralls commented Jun 6, 2019

Coverage Status

Coverage remained the same at 11.081% when pulling 2025b35 on swizzle into 3797c55 on master.

@recp recp added the feature label Jun 6, 2019
@recp recp merged commit 24de86c into master Jun 6, 2019
@recp recp deleted the swizzle branch June 6, 2019 10:24
@recp
Copy link
Owner Author

recp commented Jun 6, 2019

@acoto87 this PR is merged, thank you for your feedbacks and help!

@FrostKiwi
Copy link
Contributor

I find swizzling to be GLSL's best feature. When doing some projection math I love to specify .xy to multiply only the XY component of a vec3.

Now I needed to do the same on the CPU side and found misusing lower dimension functions for higher dimension vectors to work. Like glm_vec2_scale(vec3, float, vec2);.

But switching to the struct API this is not possible anymore, so I have to resort to creating a new struct there an then via
vec2s vec2_scale((vec3s){ vec3s.x , vec3s.y}, float);.

Now I wonder, whether there could be some unenthical MACRO MAGIC™ which would abstract away stuff like this to create faux advanced swizzle support.

@recp
Copy link
Owner Author

recp commented Jun 15, 2023

vec2s vec2_scale((vec3s){ vec3s.x , vec3s.y}, float);

One possible alternative could be:

// get WZ from vec3 then 
vec2s v2 = vec2_scale(glms_vec2(vec3_swizzle(v3, GLM_ZY)), float);

There could be helpers to create vector from others with swizzling by using vec[2,3,,4]_swizzle(v, GLM_XYZW...):

vec2_sw3(v3, GLM_ZY)
vec2_sw4(v3, GLM_WX)
vec3_sw4(v3, GLM_WXZ)

// then this could be possible:
vec2s v2 = vec2_scale(vec2_sw3(v3, GLM_ZY), float);

// if C++ is used, we can add some helpers to struct API for C++
vec2s v2 = vec2_scale(v3.zy(), float);

// even even basic operator wrappers top of actual functions maybe:
vec2s v2 = v3.zy() * scalar

better names can be used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants