Skip to content

Added the basic Rotors Template#2

Open
GildedSeraphim wants to merge 1 commit intoHaraldWik:masterfrom
GildedSeraphim:master
Open

Added the basic Rotors Template#2
GildedSeraphim wants to merge 1 commit intoHaraldWik:masterfrom
GildedSeraphim:master

Conversation

@GildedSeraphim
Copy link
Copy Markdown

Added rotors, review needed and it might not work idk we'll see where this goes :)

Copy link
Copy Markdown
Owner

@HaraldWik HaraldWik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly just some formatting / styling fixes. Otherwise looks great!

result.yz = -r.yz;
result.zx = -r.zx;
return result;
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do result = .{ } and use aggregate struct initialization.

result[0] = S_x * r.scalar + S_y * r.xy + S_xyz * r.yz - S_z * r.zx;
result[1] = S_y * r.scalar - S_x * r.xy + S_z * r.yz + S_xyz * r.zx;
result[2] = S_z * r.scalar + S_xyz * r.xy - S_y * r.yz + S_x * r.zx;
return result;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, you can just do

return .{
            result[0] = S_x * r.scalar + S_y * r.xy + S_xyz * r.yz - S_z * r.zx;
            result[1] = S_y * r.scalar - S_x * r.xy + S_z * r.yz + S_xyz * r.zx;
            result[2] = S_z * r.scalar + S_xyz * r.xy - S_y * r.yz + S_x * r.zx;
}

const new_z: @Vector(3, T) = transform(r, @Vector(3, f32){ 0.0, 0.0, 1.0 });

const result: Mat4x4(T) = .new(.{ new_x[0], new_x[1], new_x[2], 0.0, new_y[0], new_y[1], new_y[2], 0.0, new_z[0], new_z[1], new_z[2], 0.0, 0.0, 0.0, 0.0, 0.0 });
return result;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can instantly just do

return .new( [...] )


if (dot > 0.99995) {
return nlerp(from, to, t);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do

if (dot > 0.99995) return nlerp(from, to, t);

const to_factor: f32 = std.math.sin((t * theta) / std.math.sin(theta));

var result: @This() = undefined;
result.scalar = from_factor * from.scalar + to_factor * to.scalar;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here just do

return .{
            .scalar = from_factor * from.scalar + to_factor * to.scalar;
            .xy = from_factor * from.xy + to_factor * to.xy;
            .yz = from_factor * from.yz + to_factor * to.yz;
            .zx = from_factor * from.zx + to_factor * to.zx;
};

return result;
}

pub fn from_vec_to_rotor(from_dir: @Vector(3, T), to_dir: @Vector(3, T)) @This() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

functions are typicaly camelCase, not snake_case


fn lerp(a: f32, b: f32, t: f32) f32 {
return a + t * (b - a);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there is std.math.lerp

}

pub fn transform(r: @This(), v: @Vector(3, T)) @Vector(3, T) {
const S_x: f32 = r.scalar * v[0] + r.xy * v[1] - r.zx * v[2];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always have variables and constants as snake_case with no upper case letters

const std = @import("std");
const vec = @import("vector.zig");
const Mat4x4 = @import("matrix.zig").@"4x4";
const Quaternion = @import("quaternion.zig").Hamiltonian;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would change it to be Quat

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.

2 participants