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

Tiny fix for lightmapper DDA #86583

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

reduz
Copy link
Member

@reduz reduz commented Dec 28, 2023

  • Ensures only one axis advances at a time
  • This fixes extremely corner cases where the DDA may skip over geometry
  • As a bonus, clarify to the user that splitting into more meshes may improve the texture size limit.

@reduz reduz requested a review from a team as a code owner December 28, 2023 16:53
@jcostello
Copy link
Contributor

Error while building

editor/plugins/lightmap_gi_editor_plugin.cpp:116:6: error: 'LightmapGIEditormPlugin' has not been declared
  116 | void LightmapGIEditormPlugin::_bake() {
      |      ^~~~~~~~~~~~~~~~~~~~~~~
editor/plugins/lightmap_gi_editor_plugin.cpp: In function 'void _bake()':
editor/plugins/lightmap_gi_editor_plugin.cpp:117:9: error: '_bake_select_file' was not declared in this scope
  117 |         _bake_select_file("");
      |         ^~~~~~~~~~~~~~~~~

@jcostello
Copy link
Contributor

Looks they are working much better now. I tried on sponza and in the TPS demo

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.

Makes sense. Using lessThanEqual could return multiple channels where the channels are equal. We can't move to lessThan as we could end up with zero channels and then we are stuck.

You just need to remove the old line before this is ready to merge.

modules/lightmapper_rd/lm_compute.glsl Outdated Show resolved Hide resolved
- Ensures only one axis advances at a time
- This fixes extremely corner cases where the DDA may skip over geometry
@reduz
Copy link
Member Author

reduz commented Jan 10, 2024

This should now be ready

@akien-mga akien-mga merged commit 9e967eb into godotengine:master Jan 11, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

5 participants