-
Notifications
You must be signed in to change notification settings - Fork 427
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
--[BE Week] Update projection matrix near/far plane calcs #2276
Conversation
also use new magnum implementations of near and far calcs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the unnecessary auto
s (I'm old and they make reading the code harder for me, not easier), this looks fine to me! :)
src/esp/gfx/RenderCamera.cpp
Outdated
auto farDistance = | ||
projectionMatrix()[3][2] / (1.0f + projectionMatrix()[2][2]); | ||
// If projMat[3][3] == 0 then perpsective, otherwise ortho | ||
auto farDistance = (projMat[3][3] == 0 ? projMat.perspectiveProjectionFar() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Annoying mode enabled)
auto farDistance = (projMat[3][3] == 0 ? projMat.perspectiveProjectionFar() | |
const float farDistance = (projMat[3][3] == 0 ? projMat.perspectiveProjectionFar() |
src/esp/gfx/RenderCamera.cpp
Outdated
@@ -207,10 +207,12 @@ esp::geo::Ray RenderCamera::unproject(const Mn::Vector2i& viewportPosition, | |||
2 * Magnum::Vector2{viewPos} / Magnum::Vector2{viewport()} - | |||
Magnum::Vector2{1.0f}, | |||
1.0}; | |||
const auto projMat = projectionMatrix(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto projMat = projectionMatrix(); | |
const Mn::Matrix4 projMat = projectionMatrix(); |
ac7ec21
to
3ff4b3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
src/esp/gfx/RenderCamera.cpp
Outdated
|
||
// compute the far plane distance | ||
auto farDistance = | ||
projectionMatrix()[3][2] / (1.0f + projectionMatrix()[2][2]); | ||
// If projMat[3][3] == 0 then perpsective, otherwise ortho |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: perpsective
Motivation and Context
This PR fixes or updates a few things related to the near/far plane calculation for projection matrices :
RenderCamera::unproject
function now not only calls the Magnum function, but also picks the correct calculation based on the nature of the Projection matrix, using the element at mat[3][3] to determine whether the matrix is perspective or orthographic.How Has This Been Tested
Types of changes
Checklist