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 regression with shadows when light range is 0 or close #100862

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Fix regression with shadows when light range is 0 or close #100862

merged 1 commit into from
Dec 30, 2024

Conversation

Flarkk
Copy link
Contributor

@Flarkk Flarkk commented Dec 27, 2024 •
edited
Loading

This PR fixes #100850.
A regression were introduced in #100319 when Omni and Spot lights have a range of 0 or close to 0.
Far distance could become smaller than near distance (0.005) which makes the shadow projection matrix non inversible.

@Flarkk Flarkk requested a review from a team as a code owner December 27, 2024 22:16
@Flarkk Flarkk marked this pull request as draft December 27, 2024 22:16
@Flarkk Flarkk mentioned this pull request Dec 27, 2024
Closed
@matheusmdx
Copy link
Contributor

I cherry-picked this pr but seems the same error persists.

@Flarkk Flarkk changed the title Fix regression with shadows rendering Fix regression with 0-range lights shadows Dec 28, 2024
@Flarkk Flarkk changed the title Fix regression with 0-range lights shadows Fix regression with shadows when light range is 0 or close Dec 28, 2024
@Flarkk Flarkk marked this pull request as ready for review December 28, 2024 10:44
@Flarkk
Copy link
Contributor Author

Flarkk commented Dec 28, 2024 •
edited
Loading

Should be okay now.
The fix ensures the shadow near distance never goes above far.
This guarantees the projection matrix stays inversible.

@Chaosus Chaosus added this to the 4.4 milestone Dec 28, 2024
stuartcarnie
stuartcarnie reviewed Dec 28, 2024
View reviewed changes
Copy link
Contributor

@stuartcarnie stuartcarnie left a comment

Choose a reason for hiding this comment

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

LGTM – tested with TPS-Demo and can no longer reproduce the log spam 👏🏻

@Repiteo Repiteo merged commit 888883d into godotengine:master Dec 30, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 30, 2024

Thanks!

@Flarkk Flarkk deleted the fix_100850 branch January 3, 2025 09:37
@Flarkk Flarkk mentioned this pull request Feb 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartcarnie stuartcarnie

@Repiteo Repiteo

Assignees
No one assigned
Labels
bug regression topic:rendering
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

Projection::invert fails with Condition "0.0 == r3[3]" is true
5 participants
@Flarkk @matheusmdx @Repiteo @stuartcarnie @Chaosus