bitshifter / glam-rs

A simple and fast linear algebra library for games and graphics
Apache License 2.0
1.46k stars 145 forks source link

Extend `.normalize()` etc. to work for more values #512

Closed lynn-lumen closed 1 month ago

lynn-lumen commented 2 months ago

The .normalize() etc. functions currently work for finite non-zero vectors only.

However Vectors of the form Vec3::new(±INFINITY, a, b) where a and b are finite also have an obvious normalised form (e.g. Vec3::new(±1., 0., 0.) for the example above). This also applies to other types of TVecN.

An argument can be made against allowing normalisation of Vecs with two or more infinite components, but I think that there is only solution for Vecs with exactly one infinite component.

This came up recently as an issue in bevy when scaling normals of meshes. When scaling normals, one divides the unscaled normal by the scale

let scaled_normal = (normal / scale).normalize();

When scale has one components that is zero, the scale is valid, but the result of the snippet above cannot be normalized. This leads to having to implement normalising for Vecs with one infinite component manually in other projects.

I am not sure whether this should be done for all current .normalize() etc. functions or whether this should be its own thing but it would be nice to have.

Edit: The docs claim that the normalize & co. functions produce valid results for all values that are not very close or equal to the zero Vec. Even if this is not added, the docs need to be updated to include infinite Vecs as invalid.

bitshifter commented 1 month ago

I don't think it's really possible to make normalize handle infinite values, at least not without making it a lot more expensive.

In the example above with (normal / scale), if that results in a component 0 / 0 then that is a NaN which permeates through subsequent operations, resulting in vector of NaNs. If if results in a component 1 / 0 then that is a inf which will give a vector legnth of inf which results in a vector containing a NaN where a non-zero value is divided by inf.

I can update the docs, the other normalize_* methods already mention inputs must be finite.

bitshifter commented 1 month ago

Docs improved in #527