Implementation of additional Projection methods
#529
Conversation
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-529 |
6fc203f to
9743e05
Compare
godot-core/src/builtin/projection.rs
Outdated
| pub fn create_fit_aabb(aabb: Aabb) -> Self { | ||
| let min = aabb.position; | ||
| let max = aabb.position + aabb.size; | ||
|
|
||
| let scale = Vector3::splat(2.0) / (max - min); | ||
| let translate = -(max + min) / (max - min); |
There was a problem hiding this comment.
(max - min) is just aabb.size -- no need to recalculate it, let alone twice.
Generally not sure we gain much from min/max variables. The only remaining expression is then -(max + min), which is the same as (-2.0 * aabb.position - aabb.size).
Maybe a small comment is enough.
| pub fn create_fit_aabb(aabb: Aabb) -> Self { | |
| let min = aabb.position; | |
| let max = aabb.position + aabb.size; | |
| let scale = Vector3::splat(2.0) / (max - min); | |
| let translate = -(max + min) / (max - min); | |
| pub fn create_fit_aabb(aabb: Aabb) -> Self { | |
| let translate_unscaled = -2.0 * aabb.position - aabb.size; // -(start+end) | |
| let scale = Vector3::splat(2.0) / aabb.size; | |
| let translate = translate_unscaled / aabb.size; |
There was a problem hiding this comment.
You're right. Makes more sense instead of min/max. 👍
godot-core/src/builtin/projection.rs
Outdated
| /// has the given horizontal field of view (in degrees) and aspect ratio. | ||
| /// | ||
| /// _Godot equivalent: Projection.get_fovy()_ | ||
| pub fn fovy_of(fov_x: real, aspect: real) -> real { | ||
| real::from_f64(InnerProjection::get_fovy(fov_x.as_f64(), aspect.as_f64())) | ||
| pub fn fovy(fov_x: real, aspect: real) -> real { | ||
| let half_angle_fov_x = f64::to_radians(fov_x.as_f64() * 0.5); | ||
| let vertical_transform = f64::atan(aspect.as_f64() * f64::tan(half_angle_fov_x)); | ||
| let full_angle_fov_y = f64::to_degrees(vertical_transform * 2.0); | ||
|
|
||
| real::from_f64(full_angle_fov_y) |
There was a problem hiding this comment.
We named the method fovy_of in the initial implementation, see #124 (comment).
But I think create_fovy might actually be a better name, in line with the other constructors. Godot's get_fovy for a static method is terrible naming.
Can you name it create_fovy and add a #[doc(alias)] with the Godot name?
There was a problem hiding this comment.
Added #[doc(alias = "get_fovy")], and changed the function name to create_fovy :)
This commit implements `create_fit_aabb()`, `create_light_atlas_rect()` and implements a native `fovy()` as a replacement for `fovy_of` wrapper that called godot directly.
9743e05 to
1b45761
Compare
This commit implements
create_fit_aabb(),create_light_atlas_rect()and re-implements a nativefovy()as a replacement forfovy_ofwrapper that called godot directly.This concerns the tracking issue #310, as
Projectionis tagged there.I've looked at the math behind the related methods straight from the source here: https://github.com/godotengine/godot/blob/4.2/core/math/projection.cpp, re-implemented them, and added tests comparing with the direct calls to Godot.
I'm unsure if we are allowed to borrow implementations from Godot itself, if not, feel free to close this PR. 😅