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

Remove CGLM_USE_DEFAULT_EPSILON and use system's float epsilon as default #170

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

recp
Copy link
Owner

@recp recp commented Nov 21, 2020

Previous:

#ifndef CGLM_USE_DEFAULT_EPSILON
#  ifndef GLM_FLT_EPSILON
#    define GLM_FLT_EPSILON 1e-6
#  endif
#else
#  define GLM_FLT_EPSILON FLT_EPSILON
#endif

New:

#ifndef GLM_FLT_EPSILON
#  ifndef FLT_EPSILON
#    define GLM_FLT_EPSILON 1e-6f
#  else
#    define GLM_FLT_EPSILON FLT_EPSILON
#  endif
#endif
  • To override float epsilon we just need to define GLM_FLT_EPSILON
  • CGLM_USE_DEFAULT_EPSILON was redundant, also it forces to override system default epsilon which may not be good idea, because not all systems may support smaller epsilon values

The epsilon value is used in tests but we must use bigger number is some places, because after matrix multiplication, projection... floating errors may occurs which may bigger than epsilon. Probably it will depend on the matrix

* to override float epsilon we just need to define GLM_FLT_EPSILON
* CGLM_USE_DEFAULT_EPSILON was redundant, also it forces to override system default epsilon which may not be good idea, because not all systems may support smaller epsilon values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant