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

Regression in 7f2a5b89b30d143014bc0363b99dc3d942457ae7 #1255

Closed
mgerhardy opened this issue Feb 12, 2024 · 6 comments · Fixed by #1257
Closed

Regression in 7f2a5b89b30d143014bc0363b99dc3d942457ae7 #1255

mgerhardy opened this issue Feb 12, 2024 · 6 comments · Fixed by #1257

Comments

@mgerhardy
Copy link

mgerhardy commented Feb 12, 2024

template<typename T, qualifier Q>
GLM_FUNC_QUALIFIER GLM_CONSTEXPR vec<3, T, Q> operator/(vec<3, T, Q> const& v, T scalar)
{
  return vec<3, T, Q>(v) *= 1/scalar;
}

7f2a5b8

this change breaks stuff like

glm::ivec3 center = _mins + _width / 2;

this basically means that center is now _mins because the division leads to *=0 for any scalar > 1

mgerhardy added a commit to vengi-voxel/vengi that referenced this issue Feb 12, 2024
@dimitre
Copy link

dimitre commented Feb 13, 2024

I've noticed some template issues also when using the define
#define GLM_FORCE_NEON

@christophe-lunarg
Copy link

Ok, I'll reverse it. :/ @laurentcau

@laurentcau
Copy link

Strange to not fix the issue instead of revert. Never mind.
What is the template issues with GLM_FORCE_NEON ?

@laurentcau
Copy link

@dimitre

@remixer-dec
Copy link

Hello!

Just wanted to let you know that PR #1278 causes major performance degradation, I tested it on m1 mac.
I tested on this project: https://github.com/benikabocha/saba
I replaced the library in external/glm with the latest one and found that it performs significantly worse than previously released version (37 FPS -> 24 FPS). After rolling back the commits I found that #1278 is what is causing it.

screenshot

All this with base config build with SIMD off.

Next, I decided to turn it on to see if it helps.

    add_compile_options(-march=armv8-a+simd -mtune=apple-m1)
    add_definitions(
        -DGLM_FORCE_NEON
        -D__ARM_ARCH=8
    )

After manually enabling it with I faced multiple errors like

saba/external/glm/include/glm/./ext/../detail/type_vec_simd.inl:942:30: error: template argument for non-type template parameter must be an expression
                        return !compute_vec_equal<int, Q, false, 32, true>::call(v1, v2);

type_vec_simd.inl meaning that manually enabling SIMD/Neon worked, but compiler was not compatible with this syntax
after dealing with it and with errors like

./saba/src/Saba/Model/MMD/MMDModel.cpp:145:20: error: no viable conversion from 'mat<4, 4, [2 * ...]>' to 'const mat<3, 3, [2 * ...]>'
                        const glm::mat3 invZ = glm::scale(glm::mat4(1.0f), glm::vec3(1, 1, -1));
                                        ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.saba/src/Saba/Model/MMD/PMXModel.cpp:508:18: error: no viable overloaded '='
mat.m_diffuse = pmxMat.m_diffuse;
~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~
./saba/external/glm/include/glm/./ext/../detail/type_vec3.hpp:185:55: note: candidate function not viable: no known conversion from 'const vec<4, [2 * ...]>' to 'const vec<3, [2 * ...]>' for 1st argument
GLM_DEFAULTED_FUNC_DECL GLM_CONSTEXPR vec<3, T, Q>& operator=(vec<3, T, Q> const& v) GLM_DEFAULT;

./saba/src/Saba/Model/MMD/MMDModel.cpp:145:46: error: no matching conversion for functional-style cast from 'glm::vec3' (aka 'vec<3, float, defaultp>') to 'glm::mat3' (aka 'mat<3, 3, float, defaultp>')
const glm::mat3 invZ = glm::mat3(1.0f) * glm::mat3(glm::vec3(1, 1, -1));


I managed to successfully build the app. To my disappointment, the performance remained the same at 24FPS as with SIMD off, maybe I missed something, I'm not really a low-level developer, correct me if I'm wrong.

@dimitre
Copy link

dimitre commented Jan 14, 2025

I've just updated to latest commit in this runners here (building for multiple platforms)
and there are some template errors for some platforms.
using commit 18feaec
https://github.com/dimitre/ofLibs/actions/runs/12768319570/job/35588448387
using release tag 1.0.2
https://github.com/dimitre/ofLibs/actions/runs/12735527744

and here the CMake parameters for each platform
https://github.com/dimitre/ofLibs/blob/main/glm/chalet.yaml

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

Successfully merging a pull request may close this issue.

5 participants