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

Unexpected compMin behavior. #1215

Closed
MaxSac opened this issue Jan 11, 2024 · 3 comments · Fixed by #1240
Closed

Unexpected compMin behavior. #1215

MaxSac opened this issue Jan 11, 2024 · 3 comments · Fixed by #1240

Comments

@MaxSac
Copy link

MaxSac commented Jan 11, 2024

I tried to calculate the compMin from a vector which includes a NaN.
The result seems to be dependent on the position of the NaN in the vector.
It may be a different behavior than the c++ standard suggests [IEEE floating-point arithmetic (IEC 60559)] (and my naive intuition 😆 ).

#include <glm/glm.hpp>
#include <glm/gtx/component_wise.hpp>
#include <iostream>
#include <limits>

int main(int argc, char* argv[])
{
    // Option I.
    std::cout << glm::compMin(glm::vec2(std::numeric_limits<float>::quiet_NaN(), 1.f)) << std::endl;
    // >>> nan
    std::cout << glm::compMin(glm::vec2(1.f, std::numeric_limits<float>::quiet_NaN())) << std::endl;
    // >>> 1

     // Option II.
    std::cout << std::fmin(std::numeric_limits<float>::quiet_NaN(), 1.f) << std::endl;
    // >>> 1
    std::cout << std::fmin(1.f, std::numeric_limits<float>::quiet_NaN()) << std::endl;
    // >>> 1

    return 0;
}

Is this kind of functionality intentional?

@gottfriedleibniz
Copy link

gottfriedleibniz commented Jan 11, 2024

GLSL defines min as:

Returns y if y < x; otherwise it returns x

which should match std::min's behaviour (not std::fmin). Unfortunately, the inconsistency in special-value behaviour between IEEE-operators vs ternary-logic will emerge elsewhere too.

@christophe-lunarg
Copy link

Erm, the comment, the example don't match. Without clarification, I will just close this issue. Maybe that's a feature request for a fcompMin function. unclear.

@MaxSac
Copy link
Author

MaxSac commented Jan 12, 2024

Sorry for the incorrect minimal working example. I wrote it yesterday in a hurry and have now corrected it.

To give a little bit of background.
I tried to calculate the interaction distance from a ray to a box, as sketched in the picture.
sketch-1

I wrote the following code and was quite surprised that a) and b) gave different results.

#include <array>
#include <glm/glm.hpp>
#include <glm/gtx/component_wise.hpp>
#include <iostream>
#include <limits>

std::array<float, 2> intersection(glm::vec2 corner1, glm::vec2 corner2,
    glm::vec2 origin, glm::vec2 invDirection)
{
    auto tMin = (corner1 - origin) * invDirection;
    auto tMax = (corner2 - origin) * invDirection;

    auto tEnter = glm::min(tMin, tMax);
    auto tExit = glm::max(tMin, tMax);

    auto enterMax = glm::compMax(tEnter); 
    auto exitMin = glm::compMin(tExit);  

    return std::array<float, 2> { enterMax, exitMin };
}

int main(int argc, char* argv[])
{
    auto c1 = glm::vec2(1, 1);
    auto c2 = glm::vec2(2, 2);

    // a)
    auto origin1 = glm::vec2(1, 0);
    auto invDirection1 = glm::vec2(std::numeric_limits<float>::quiet_NaN(), 1.f);
    auto intersection_points1 = intersection(c1, c2, origin1, invDirection1);
    std::cout << "entry: " << intersection_points1[0] << ", exit: " << intersection_points1[1] << std::endl;
    // >>> entry: nan, exit: nan


    // b)
    auto origin2 = glm::vec2(0, 1);
    auto invDirection2 = glm::vec2(1.f, std::numeric_limits<float>::quiet_NaN());
    auto intersection_points2 = intersection(c1, c2, origin2, invDirection2);
    std::cout << "entry: " << intersection_points2[0] << ", exit: " << intersection_points2[1] << std::endl;
    // >>> entry: 1, exit: 2

    return 0;
}

Technically I can understand what happens, but it was against my naive expectation.
I guess using something like fcompMax would help in this case and would be a nice feature.

