Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Moon Rotation and Align Planets to Face the User #307

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

AdrienBousquieEPFL
Copy link
Contributor

Description

This PR addresses the issue where the Moon's rotation was causing its texture to appear inconsistent, and ensures that all celestial objects (Moon and planets) face the user directly for an improved visual experience.

Changes Implemented

Moon Rotation Fix

  • Disabled the Moon's rotation by setting rotationSpeed to 0.0f in Moon.kt.
  • Ensured the Moon remains stationary, maintaining the intended visual consistency.

Planet Orientation

  • Implemented a billboard effect to align all planets and the Moon to face the user's perspective.
  • Adjusted the Planet rendering logic in Planet.kt:
    • Added a billboardMatrix to align objects with the camera's look direction.

Code Modifications

  • Moon.kt

    • Set rotationSpeed to 0.0f.
  • Planet.kt

    • Added new logic for billboard transformations using the camera's view matrix.
    • Reorganized rotation and scaling transformations.
    • Enhanced rendering pipeline to use a combined billboardMatrix and rotation adjustments.

Updated `Moon` to include a `rotationSpeed` property with a default value of `0.0f`.
Added a configurable `rotationSpeed` property to `Planet`, making it adjustable per instance instead of constant.
Refactored the `rotationSpeed` logic in `Planet` to improve flexibility and clarity.
Added a `billboardMatrix` to ensure planets always face the camera, creating a billboard effect.
Copy link
Contributor

@Kenzoud Kenzoud left a comment

Choose a reason for hiding this comment

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

Good job on this pull request. The Moon doesn't move now, which makes more sense than before and the fact that the planets are facing us now, does look better. However, I have noticed some code duplication that could be avoided by using the code snippet that I have provided. I can go ahead and change this part of code if you want. This new code also improves the code coverage of this PR.

val rightZ = upX * lookY - upY * lookX

// Set billboard matrix
billboardMatrix[0] = rightX
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be replaced by:

// Set billboard matrix
    fun setMatrixRow(matrix: FloatArray, startIndex: Int, x: Float, y: Float, z: Float, w: Float) {
      matrix[startIndex] = x
      matrix[startIndex + 1] = y
      matrix[startIndex + 2] = z
      matrix[startIndex + 3] = w
    }

    // Set billboard matrix
    setMatrixRow(billboardMatrix, 0, rightX, rightY, rightZ, 0f)
    setMatrixRow(billboardMatrix, 4, upX, upY, upZ, 0f)
    setMatrixRow(billboardMatrix, 8, lookX, lookY, lookZ, 0f)
    setMatrixRow(billboardMatrix, 12, 0f, 0f, 0f, 1f)

to avoid code duplication.

@Kenzoud Kenzoud self-requested a review December 19, 2024 20:40
Copy link
Contributor

@Kenzoud Kenzoud left a comment

Choose a reason for hiding this comment

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

Given the time constraint of tomorrow's deadline, I went ahead and took the liberty of adding the commit with my code suggestion to your PR (Sonar cloud now passes with no code duplication and a sufficient code coverage :) ). Approved for merge.

@Kenzoud Kenzoud merged commit 14960c7 into main Dec 19, 2024
3 checks passed
@Kenzoud Kenzoud deleted the fix/sphere-not-facing-user branch December 19, 2024 20:47
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.

Orientation and Rotation of Celestial Objects
2 participants