Skip to content

added length, length_sq, normalize for vec4.#14

Merged
Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
mcdubhghlas:master
Apr 7, 2026
Merged

added length, length_sq, normalize for vec4.#14
Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
mcdubhghlas:master

Conversation

@mcdubhghlas
Copy link
Copy Markdown
Member

@mcdubhghlas mcdubhghlas commented Apr 7, 2026

AR needed this, still needs more for vec4.

Summary by CodeRabbit

  • New Features
    • Added four utility functions for 4D vector mathematics: squared magnitude calculation, magnitude calculation, safe normalization with zero-length validation, and optimized fast normalization for performance-critical code paths. All functions include compiler optimization hints for efficient, low-overhead vector operations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

Four new exported inline utility functions added to draco::math::Vector4 for computing squared magnitude, magnitude, normalization (with zero-check), and fast normalization (without zero-check). All functions are marked [[nodiscard]], FORCEINLINE, and noexcept.

Changes

Cohort / File(s) Summary
Vector4 Utility Functions
engine/native/core/math/vector4.cppm
Added four inline utility functions: length_sq(), length(), normalize(), and normalize_fast() for Vector4 magnitude and normalization operations with [[nodiscard]], FORCEINLINE, and noexcept annotations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Four functions hop into the code,
Vector math takes lighter load!
Length, normalize, fast and clean,
Simplest utilities you've ever seen! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: it lists the three primary functions added (length, length_sq, normalize) and specifies they are for vec4.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
engine/native/core/math/vector4.cppm (1)

161-166: Consider epsilon threshold for near-zero vectors.

The exact len > 0.0f check prevents division by zero, but vectors with extremely small magnitudes (e.g., 1e-40) will still be normalized, potentially producing numerically unstable results due to floating-point precision loss.

If robustness against near-zero vectors is desired, consider using an epsilon:

constexpr float EPSILON = 1e-8f;
return (len > EPSILON) ? v / len : Vector4{0,0,0,0};

That said, the current implementation is valid and the behavior is well-documented by the comment. This can be addressed later if it causes issues in practice.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@engine/native/core/math/vector4.cppm` around lines 161 - 166, The normalize
function currently uses a strict len > 0.0f check which can produce unstable
results for very small magnitudes; update Vector4 normalize(const Vector4& v)
noexcept to compare length(v) against a small epsilon (e.g. constexpr float
EPSILON = 1e-8f) instead of 0.0f and return v / len only when len > EPSILON,
otherwise return Vector4{0,0,0,0}; use the existing length() helper and keep the
function signature and noexcept intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@engine/native/core/math/vector4.cppm`:
- Around line 161-166: The normalize function currently uses a strict len > 0.0f
check which can produce unstable results for very small magnitudes; update
Vector4 normalize(const Vector4& v) noexcept to compare length(v) against a
small epsilon (e.g. constexpr float EPSILON = 1e-8f) instead of 0.0f and return
v / len only when len > EPSILON, otherwise return Vector4{0,0,0,0}; use the
existing length() helper and keep the function signature and noexcept intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 405e18d2-d437-47c7-93af-a4cb11faa0f1

📥 Commits

Reviewing files that changed from the base of the PR and between 54bb414 and 1833dae.

📒 Files selected for processing (1)
  • engine/native/core/math/vector4.cppm

Copy link
Copy Markdown
Contributor

@AR-DEV-1 AR-DEV-1 left a comment

Choose a reason for hiding this comment

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

LGTM

@Arctis-Fireblight Arctis-Fireblight enabled auto-merge (rebase) April 7, 2026 14:17
@Arctis-Fireblight Arctis-Fireblight merged commit 17595c0 into Redot-Engine:master Apr 7, 2026
1 check passed
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.

4 participants