Skip to content

Conversation

@Skyress-s
Copy link

Hi! This is my first PR so sorry for any inconveniences!

For certain argument values, returns incorrect quaternion.

Looked at linalg.quaternion_from_matrix3 (which worked) and discovered a minus sign should have been a pluss sign in linalg.quaternion_from_forward_and_up.

Ran the program below with the current odin compiler and the compiler compiled locally with these changes. Issue was resolved on the locally compiled one!

Here is the test code i used to verify the changes:

package main

import "core:fmt"
import "core:math"
import "core:math/linalg"
import "core:math/rand"

Vec :: linalg.Vector3f32

FORWARD :: Vec{0, 0, -1}
UP :: Vec{0, 1, 0}

rand_f32_normalized :: proc() -> f32 {return rand.float32_range(-1, 1)}
rand_vec_normalized :: proc() -> Vec {return linalg.normalize(
		Vec{rand_f32_normalized(), rand_f32_normalized(), rand_f32_normalized()},
	)}

main :: proc() {
	// Ensure the runs of the program are the same.
	context.random_generator = rand.default_random_generator(&rand.Default_Random_State{state = 1})

	for i in 0 ..< 1000 {
		// New random forward with up that is at 90 degrees. 
		forward := rand_vec_normalized()
		up := linalg.normalize(linalg.cross(linalg.cross(rand_vec_normalized(), forward), forward))

		q := linalg.quaternion_from_forward_and_up(forward, up)
		q = linalg.normalize(q)

		// Recalculate forward and up with the quat
		forward_recalc := linalg.normalize(linalg.mul(q, FORWARD))
		up_recalc := linalg.mul(q, UP)

 		// check if the vectors are similar enough
		if (linalg.length(forward - forward_recalc) > 0.00001) || linalg.length(up - up_recalc) > 0.00001 { 
			fmt.printfln("Error at iteration = {}", i)
			fmt.printfln("{:.3f} | {:.3f}", forward, forward_recalc)
			fmt.printfln("{:.3f} | {:.3f}", up, up_recalc)
		}
	}
}

…urn incorrect values in certain scenarioes
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