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

Lightmapper: Prevent infinite loop while blitting lightmaps into an atlas #94237

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Lightmapper: Prevent infinite loop while blitting lightmaps into an atlas #94237

merged 2 commits into from
Jul 18, 2024

Conversation

BlueCube3310
Copy link
Contributor

@BlueCube3310 BlueCube3310 commented Jul 11, 2024 •
edited
Loading

Fixes #63423
Follow-up to #94236

Quits the blitting method when a lightmap cannot fit into an atlas. This should never happen though, and is only done to prevent an infinite loop.

TODO:

  • Return a more descriptive error. The current one pops up a window informing the user about the atlas being too big, which is not the case here.

@BlueCube3310 BlueCube3310 force-pushed the lightmap-atlas-loop-fix-always branch from c0ea402 to ca5bd2c Compare July 12, 2024 09:24
@BlueCube3310 BlueCube3310 changed the title Lightmapper: Prevent infinite loop during UV atlas blitting Lightmapper: Prevent infinite loop while blitting lightmaps into an atlas Jul 12, 2024
@BlueCube3310 BlueCube3310 marked this pull request as ready for review July 12, 2024 09:25
@BlueCube3310 BlueCube3310 requested review from a team as code owners July 12, 2024 09:25
@BlueCube3310 BlueCube3310 force-pushed the lightmap-atlas-loop-fix-always branch from ca5bd2c to 63eb6a7 Compare July 12, 2024 09:43
@BlueCube3310 BlueCube3310 requested a review from a team as a code owner July 12, 2024 09:43
@Chaosus Chaosus added this to the 4.3 milestone Jul 12, 2024
@BlueCube3310 BlueCube3310 mentioned this pull request Jul 12, 2024
Merged
Calinou
Calinou approved these changes Jul 13, 2024
View reviewed changes
Copy link
Member

@Calinou Calinou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I can't reproduce the issue in #63423 MRP as of 97b8ad1, but this PR works fine on the MRP too. Either way, adding this safeguard makes sense.

AThousandShips
AThousandShips approved these changes Jul 13, 2024
View reviewed changes
scene/3d/lightmap_gi.h Outdated Show resolved Hide resolved
scene/3d/lightmapper.h Outdated Show resolved Hide resolved
@BlueCube3310 BlueCube3310 force-pushed the lightmap-atlas-loop-fix-always branch from 63eb6a7 to 60a255a Compare July 13, 2024 11:10
@BlueCube3310 BlueCube3310 mentioned this pull request Jul 14, 2024
Closed
@akien-mga akien-mga requested a review from clayjohn July 17, 2024 09:37
@akien-mga akien-mga merged commit 43902f4 into godotengine:master Jul 18, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@BlueCube3310 BlueCube3310 deleted the lightmap-atlas-loop-fix-always branch July 22, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Calinou Calinou Calinou approved these changes

@clayjohn clayjohn clayjohn approved these changes

@AThousandShips AThousandShips AThousandShips approved these changes

Assignees
No one assigned
Labels
bug topic:rendering topic:3d
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

LightmapGI: Freeze when trying to bake with custom UV2
6 participants
@BlueCube3310 @akien-mga @Calinou @clayjohn @AThousandShips @Chaosus