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.

Sign up for GitHub

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

Jump to bottom

Fix ReflectionProbe AABB #99802

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Fix ReflectionProbe AABB #99802

merged 1 commit into from
Nov 29, 2024

Conversation

lander-vr
Copy link
Contributor

This PR resolves some issues in the AABB calculation of the reflection probe:

  • The AABB of ReflectionProbe is now calculated from corner to corner
  • Removed the origin_offset from the AABB calculation. Origin_offset should not have any bearing on the bounding box.

Additionally I've also removed the green wire-frame, since the bounding box is displayed on top of it anyways.

4.3 This PR
image image
image image
24_11_28_17_27__Godot_v4 3-stable_win64_KaL9XxGQDx 24_11_28_17_26__godot windows editor x86_64_cVjwQTIUl8

@lander-vr lander-vr requested review from a team as code owners November 28, 2024 16:36
@Calinou Calinou added bug topic:rendering topic:editor topic:3d cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Nov 28, 2024
@Calinou Calinou added this to the 4.4 milestone Nov 28, 2024
Calinou
Calinou approved these changes Nov 28, 2024
View reviewed changes
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

clayjohn
clayjohn approved these changes Nov 28, 2024
View reviewed changes
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

As far as I can tell this function is only ever used for the editor drawing of the AABB. The AABB used by the RenderingServer was already generated using the exact same code you have here.

@akien-mga akien-mga merged commit 8231179 into godotengine:master Nov 29, 2024
20 checks passed
@lander-vr lander-vr deleted the reflection-probe-wireframe branch November 29, 2024 22:00
@akien-mga
Copy link
Member

Thanks!

@Spartan322 Spartan322 mentioned this pull request Jan 22, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Calinou Calinou Calinou approved these changes

@clayjohn clayjohn clayjohn approved these changes

Assignees
No one assigned
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:editor topic:rendering topic:3d
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
4 participants
@lander-vr @akien-mga @Calinou @clayjohn