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

Add config warnings for texture size/format/mipmap mismatch #591

Merged
merged 1 commit into from
Jan 7, 2025
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 src/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ using namespace godot;
#define V3_ZERO Vector3(0.f, 0.f, 0.f)
#define V3_MAX Vector3(FLT_MAX, FLT_MAX, FLT_MAX)

// Terrain3D::_warnings is uint8_t
#define WARN_MISMATCHED_SIZE 0x01
#define WARN_MISMATCHED_FORMAT 0x02
#define WARN_MISMATCHED_MIPMAPS 0x04
#define WARN_ALL 0xFF

// Set class name for logger.h

#define CLASS_NAME() const String __class__ = get_class_static() + \
Expand Down
21 changes: 19 additions & 2 deletions src/terrain_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,9 @@ void Terrain3D::set_data_directory(String p_dir) {
_data_directory = p_dir;
_initialize();
}
update_configuration_warnings();
}

void Terrain3D::set_material(const Ref<Terrain3DMaterial> &p_material) {
if (_material != p_material) {
_clear_meshes();
Expand Down Expand Up @@ -1291,13 +1293,28 @@ PackedVector3Array Terrain3D::generate_nav_mesh_source_geometry(const AABB &p_gl
return faces;
}

void Terrain3D::set_warning(const uint8_t p_warning, const bool p_enabled) {
if (p_enabled) {
_warnings |= p_warning;
} else {
_warnings &= ~p_warning;
}
update_configuration_warnings();
}

PackedStringArray Terrain3D::_get_configuration_warnings() const {
PackedStringArray psa;
if (_data_directory.is_empty()) {
psa.push_back("No data directory specified. Select a directory then save the scene to write data.");
}
if (!psa.is_empty()) {
psa.push_back("To update this message, deselect and reselect Terrain3D in the Scene panel.");
if (_warnings & WARN_MISMATCHED_SIZE) {
psa.push_back("Texture dimensions don't match. Double-click a texture in the FileSystem panel to see its size. Read Texture Prep in docs.");
}
if (_warnings & WARN_MISMATCHED_FORMAT) {
psa.push_back("Texture formats don't match. Double-click a texture in the FileSystem panel to see its format. Check Import panel. Read Texture Prep in docs.");
}
if (_warnings & WARN_MISMATCHED_MIPMAPS) {
psa.push_back("Texture mipmap settings don't match. Change on the Import panel.");
}
return psa;
}
Expand Down
5 changes: 4 additions & 1 deletion src/terrain_3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Terrain3D : public Node3D {
String _data_directory;
bool _is_inside_world = false;
bool _initialized = false;
uint8_t _warnings = 0;

// Object references
Terrain3DData *_data = nullptr;
Expand Down Expand Up @@ -253,7 +254,9 @@ class Terrain3D : public Node3D {
Ref<Mesh> bake_mesh(const int p_lod, const Terrain3DData::HeightFilter p_filter = Terrain3DData::HEIGHT_FILTER_NEAREST) const;
PackedVector3Array generate_nav_mesh_source_geometry(const AABB &p_global_aabb, const bool p_require_nav = true) const;

// Godot Callbacks
// Warnings
void set_warning(const uint8_t p_warning, const bool p_enabled);
uint8_t get_warnings() const { return _warnings; }
PackedStringArray _get_configuration_warnings() const override;

protected:
Expand Down
84 changes: 56 additions & 28 deletions src/terrain_3d_assets.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright © 2025 Cory Petkovsek, Roope Palmroos, and Contributors.

#include <godot_cpp/classes/engine.hpp>
#include <godot_cpp/classes/environment.hpp>
#include <godot_cpp/classes/image_texture.hpp>
#include <godot_cpp/classes/rendering_server.hpp>
Expand Down Expand Up @@ -165,15 +166,14 @@ void Terrain3DAssets::_update_texture_files() {
}

// Detect image sizes and formats

LOG(INFO, "Validating texture sizes");
LOG(DEBUG, "Validating texture sizes");
Vector2i albedo_size = V2I_ZERO;
Vector2i normal_size = V2I_ZERO;

Image::Format albedo_format = Image::FORMAT_MAX;
Image::Format normal_format = Image::FORMAT_MAX;
bool albedo_mipmaps = true;
bool normal_mipmaps = true;
_terrain->set_warning(WARN_ALL, false);
//DEPRECATED 1.0 - remove in Godot 4.4
bool warn_compatibility_decompress = false;

Expand All @@ -182,58 +182,80 @@ void Terrain3DAssets::_update_texture_files() {
if (texture_set.is_null()) {
continue;
}
Ref<Texture2D> albedo_tex = texture_set->_albedo_texture;
Ref<Texture2D> normal_tex = texture_set->_normal_texture;

// If this is the first texture, set expected size and format for the arrays
Ref<Texture2D> albedo_tex = texture_set->_albedo_texture;
if (albedo_tex.is_valid()) {
Vector2i tex_size = albedo_tex->get_size();
if (albedo_size.length() == 0.0) {
albedo_size = tex_size;
} else if (tex_size != albedo_size) {
LOG(ERROR, "Texture ID ", i, " albedo size: ", tex_size, " doesn't match size of first texture: ", albedo_size, ". They must be identical. Read Texture Prep in docs.");
return;
}
Ref<Image> img = albedo_tex->get_image();
Image::Format format = img->get_format();
bool mipmaps = img->has_mipmaps();

//DEPRECATED 1.0 - remove in Godot 4.4
if (_terrain->is_compatibility_mode() && img->is_compressed()) {
warn_compatibility_decompress = true;
img->decompress();
}
Image::Format format = img->get_format();

// If this is the first valid texture, set expected size and format for the arrays
if (albedo_format == Image::FORMAT_MAX) {
albedo_size = tex_size;
albedo_format = format;
albedo_mipmaps = img->has_mipmaps();
} else if (format != albedo_format) {
LOG(ERROR, "Texture ID ", i, " albedo format: ", format, " doesn't match format of first texture: ", albedo_format, ". They must be identical. Read Texture Prep in docs.");
return;
albedo_mipmaps = mipmaps;
} else { // else validate against first texture
if (tex_size != albedo_size) {
_terrain->set_warning(WARN_MISMATCHED_SIZE, true);
LOG(ERROR, "Texture ID ", i, " albedo size: ", tex_size, " doesn't match size of first texture: ", albedo_size, ". They must be identical. Read Texture Prep in docs.");
}
if (format != albedo_format) {
_terrain->set_warning(WARN_MISMATCHED_FORMAT, true);
LOG(ERROR, "Texture ID ", i, " albedo format: ", format, " doesn't match format of first texture: ", albedo_format, ". They must be identical. Read Texture Prep in docs.");
}
if (mipmaps != albedo_mipmaps) {
_terrain->set_warning(WARN_MISMATCHED_MIPMAPS, true);
LOG(ERROR, "Texture ID ", i, " albedo mipmap setting (", mipmaps, ") doesn't match first texture (", albedo_mipmaps, "). They must be identical. Read Texture Prep in docs.");
}
}
}

Ref<Texture2D> normal_tex = texture_set->_normal_texture;
if (normal_tex.is_valid()) {
Vector2i tex_size = normal_tex->get_size();
if (normal_size.length() == 0.0) {
normal_size = tex_size;
} else if (tex_size != normal_size) {
LOG(ERROR, "Texture ID ", i, " normal size: ", tex_size, " doesn't match size of first texture: ", normal_size, ". They must be identical. Read Texture Prep in docs.");
return;
}
Ref<Image> img = normal_tex->get_image();
Image::Format format = img->get_format();
bool mipmaps = img->has_mipmaps();

//DEPRECATED 1.0 - remove in Godot 4.4
if (_terrain->is_compatibility_mode() && img->is_compressed()) {
warn_compatibility_decompress = true;
img->decompress();
}
Image::Format format = img->get_format();

// If this is the first valid texture, set expected size and format for the arrays
if (normal_format == Image::FORMAT_MAX) {
normal_size = tex_size;
normal_format = format;
normal_mipmaps = img->has_mipmaps();
} else if (format != normal_format) {
LOG(ERROR, "Texture ID ", i, " normal format: ", format, " doesn't match format of first texture: ", normal_format, ". They must be identical. Read Texture Prep in docs.");
return;
normal_mipmaps = mipmaps;
} else { // else validate against first texture
if (tex_size != normal_size) {
_terrain->set_warning(WARN_MISMATCHED_SIZE, true);
LOG(ERROR, "Texture ID ", i, " normal size: ", tex_size, " doesn't match size of first texture: ", normal_size, ". They must be identical. Read Texture Prep in docs.");
}
if (format != normal_format) {
_terrain->set_warning(WARN_MISMATCHED_FORMAT, true);
LOG(ERROR, "Texture ID ", i, " normal format: ", format, " doesn't match format of first texture: ", normal_format, ". They must be identical. Read Texture Prep in docs.");
}
if (mipmaps != normal_mipmaps) {
_terrain->set_warning(WARN_MISMATCHED_MIPMAPS, true);
LOG(ERROR, "Texture ID ", i, " normal mipmap setting (", mipmaps, ") doesn't match first texture (", albedo_mipmaps, "). They must be identical. Read Texture Prep in docs.");
}
}
}
}
if (_terrain->get_warnings()) {
return;
}

// Setup defaults for generated texture
if (normal_size == V2I_ZERO) {
normal_size = albedo_size;
} else if (albedo_size == V2I_ZERO) {
Expand Down Expand Up @@ -270,6 +292,9 @@ void Terrain3DAssets::_update_texture_files() {
}
LOG(DEBUG, "ID ", i, " albedo texture is valid. Format: ", img->get_format());
}
if (!IS_EDITOR && !tex->get_path().get_file().is_valid_filename()) {
LOG(WARN, "ID ", i, " normal texture is not connected to a file.");
}
albedo_texture_array.push_back(img);
}
if (!albedo_texture_array.is_empty()) {
Expand Down Expand Up @@ -303,6 +328,9 @@ void Terrain3DAssets::_update_texture_files() {
}
LOG(DEBUG, "ID ", i, " Normal texture is valid. Format: ", img->get_format());
}
if (!IS_EDITOR && !tex->get_path().get_file().is_valid_filename()) {
LOG(WARN, "ID ", i, " normal texture is not connected to a file.");
}
normal_texture_array.push_back(img);
}
if (!normal_texture_array.is_empty()) {
Expand Down
32 changes: 29 additions & 3 deletions src/terrain_3d_texture_asset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,22 @@ void Terrain3DTextureAsset::set_albedo_texture(const Ref<Texture2D> &p_texture)
LOG(INFO, "Setting albedo texture: ", p_texture);
if (_is_valid_format(p_texture)) {
_albedo_texture = p_texture;
if (p_texture.is_valid() && _name == "New Texture") {
_name = p_texture->get_path().get_file().get_basename();
LOG(INFO, "Naming texture based on filename: ", _name);
if (p_texture.is_valid()) {
String filename = p_texture->get_path().get_file().get_basename();
if (_name == "New Texture") {
_name = filename;
LOG(INFO, "Naming texture based on filename: ", _name);
}
Ref<Image> img = p_texture->get_image();
if (!img->has_mipmaps()) {
LOG(WARN, "Texture '", filename, "' has no mipmaps. Change on the Import panel if desired.");
}
if (img->get_width() != img->get_height()) {
LOG(WARN, "Texture '", filename, "' is not square. Mipmaps might have artifacts.");
}
if (!is_power_of_2(img->get_width()) || !is_power_of_2(img->get_height())) {
LOG(WARN, "Texture '", filename, "' size is not power of 2. This is sub-optimal.");
}
}
emit_signal("file_changed");
}
Expand All @@ -78,6 +91,19 @@ void Terrain3DTextureAsset::set_normal_texture(const Ref<Texture2D> &p_texture)
LOG(INFO, "Setting normal texture: ", p_texture);
if (_is_valid_format(p_texture)) {
_normal_texture = p_texture;
if (p_texture.is_valid()) {
String filename = p_texture->get_path().get_file().get_basename();
Ref<Image> img = p_texture->get_image();
if (!img->has_mipmaps()) {
LOG(WARN, "Texture '", filename, "' has no mipmaps. Change on the Import panel if desired.");
}
if (img->get_width() != img->get_height()) {
LOG(WARN, "Texture '", filename, "' is not square. Not recommended. Mipmaps might have artifacts.");
}
if (!is_power_of_2(img->get_width()) || !is_power_of_2(img->get_height())) {
LOG(WARN, "Texture '", filename, "' dimensions are not power of 2. This is sub-optimal.");
}
}
emit_signal("file_changed");
}
}
Expand Down
Loading