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

Revisit ffmin/ffmax? #444

Closed
manas96 opened this issue Apr 5, 2020 · 1 comment
Closed

Revisit ffmin/ffmax? #444

manas96 opened this issue Apr 5, 2020 · 1 comment
Assignees
Milestone

Comments

@manas96
Copy link
Contributor

manas96 commented Apr 5, 2020

In Listing 22, I assume the ff in functions ffmin and ffmax stands for floatfloatmin and floatfloatmax. However, these functions use doubles.

@hollasch hollasch self-assigned this Apr 5, 2020
@hollasch hollasch added this to the v3.1.0 milestone Apr 5, 2020
@hollasch
Copy link
Collaborator

hollasch commented Apr 5, 2020

In the C++ standard library, the f prefix generally stands for floating point, and double is the standard default. See https://en.cppreference.com/w/cpp/numeric/math/fmin.

These functions were defined before we based our code on C++11 (for std::shared_ptr), among other things. We're generally trying to limit the amount of C++11 we use for simplicity, but we might consider just adopting std::fmin and std::fmax.

However, fmin/fmax support additional float-specific behavior, such as handling of NaN values (only returning the non-NaN value), and some implementations return ±0 properly (std::fmin(+0,-0) may guarantee return of -0, for example).

In our simple implementation, we really just emulate std::min and std::max. It might be better just to rename these functions min and max, relying on specific overloads. image_texture for example could use max(int,int).

Generally, not sure if it's worth the effort to refactor these, but thought I'd write down the considerations here.

@hollasch hollasch changed the title Book one - Functions named ffmin & ffmax but they use doubles Revisit ffmin/ffmax? Apr 6, 2020
@hollasch hollasch modified the milestones: v3.1.0, Future Apr 6, 2020
@hollasch hollasch modified the milestones: Future, v3.1.0 May 2, 2020
@hollasch hollasch closed this as completed May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants