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

Implement unit tests for Camera3D #67170

Closed
wants to merge 1 commit into from

Conversation

TechnoPorg
Copy link
Contributor

This PR adds unit tests for Camera3D. Methods tested are:

  • is_position_behind
  • is_position_in_frustum
  • project_position
  • unproject_position

The projection test is currently failing with Frustum projection mode. I don't really know how that mode works, to be honest, so any input would be appreciated.

A placeholder size variable is also added here to DisplayServerHeadless in order to make it possible to project points in headless mode. I'm not sure if this will have any unintended consequences, so I would appreciate feedback from a DisplayServer expert. If not, it could be helpful for godotengine/godot-proposals#1760.

I noticed a fair bit of code duplication between project_position, project_ray_origin, and project_local_ray_normal while working on this. I think they can probably be refactored to all forward calls to a single function, but I thought that was better left for another PR.

Part of #43440.

@fire
Copy link
Member

fire commented Oct 10, 2022

@JFonS Are you able to give some insight on the frustum node and how it relates?

@TechnoPorg
Copy link
Contributor Author

I've rebased this PR to use doctest::Approx, following #68275.

@TechnoPorg
Copy link
Contributor Author

@JFonS Sorry to bump, but would you mind taking a look at this when you have time?

This commit adds unit tests for Camera3D. Methods tested are:
- is_position_behind
- is_position_in_frustum
- project_position
- unproject_position

A placeholder size variable is also added here to DisplayServerHeadless in order to make it possible to project points in headless mode.
@TechnoPorg
Copy link
Contributor Author

Rebased on current master.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.x Jun 14, 2023
@Calinou
Copy link
Member

Calinou commented Apr 10, 2024

Superseded by #84826 (which includes tests for all methods included in this PR). Thanks for the contribution nonetheless!

@Calinou Calinou closed this Apr 10, 2024
@Calinou Calinou removed this from the 4.x milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants