Restrict use of color vec components to XYZW

[dimateos]

This allows to compile GLM using GLM_FORCE_XYZW_ONLY, which personally simplifies debugging etc.

[paulhoux]

I see your point. But this change will affect projects that depend on the .r, .g and .b properties. If projects update to a Cinder version containing this change, they will brake. I don't think Cinder guarantees backward compatibility, but it sure would be a pain to fix your code when upgrading, while making sure no new errors are introduced.

I will let other chime in.

[dimateos]

Hmm I think it would work.

Here I only changed the source files accessing .rgba components of glm::vectors to use .xyzw, but I am not enforcing GLM_FORCE_XYZW_ONLY itself nor touching any cinder defined types.

Basically, is hard to see in the diff but if you look closely, these PR targets only usages INSIDE functions that take a glm::vector as a param. All colorT types still have rgba etc, publicly and also used internally.

Overall, I think the public API reflects no changes at all with this PR. So simply whoever prefers it can use GLM_FORCE_XYZW_ONLY without having to touch cinder sources.

The only issue I could forsee would be future changes that forget about using glm::vec::xyzw internally only. But again would only affect people who want to use GLM_FORCE_XYZW_ONLY, and who could post another fix.

Idk tests pass too, but I could be missing some insights like you say etc. Anyway, thanks for checking it out!

[paulhoux]

Ok, so what you're saying is that vec2's, vec3's and vec4's still have their r, g, b and a properties? Or am I misunderstanding the purpose of the PR? I should try your branch for myself, of course, but am a little short on time. My understanding was that you'd like to get rid of the union defined in vec, so that it only uses x, y, z and w. This makes it easier to debug, as the 4 values would all show up in the debugger.

[dimateos]

Yes, the vectors keep those components if you compile the code with no additional GLM defines.

The purpose of this PR is to make the code compatible with the GLM_FORCE_XYZW_ONLY option, which does exactly what you said:

https://github.com/g-truc/glm/blob/master/manual.md#-215-glm_force_xyzw_only-only-exposes-x-y-z-and-w-components

In my case, as an end user I compile my code with this flag. But I also compile it along cinder to reflect modifications to cinder sources.

So to my surprise, to be able to compile all the code with the same flags I just had to make these few edits to color.h and .cpp

[richardeakin]

I think this makes sense - libcinder sources itself should try to make as few assumptions about the GLM macro settings that people are using if it can. The swizzle versions of vecs could easily be avoided in the internal code, so that users can choose their macros and recompile from source if they wish.

[dimateos]

That was my rationale behind sharing these changes in a PR :)

In the case of swizzling, it is disabled by default and only available when compiling GLM with GLM_FORCE_SWIZZLE:

https://github.com/g-truc/glm/blob/master/manual.md#-214-glm_force_swizzle-enable-swizzle-operators

So cinder is not currently using it. In fact, there are no additional GLM defines. I have seen it in some code as part of shader MACROS tho, but that is totally fine. Furthermore, defining GLM_FORCE_XYZW_ONLY as in my situation forcefully disables swizzling.

I think these two defines are the most common for people to use if any, so it seems resonable to keep the code compatible with them in mind. That would be:

• Use XYZW only, for users potentially defining GLM_FORCE_XYZW_ONLY.
  • Note: would only affect glm::vecX not ColorT wrappers and derived defined by cinder.
• Avoid swizzling, for users potentially wanting to NOT define GLM_FORCE_SWIZZLE.
  • Note: this is the current situation, plus it is desirable because swizzling in GLM makes binaries larger!

As of now, this PR makes the minimal changes to comply with the stated above (just had to make it compatible with GLM_FORCE_XYZW_ONLY).

Also, previously I only compiled part of cinder (no audio nor video), but now I checked the whole library in desktop WINDOWS and everything compiles fine. Probably GLM as part of the core is unlikely to be used differently on other platforms, but tested tho.