Skip to content

Commit

Permalink
replace GLM_FORCE_QUAT_CTOR_XYZW to GLM_FORCE_QUAT_DATA_XYZW
Browse files Browse the repository at this point in the history
  • Loading branch information
@junjie020 @christophe-lunarg
junjie020 authored and christophe-lunarg committed Feb 6, 2024
1 parent 38edba1 commit dcb8496
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion glm/detail/type_quat.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace glm

GLM_FUNC_DECL GLM_CONSTEXPR qua(T s, vec<3, T, Q> const& v);

# ifdef GLM_FORCE_QUAT_CTOR_XYZW
# ifdef GLM_FORCE_QUAT_DATA_XYZW
GLM_FUNC_DECL GLM_CONSTEXPR qua(T x, T y, T z, T w);
# else
GLM_FUNC_DECL GLM_CONSTEXPR qua(T w, T x, T y, T z);
Expand Down
2 changes: 1 addition & 1 deletion glm/detail/type_quat.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ namespace detail
{}

template <typename T, qualifier Q>
# ifdef GLM_FORCE_QUAT_CTOR_XYZW
# ifdef GLM_FORCE_QUAT_DATA_XYZW
GLM_FUNC_QUALIFIER GLM_CONSTEXPR qua<T, Q>::qua(T _x, T _y, T _z, T _w)
# else
GLM_FUNC_QUALIFIER GLM_CONSTEXPR qua<T, Q>::qua(T _w, T _x, T _y, T _z)
Expand Down

3 comments on commit dcb8496

@Kosmokleaner
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing but this new macro name is quite confusing, the CTOR was less confusing. See:
https://twitter.com/MittringMartin/status/1756041311952806261

@christophe-lunarg
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing but this new macro name is quite confusing, the CTOR was less confusing. See: https://twitter.com/MittringMartin/status/1756041311952806261

It is actually not a new name but the old name being used everywhere... including where the data is defined. A previous contribution added GLM_FORCE_QUAT_CTOR_XYZW I think to split the contructor order from the underlying data order but people reported issues so that contribution was made.

Well, I agree there is annoying legacy here... :/

@Kosmokleaner
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually not a new name but the old name being used everywhere

Where the data is defined a different but similar name is used,
GLM_FORCE_QUAT_DATA_XYZW vs GLM_FORCE_QUAT_DATA_WXYZ

I've seen the GLM_FORCE_QUAT_CTOR_XYZW split and removing that seems ok. But the data order is in memory only and quite independent.
With the new name I would expect to be able to customize the data order in more than 2 ways and that is not how the define is used.

Please sign in to comment.