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

Vec2 clamp_length alters vector when length is within specified range #515

Closed semihbkgr closed 2 months ago

semihbkgr commented 2 months ago

The clamp_length method in the Vec2 incorrectly alters the vector components when the vector's length is within the specified min and max range. This behavior deviates from the expected functionality where the vector should remain unchanged if its length falls within the specified bounds.

let vec = Vec2::new(3., 4.);
let new_vec = vec.clamp_length(-10., 10.);
println!("{}", new_vec);
//the new_vec is [-6, -8]

While it is possible that this behavior is intentional in the current implementation, it seems counterintuitive for the method to alter the vector when its length is already within the specified range. Clarification on whether this is the expected behavior would be appreciated.

bitshifter commented 2 months ago

Thanks for the report, that definitely sounds wrong. I think I can see what the problem is in the implementation, I'll look at fixing that soon.

semihbkgr commented 2 months ago

I've sent a draft PR for the f32 Vec2 method. If it looks good, I can update the other methods afterward.

bitshifter commented 2 months ago

Looking into this some more, I think part of the issue is negative values probably shouldn't be allowed with this function. Basically vector length cannot be less than zero, the clamp function makes sure then length is no less than the given min, but the doesn't really make sense if min (or max for that matter are negative. What should probably happen here is that the docs should say that min and max must be positive and we could add some glam_assets to check the inputs.

semihbkgr commented 2 months ago

I agree. It's much better

Nul-led commented 1 month ago

@bitshifter length shouldn't be zero either, else we'd be dividing by zero which results in NaN values, should prob add check for that in clamp_length & clamp_length_min