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

Switch to infinite_reverse_rh perspective #2733

Closed
wants to merge 1 commit into from
Closed

Switch to infinite_reverse_rh perspective #2733

wants to merge 1 commit into from

Conversation

thechubbypanda
Copy link

@thechubbypanda thechubbypanda commented Aug 26, 2021

Objective

As with #2543, switch to infinite_reverse_rh perspective for now, before the rendering rework is merged.

Will only affect those working off main and will (I assume) be overwritten by the rework.

Solution

  • Changed projection matrix for PerspectiveProjection and removed far plane

Alternate Solutions

  • I thought about adding a 3rd projection InverseInfiniteProjection but we'd need to add another camera system. One system would (almost) always be redundant.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Aug 26, 2021
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Code-Quality A section of code that is hard to understand or change and removed S-Needs-Triage This issue needs to be labelled C-Feature A new feature, making something new possible labels Aug 26, 2021
@cart
Copy link
Member

cart commented Aug 26, 2021

Switching to infinite_reverse_rh involves changes in a number of other places to ensure depth is handled correctly (including render pipeline definitions). This is missing those changes. Additionally, it is a breaking change. I'd rather not put "main branch" users through two sets of breaking changes: (1) adjust current code to account for infinite reverse rh (2) Rewrite everything when pipelined-rendering lands. Given that pipelined-rendering is going to be merged in the very near future, I think we should just close this out and wait.

@thechubbypanda
Copy link
Author

Ah I was unaware of the supplementary required changes.

In that case, I'll close :)

@cart
Copy link
Member

cart commented Aug 26, 2021

Cool cool. Thanks!

@thechubbypanda thechubbypanda deleted the inverse_infinite_projection branch August 26, 2021 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants