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

[Merged by Bors] - Fixing confusing near and far fields in Camera #4457

Closed
wants to merge 11 commits into from
Closed

[Merged by Bors] - Fixing confusing near and far fields in Camera #4457

wants to merge 11 commits into from

Conversation

AronDerenyi
Copy link
Contributor

Objective

Solution

  • Removed the near and far fields from the camera and the views.

Changelog

  • Removed the near and far fields from the camera and the views.
  • Removed the ClusterFarZMode::CameraFarPlane far z mode.

Migration Guide

  • Cameras no longer accept near and far values during initialization
  • ClusterFarZMode::Constant should be used with the far value instead of ClusterFarZMode::CameraFarPlane

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 11, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Docs An addition or correction to our documentation C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Apr 11, 2022
@mockersf mockersf mentioned this pull request Apr 25, 2022
Open
superdump
superdump requested changes May 5, 2022
View reviewed changes
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Please test the many_lights example with a 3D orthographic camera and log the appropriate near value as extracted from the orthographic projection to check it is correct. :)

crates/bevy_pbr/src/light.rs Outdated Show resolved Hide resolved
@AronDerenyi @superdump
Update crates/bevy_pbr/src/light.rs
5fe54b4 
Fixed the near value calculation and added comments

Co-authored-by: Robert Swain <[email protected]>
@cart
Copy link
Member

cart commented May 10, 2022

I just independently came to the same conclusion with my recent camera work. I think this is a good call.

@AronDerenyi AronDerenyi requested review from superdump and cart May 13, 2022 20:56
@cart cart mentioned this pull request May 14, 2022
Closed
superdump
superdump approved these changes May 14, 2022
View reviewed changes
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

It's a shame to have to remove the CameraFarPlane option, but it's easy enough to configure a constant far plane instead if you need it.

@superdump
Copy link
Contributor

@robtfm @cart any objections to merging this?

@robtfm
Copy link
Contributor

robtfm commented May 14, 2022

no objection from me. ClusterFarZMode::CameraFarPlane was really only there because it was the way clusters used to work and i didn't want to remove support for it without a good reason. this is a good reason.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 16, 2022
@superdump
Copy link
Contributor

bors r+

bors bot pushed a commit that referenced this pull request May 16, 2022
@AronDerenyi
Fixing confusing near and far fields in Camera (#4457)
2e8dfc0 
# Objective

- Fixes #4456 

## Solution

- Removed the `near` and `far` fields from the camera and the views.

---

## Changelog

- Removed the `near` and `far` fields from the camera and the views.
- Removed the `ClusterFarZMode::CameraFarPlane` far z mode.

## Migration Guide

- Cameras no longer accept near and far values during initialization
- `ClusterFarZMode::Constant` should be used with the far value instead of `ClusterFarZMode::CameraFarPlane`
@bors
Copy link
Contributor

bors bot commented May 16, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Fixing confusing near and far fields in Camera [Merged by Bors] - Fixing confusing near and far fields in Camera May 16, 2022
@bors bors bot closed this May 16, 2022
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 17, 2022
@AronDerenyi @robtfm
Fixing confusing near and far fields in Camera (bevyengine#4457)
e59d274 
# Objective

- Fixes bevyengine#4456 

## Solution

- Removed the `near` and `far` fields from the camera and the views.

---

## Changelog

- Removed the `near` and `far` fields from the camera and the views.
- Removed the `ClusterFarZMode::CameraFarPlane` far z mode.

## Migration Guide

- Cameras no longer accept near and far values during initialization
- `ClusterFarZMode::Constant` should be used with the far value instead of `ClusterFarZMode::CameraFarPlane`
@DotRed108
Copy link

How do you migrate this if you were previously using .near field

@superdump
Copy link
Contributor

How do you migrate this if you were previously using .near field

What were you using it for?

exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
@AronDerenyi @exjam
Fixing confusing near and far fields in Camera (bevyengine#4457)
0497911 
# Objective

- Fixes bevyengine#4456 

## Solution

- Removed the `near` and `far` fields from the camera and the views.

---

## Changelog

- Removed the `near` and `far` fields from the camera and the views.
- Removed the `ClusterFarZMode::CameraFarPlane` far z mode.

## Migration Guide

- Cameras no longer accept near and far values during initialization
- `ClusterFarZMode::Constant` should be used with the far value instead of `ClusterFarZMode::CameraFarPlane`
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@AronDerenyi @ItsDoot
Fixing confusing near and far fields in Camera (bevyengine#4457)
3cc0ac4 
# Objective

- Fixes bevyengine#4456 

## Solution

- Removed the `near` and `far` fields from the camera and the views.

---

## Changelog

- Removed the `near` and `far` fields from the camera and the views.
- Removed the `ClusterFarZMode::CameraFarPlane` far z mode.

## Migration Guide

- Cameras no longer accept near and far values during initialization
- `ClusterFarZMode::Constant` should be used with the far value instead of `ClusterFarZMode::CameraFarPlane`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robtfm robtfm robtfm approved these changes

@IceSentry IceSentry IceSentry approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@superdump superdump superdump approved these changes

@cart cart Awaiting requested review from cart

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Docs An addition or correction to our documentation C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Confusing near and far fields in Camera
7 participants
@AronDerenyi @cart @superdump @robtfm @DotRed108 @alice-i-cecile @IceSentry