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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/classes/LightmapGI.xml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@
<constant name="BAKE_ERROR_TEXTURE_SIZE_TOO_SMALL" value="9" enum="BakeError">
Lightmap baking failed as the maximum texture size is too small to fit some of the meshes marked for baking.
</constant>
<constant name="BAKE_ERROR_LIGHTMAP_TOO_SMALL" value="10" enum="BakeError">
Lightmap baking failed as the lightmap is too small.
</constant>
<constant name="BAKE_ERROR_ATLAS_TOO_SMALL" value="11" enum="BakeError">
Lightmap baking failed as the lightmap was unable to fit into an atlas.
</constant>
<constant name="ENVIRONMENT_MODE_DISABLED" value="0" enum="EnvironmentMode">
Ignore environment lighting when baking lightmaps.
</constant>
Expand Down
3 changes: 3 additions & 0 deletions editor/plugins/lightmap_gi_editor_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ void LightmapGIEditorPlugin::_bake_select_file(const String &p_file) {
case LightmapGI::BAKE_ERROR_LIGHTMAP_TOO_SMALL: {
EditorNode::get_singleton()->show_warning(TTR("Failed creating lightmap images. Make sure all meshes selected to bake have `lightmap_size_hint` value set high enough, and `texel_scale` value of LightmapGI is not too low."));
} break;
case LightmapGI::BAKE_ERROR_ATLAS_TOO_SMALL: {
EditorNode::get_singleton()->show_warning(TTR("Failed fitting a lightmap image into an atlas. This should never happen and should be reported."));
} break;
default: {
} break;
}
Expand Down
16 changes: 12 additions & 4 deletions modules/lightmapper_rd/lightmapper_rd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,14 @@ Lightmapper::BakeError LightmapperRD::_blit_meshes_into_atlas(int p_max_texture_
MeshInstance &mi = mesh_instances.write[m_i];
Size2i s = Size2i(mi.data.albedo_on_uv2->get_width(), mi.data.albedo_on_uv2->get_height());
sizes.push_back(s);
atlas_size = atlas_size.max(s + Size2i(2, 2));
atlas_size = atlas_size.max(s + Size2i(2, 2).maxi(p_denoiser_range));
}

int max = nearest_power_of_2_templated(atlas_size.width);
max = MAX(max, nearest_power_of_2_templated(atlas_size.height));

if (max > p_max_texture_size) {
return BAKE_ERROR_LIGHTMAP_TOO_SMALL;
return BAKE_ERROR_TEXTURE_EXCEEDS_MAX_SIZE;
}

if (p_step_function) {
Expand All @@ -254,19 +254,27 @@ Lightmapper::BakeError LightmapperRD::_blit_meshes_into_atlas(int p_max_texture_
int best_atlas_memory = 0x7FFFFFFF;
Vector<Vector3i> best_atlas_offsets;

//determine best texture array atlas size by bruteforce fitting
// Determine best texture array atlas size by bruteforce fitting.
while (atlas_size.x <= p_max_texture_size && atlas_size.y <= p_max_texture_size) {
Vector<Vector2i> source_sizes;
Vector<int> source_indices;
source_sizes.resize(sizes.size());
source_indices.resize(sizes.size());
for (int i = 0; i < source_indices.size(); i++) {
source_sizes.write[i] = sizes[i] + Vector2i(2, 2).maxi(p_denoiser_range); // Add padding between lightmaps
source_sizes.write[i] = sizes[i] + Vector2i(2, 2).maxi(p_denoiser_range); // Add padding between lightmaps.
source_indices.write[i] = i;
}
Vector<Vector3i> atlas_offsets;
atlas_offsets.resize(source_sizes.size());

// Ensure the sizes can all fit into a single atlas layer.
// This should always happen, and this check is only in place to prevent an infinite loop.
for (int i = 0; i < source_sizes.size(); i++) {
if (source_sizes[i] > atlas_size) {
return BAKE_ERROR_ATLAS_TOO_SMALL;
}
}

int slices = 0;

while (source_sizes.size() > 0) {
Expand Down
6 changes: 5 additions & 1 deletion scene/3d/lightmap_gi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1104,10 +1104,12 @@ LightmapGI::BakeError LightmapGI::bake(Node *p_from_node, String p_image_data_pa

Lightmapper::BakeError bake_err = lightmapper->bake(Lightmapper::BakeQuality(bake_quality), use_denoiser, denoiser_strength, denoiser_range, bounces, bounce_indirect_energy, bias, max_texture_size, directional, use_texture_for_bounces, Lightmapper::GenerateProbes(gen_probes), environment_image, environment_transform, _lightmap_bake_step_function, &bsud, exposure_normalization);

if (bake_err == Lightmapper::BAKE_ERROR_LIGHTMAP_TOO_SMALL) {
if (bake_err == Lightmapper::BAKE_ERROR_TEXTURE_EXCEEDS_MAX_SIZE) {
return BAKE_ERROR_TEXTURE_SIZE_TOO_SMALL;
} else if (bake_err == Lightmapper::BAKE_ERROR_LIGHTMAP_CANT_PRE_BAKE_MESHES) {
return BAKE_ERROR_MESHES_INVALID;
} else if (bake_err == Lightmapper::BAKE_ERROR_ATLAS_TOO_SMALL) {
return BAKE_ERROR_ATLAS_TOO_SMALL;
}

// POSTBAKE: Save Textures.
Expand Down Expand Up @@ -1711,6 +1713,8 @@ void LightmapGI::_bind_methods() {
BIND_ENUM_CONSTANT(BAKE_ERROR_CANT_CREATE_IMAGE);
BIND_ENUM_CONSTANT(BAKE_ERROR_USER_ABORTED);
BIND_ENUM_CONSTANT(BAKE_ERROR_TEXTURE_SIZE_TOO_SMALL);
BIND_ENUM_CONSTANT(BAKE_ERROR_LIGHTMAP_TOO_SMALL);
BIND_ENUM_CONSTANT(BAKE_ERROR_ATLAS_TOO_SMALL);

BIND_ENUM_CONSTANT(ENVIRONMENT_MODE_DISABLED);
BIND_ENUM_CONSTANT(ENVIRONMENT_MODE_SCENE);
Expand Down
1 change: 1 addition & 0 deletions scene/3d/lightmap_gi.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ class LightmapGI : public VisualInstance3D {
BAKE_ERROR_USER_ABORTED,
BAKE_ERROR_TEXTURE_SIZE_TOO_SMALL,
BAKE_ERROR_LIGHTMAP_TOO_SMALL,
BAKE_ERROR_ATLAS_TOO_SMALL,
};

enum EnvironmentMode {
Expand Down
5 changes: 3 additions & 2 deletions scene/3d/lightmapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,10 @@ class Lightmapper : public RefCounted {
};

enum BakeError {
BAKE_ERROR_LIGHTMAP_TOO_SMALL,
BAKE_OK,
BAKE_ERROR_TEXTURE_EXCEEDS_MAX_SIZE,
BAKE_ERROR_LIGHTMAP_CANT_PRE_BAKE_MESHES,
BAKE_OK
BAKE_ERROR_ATLAS_TOO_SMALL,
};

enum BakeQuality {
Expand Down
Loading