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

Fix cluster artifacts and negative light #82546

Merged

Conversation

viksl
Copy link
Contributor

@viksl viksl commented Sep 29, 2023

Fixes #78500

Instructions:
Definitely should be merged on top of this PR: #80992

Details:
Just added the same shader adjustment for the cone angle as in this PR: #71832 it might have been just forgotten?

EDIT:
There's one more place which also uses the cone angle, but this time in gles3 shader, which doesn't use the highp for the cone angle in case it matters (?), the original author did not rework that one, it might not be an issue but I'm keeping this note here in case in future there's a similar clustered artifact issue with spotlight in gles3 in some other areas, it might not be need to make a change there at all but let's try not to foget it.

@viksl viksl requested a review from a team as a code owner September 29, 2023 19:46
@Chaosus Chaosus added this to the 4.2 milestone Sep 29, 2023
@viksl viksl changed the title Fixes clsuter artifacts and negative light, same approach as here: ht… Fixes cluster artifacts and negative light Sep 29, 2023
@akien-mga akien-mga changed the title Fixes cluster artifacts and negative light Fix cluster artifacts and negative light Sep 30, 2023
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.

Tested locally on Linux + NVIDIA (rebased on top of master 0ca8542 + #80992), it works as expected for the MRP linked in #78500.

Before

before.mp4

After

after.mp4

@akien-mga
Copy link
Member

Could you amend the commit message to fix the typo and make it shorter, like the PR title?

A commit message should have a short, imperative sentence as title, and additional context can be given in the body of the message.

@viksl
Copy link
Contributor Author

viksl commented Oct 1, 2023

Oki, I think I did it wrong so I'll try to squash it into one commit today.

EDIT: Is it ok now?

@viksl viksl force-pushed the volumetric-fog-spot-light-artifacts branch from 2284c3a to 8a2d345 Compare October 1, 2023 07:22
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.

Great catch!

I can't reproduce the problem locally, but I trust Calinou's testing and the rationale makes sense.

@clayjohn clayjohn added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 5, 2023
@akien-mga akien-mga merged commit 950139e into godotengine:master Oct 5, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@viksl viksl deleted the volumetric-fog-spot-light-artifacts branch October 5, 2023 21:38
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpotLight3D turns negative, and creates black boxes on small angles with volumetric fog
6 participants