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 unshaded debug draw mode for LightmapGI #88384

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

oxi-dev0
Copy link
Contributor

Fixes #88341

Copy link
Contributor

@jsjtxietian jsjtxietian left a comment

Choose a reason for hiding this comment

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

See https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html#modifying-a-pull-request , "we will prefer a single commit in a given PR (unless there's a good reason to keep the changes separate)" , don't forget to squash your pr 😄

@Calinou
Copy link
Member

Calinou commented Feb 16, 2024

I just realized objects with the Dynamic GI mode have the same issue in Forward+ and Mobile. This is in master, and it also occurs with this PR:

image

Object on the left uses Disabled GI mode, object on the right uses the Dynamic GI mode.

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, it works as expected in all rendering methods.

However, when using Forward+ or Mobile, dynamic objects are still shaded as seen in the comment above. Could you look into resolving this as well? Thanks in advance 🙂

@oxi-dev0
Copy link
Contributor Author

yes i will look into fixing dynamic objects, and squash the pr 😁

@oxi-dev0
Copy link
Contributor Author

Looking at that dynamic bug, it fixes itself if you bake the lightmap while the mesh is dynamic. It just seems like a general lightmapper issue

@Calinou
Copy link
Member

Calinou commented Feb 16, 2024

Looking at that dynamic bug, it fixes itself if you bake the lightmap while the mesh is dynamic. It just seems like a general lightmapper issue

This is due to #77167. The object with Dynamic GI mode is considered to use the Disabled GI mode until it's moved after baking lightmaps. That bug ends up hiding this particular issue, but once it's resolved, the object with Dynamic GI won't be unshaded anymore after baking lightmaps.

@oxi-dev0
Copy link
Contributor Author

Right ok, I'll take a look at it anyway

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.

This PR now seems to work correctly for dynamic objects with all rendering methods. I'd appreciate a second look just in case as I don't know if it's a fluke with my testing project.

@akien-mga akien-mga requested a review from a team February 24, 2024 21:17
@akien-mga
Copy link
Member

@oxi-dev0 Could you rebase to resolve the merge conflicts?

@oxi-dev0 oxi-dev0 requested review from a team as code owners April 17, 2024 09:19
@AThousandShips AThousandShips removed request for a team April 17, 2024 09:40
@oxi-dev0
Copy link
Contributor Author

Thanks for the help!

@akien-mga akien-mga merged commit 578fc2e into godotengine:master Apr 17, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@oxi-dev0 oxi-dev0 deleted the fix-unshaded-debug branch April 18, 2024 08:14
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.

Unshaded debug draw mode still draws lightmaps on meshes
7 participants