MaxSac pushed a commit to MaxSac/glm that referenced this issue Jan 25, 2024
@christophe-lunarg christophe-lunarg linked a pull request Feb 6, 2024 that will close this issue
larioteo added a commit to dodecai/hedron that referenced this issue Aug 5, 2024
33b4a621a6 Update GitHub Actions
45008b225e Fixed vec equality check function from the compute_vector_decl.hpp file
a2844eede8 Use [[deprecated]] when CXX standard is at least 14
0904870e37 Fix log2 func. qualifier
4137519418 Simd improvement
ab913bbdd0 Add value_ptr method for vec1 types
c32a481dd4 Fix additional clang issues
05c93eeae0 Use value_ptr in packing.inl
0df8dcb454 Supporess unused-variable warnings
08a11905cf Fix sign-compare warnings
c48d16b911 Fix sequence-point warnings
7a812397a2 Disable unit tests by default to avoid C.I. time out
61caae4d05 Fix GTX_norm cyclic include
e009bcbe7c Update hash.hpp to detect msvc
ab2d7b4291 Release: Light releases are 'normal' release packages
49942a611c Fixed typos
dcc5cfdc4a Cast clock_t to match printf format specifier
f8df2f3e2e Trying to fix C.I. timeout...
be3beb7788 Disable test that time out on C.I.?
0892ccd214 Quicker tests for C.I.
1f25000a30 Quicker unit tests
b9424441b1 Add automatic release
3ac3589ed2 Fix GTX_number_precision build #1258
adf31f555e Revert SIMD improv 7f2a5b89b30d143014bc0363b99dc3d942457ae7
5d73e17e58 Added C++17 [[nodiscard]] support #1217
1ac95994c4 Fixed SIMD smoothstep
b101e8f3de Fixed  SIMD implementation #1222
90f2b025b1 Fixed EXT_matrix_transform inverted shear matrix multiple #1140 #1182
7adb4a5040 changelog: Added aligned_*vec3 simd support #1245
9e72e5ae67 Fix and ignore warnings
7f2a5b89b3 Simd improvement
88a6ed6ee1 Visual C++: Enables /Wall
4eb3fe1d7d fcomp: Fix build in C++98 mode
c9ca4dc77c Implementing fcompMin / fcompMax, closes g-truc/glm#1215
7b53739128 Only enable compiler warnings and warnings as error on unit tests
ce1362faba Updated changelog
b251b22d00 Fixed C++ language auto detection build, disable C++98 warnings with Clang #1235, #1231
31a5f56a7c Fix epsilon not declared in color_space.inl #1233
7872e05246 Update logo
48336e2637 Update logo
dcb8496300 replace GLM_FORCE_QUAT_CTOR_XYZW to GLM_FORCE_QUAT_DATA_XYZW
38edba1818 Avoid warnings about comparisons being always true
8ebe4b5e57 Fix compare that is always true
33b0eb9fa3 Updated doc
d58f9bcfed GLM bumps to version 1.0.0
673a963a0f Added description for fetching glm with cmake in readme.md
f86092a658 Remove disabled warnings (#1213)
b06b775c1c C.I.: Enable manually run tests on PRs
da9a21d7e3 Add C++ 20 Modules (#1208)
1d8467f606 Annotate swizzle operations with `GLM_FUNC_QUALIFIER`
d2033739cf Fix quat angle documentation #820
8d337c0c65 Fix quat packing XYZW usage (#1204)
5ce98b7514 Fix hash message (#1205)
b6618171dd Fix GLM_GTX_hash (#1202)
a40974fb86 Test multiple platforms to test multiple compiler versions (#1199)
7882684a2c Update manual.md
5700afbbf8 Add missing extensions in ext.hpp
8e2bdd1fdb Add GLM_FORCE_UNRESTRICTED_FLOAT to skip static assert when using not float types
869f9da00e Add GLM_FORCE_UNRESTRICTED_FLOAT to skip static assert when using not float types
229f3eced4 Add GLM_FORCE_UNRESTRICTED_FLOAT to skip static assert when using not float types
2993560ec9 Remove dead files
89850e6f4b Update readme
46b796dd79 Optimize cmake targets
e4dd44d584 Trying to make sure external contributor trigger C.I.
e98ad7c9f8 Avoid duplicated C.I. runs
e357f58c96 gni
cf69137d6d quaternion: Revert #1069
1cc8e80e3b Make mat operators constexpr
fc236e0bf8 Adding constexpr qualifier for helper functions #1050 (#1184)
4ecc8af5b9 trigger C.I.
e6b9b76027 trigger C.I.
d6e24170b4 Nan is not supported with C++98
0ceaba1da9 Nan is not supported with C++98
0d52d5ddab Fix master build
557f5f2731 Fix test
92e945cc8a GLM_EXT_vector_integer: add integer mix tests
48516f31a7 Fix vec1 types redefinition
ec6e3d6cb5 Fix invalid namespaces
4681c5b347 Fix lib build dependent of test build
820557cf31 Fix GTC_matrix_transform test for C++98
ed1059731f add infinitePerspectiveRH_ZO
9cb19aa43f Added infinitePerspectiveLH_ZO and tests
926e5d4b70 Add tau constant ##1153
85f2e6b998 Add tau constant ##1153
08f2fd1099 Fix C++20
edc5e624df Fix build
f9a5a404dd Fix C.I. file
bab156f795 add status badge
46818dccca fix: reinterpret_cast to explicit conversions #1123
f1bfe6cc95 Fix C++20 build
c668158672 Add Clang on Linux
e000a4703e Remove AVX2 on macOS
53302ad486 Apply suggestions from code review
5ae05c9296 Fix GLM_DISABLE_AUTO_DETECTION error with Werror on Windows
971f22222e C.I. Fix timeout
e27fcc7bbd C.I.: Add more C++ language versions
3b21b05ac2 C.I.: Add AVX tests
66991e59aa Fix bitCount test version
c83236b044 cuda: Fix GTX_vec_swizzle
2171a5b818 Fix intersectRayTriangle from GLM_GTX_intersect #1158
1682a8c360 Fix doc typo
5c008438db doc: Updated readme
8a700ad040 More C.I. tests
641bb363a7 gni
c568980cf5 Revert broken vec4 SIMD
edecbf1c59 Revert "Add support for non aligned SIMD for vec4"
2d38c41161 Fixes for tests build
b90333c124 C.I. Add GitHub Actions

git-subtree-dir: 3rd-Party/glm
git-subtree-split: 33b4a621a697a305bc3a7610d290677b96beb181
Zuzu-Typ pushed a commit to Zuzu-Typ/glm that referenced this issue Oct 11, 2024
Zuzu-Typ added a commit to Zuzu-Typ/glm that referenced this issue Oct 11, 2024
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.

3 participants