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

Handle mipmaps and VRAM compression for glTF binary images #62499

Merged
merged 1 commit into from
Jan 27, 2023
+241 −55
Merged

Handle mipmaps and VRAM compression for glTF binary images #62499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion core/string/ustring.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4342,6 +4342,9 @@ bool String::is_valid_html_color() const {
return Color::html_is_valid(*this);
}

// Changes made to the set of invalid filename characters must also be reflected in the String documentation for is_valid_filename.
static const char *invalid_filename_characters = ": / \\ ? * \" | % < >";

bool String::is_valid_filename() const {
String stripped = strip_edges();
if (*this != stripped) {
Expand All @@ -4352,7 +4355,22 @@ bool String::is_valid_filename() const {
return false;
}

return !(find(":") != -1 || find("/") != -1 || find("\\") != -1 || find("?") != -1 || find("*") != -1 || find("\"") != -1 || find("|") != -1 || find("%") != -1 || find("<") != -1 || find(">") != -1);
Vector<String> chars = String(invalid_filename_characters).split(" ");
for (const String &ch : chars) {
if (contains(ch)) {
return false;
}
}
return true;
}

String String::validate_filename() const {
Vector<String> chars = String(invalid_filename_characters).split(" ");
Copy link
Member

@akien-mga akien-mga Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Could be LocalVector maybe.
Edit: Nevermind, I guess split() returns Vector<String> so you can't just assign it to a different container.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is .split() allocates a Vector. validate_node_name does the same. It's called infrequently in imoprt code.
I was originally thinking of doing .replace().replace()...

Copy link
Member

Choose a reason for hiding this comment

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

It can be for a follow-up PR if needed/relevant, but I guess invalid_filename_characters could be made a static LocalVector<CharString> with all the chars already split, so we don't need to redo this computation for what's going to always be the same every time we validate a filename.

String name = strip_edges();
for (int i = 0; i < chars.size(); i++) {
name = name.replace(chars[i], "_");
}
return name;
}

bool String::is_valid_ip_address() const {
Expand Down
1 change: 1 addition & 0 deletions core/string/ustring.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ class String {
static const String invalid_node_name_characters;
String validate_node_name() const;
String validate_identifier() const;
String validate_filename() const;

bool is_valid_identifier() const;
bool is_valid_int() const;
Expand Down
1 change: 1 addition & 0 deletions core/variant/variant_call.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,7 @@ static void _register_variant_builtin_methods() {
bind_string_method(json_escape, sarray(), varray());

bind_string_method(validate_node_name, sarray(), varray());
bind_string_method(validate_filename, sarray(), varray());

bind_string_method(is_valid_identifier, sarray(), varray());
bind_string_method(is_valid_int, sarray(), varray());
Expand Down
6 changes: 6 additions & 0 deletions doc/classes/String.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,12 @@
[/codeblocks]
</description>
</method>
<method name="validate_filename" qualifiers="const">
<return type="String" />
<description>
Replace all characters that are not allowed in [method is_valid_filename] with underscores.
</description>
</method>
<method name="validate_node_name" qualifiers="const">
<return type="String" />
<description>
Expand Down
16 changes: 6 additions & 10 deletions editor/import/resource_importer_texture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,11 @@ void ResourceImporterTexture::save_to_ctex_format(Ref<FileAccess> f, const Ref<I
f->store_16(p_image->get_height());
f->store_32(p_image->get_mipmap_count());
f->store_32(p_image->get_format());

for (int i = 0; i < p_image->get_mipmap_count() + 1; i++) {
Vector<uint8_t> data = Image::basis_universal_packer(p_image->get_image_from_mipmap(i), p_channels);
int data_len = data.size();
f->store_32(data_len);

const uint8_t *r = data.ptr();
f->store_buffer(r, data_len);
}
Vector<uint8_t> data = Image::basis_universal_packer(p_image, p_channels);
int data_len = data.size();
f->store_32(data_len);
const uint8_t *r = data.ptr();
f->store_buffer(r, data_len);
} break;
}
}
Expand Down Expand Up @@ -387,7 +383,7 @@ void ResourceImporterTexture::_save_ctex(const Ref<Image> &p_image, const String

Ref<Image> image = p_image->duplicate();

if (((p_compress_mode == COMPRESS_BASIS_UNIVERSAL) || (p_compress_mode == COMPRESS_VRAM_COMPRESSED && p_force_po2_for_compressed)) && p_mipmaps) {
if (p_force_po2_for_compressed && p_mipmaps && ((p_compress_mode == COMPRESS_BASIS_UNIVERSAL) || (p_compress_mode == COMPRESS_VRAM_COMPRESSED))) {
image->resize_to_po2();
}

Expand Down
53 changes: 29 additions & 24 deletions modules/basis_universal/register_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,44 +52,50 @@ enum BasisDecompressFormat {
#ifdef TOOLS_ENABLED
static Vector<uint8_t> basis_universal_packer(const Ref<Image> &p_image, Image::UsedChannels p_channels) {
Vector<uint8_t> budata;

{
basisu::basis_compressor_params params;
Ref<Image> image = p_image->duplicate();

// unfortunately, basis universal does not support compressing supplied mipmaps,
// so for the time being, only compressing individual images will have to do.

if (image->has_mipmaps()) {
image->clear_mipmaps();
}
if (image->get_format() != Image::FORMAT_RGBA8) {
image->convert(Image::FORMAT_RGBA8);
}

basisu::image buimg(image->get_width(), image->get_height());

Ref<Image> image_single = image->duplicate();
{
Vector<uint8_t> vec = image->get_data();
if (image_single->has_mipmaps()) {
image_single->clear_mipmaps();
}
basisu::image buimg(image_single->get_width(), image_single->get_height());
Vector<uint8_t> vec = image_single->get_data();
const uint8_t *r = vec.ptr();

memcpy(buimg.get_ptr(), r, vec.size());
params.m_source_images.push_back(buimg);
}
basisu::vector<basisu::image> source_images;
for (int32_t mipmap_i = 1; mipmap_i < image->get_mipmap_count(); mipmap_i++) {
Ref<Image> mip = image->get_image_from_mipmap(mipmap_i);
basisu::image buimg(mip->get_width(), mip->get_height());
Vector<uint8_t> vec = mip->get_data();
const uint8_t *r = vec.ptr();
memcpy(buimg.get_ptr(), r, vec.size());
source_images.push_back(buimg);
}

basisu::basis_compressor_params params;
params.m_uastc = true;
params.m_max_endpoint_clusters = 512;
params.m_max_selector_clusters = 512;
params.m_quality_level = basisu::BASISU_QUALITY_MIN;

params.m_pack_uastc_flags &= ~basisu::cPackUASTCLevelMask;

static const uint32_t s_level_flags[basisu::TOTAL_PACK_UASTC_LEVELS] = { basisu::cPackUASTCLevelFastest, basisu::cPackUASTCLevelFaster, basisu::cPackUASTCLevelDefault, basisu::cPackUASTCLevelSlower, basisu::cPackUASTCLevelVerySlow };
params.m_pack_uastc_flags |= s_level_flags[0];
params.m_rdo_uastc = 0.0f;
params.m_rdo_uastc_quality_scalar = 0.0f;
params.m_rdo_uastc_dict_size = 1024;

params.m_mip_fast = true;
params.m_multithreading = true;
//params.m_quality_level = 0;
//params.m_disable_hierarchical_endpoint_codebooks = true;
//params.m_no_selector_rdo = true;

basisu::job_pool jpool(OS::get_singleton()->get_processor_count());
params.m_pJob_pool = &jpool;

params.m_mip_gen = false; //sorry, please some day support provided mipmaps.
params.m_source_images.push_back(buimg);

BasisDecompressFormat decompress_format = BASIS_DECOMPRESS_RG;
params.m_check_for_alpha = false;

Expand Down Expand Up @@ -252,8 +258,7 @@ static Ref<Image> basis_universal_unpacker_ptr(const uint8_t *p_data, int p_size
};
};

image.instantiate();
image->set_data(info.m_width, info.m_height, info.m_total_levels > 1, imgfmt, gpudata);
image = Image::create_from_data(info.m_width, info.m_height, info.m_total_levels > 1, imgfmt, gpudata);

return image;
}
Expand Down
11 changes: 11 additions & 0 deletions modules/gltf/doc_classes/GLTFState.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@
<description>
</description>
</method>
<method name="get_handle_binary_image">
<return type="int" />
<description>
</description>
</method>
<method name="get_images">
<return type="Texture2D[]" />
<description>
Expand Down Expand Up @@ -155,6 +160,12 @@
<description>
</description>
</method>
<method name="set_handle_binary_image">
<return type="void" />
<param index="0" name="method" type="int" />
<description>
</description>
</method>
<method name="set_images">
<return type="void" />
<param index="0" name="images" type="Texture2D[]" />
Expand Down
6 changes: 2 additions & 4 deletions modules/gltf/editor/editor_scene_importer_blend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#ifdef TOOLS_ENABLED

#include "../gltf_defines.h"
#include "../gltf_document.h"

#include "core/config/project_settings.h"
Expand All @@ -43,7 +44,6 @@
#include "scene/gui/line_edit.h"

#ifdef WINDOWS_ENABLED
// Code by Pedro Estebanez (https://github.com/godotengine/godot/pull/59766)
#include <shlwapi.h>
#endif

Expand Down Expand Up @@ -221,6 +221,7 @@ Node *EditorSceneFormatImporterBlend::import_scene(const String &p_path, uint32_
gltf.instantiate();
Ref<GLTFState> state;
state.instantiate();

String base_dir;
if (p_options.has(SNAME("blender/materials/unpack_enabled")) && p_options[SNAME("blender/materials/unpack_enabled")]) {
base_dir = sink.get_base_dir();
Expand Down Expand Up @@ -274,9 +275,6 @@ void EditorSceneFormatImporterBlend::get_import_options(const String &p_path, Li
ADD_OPTION_BOOL("blender/animation/limit_playback", true);
ADD_OPTION_BOOL("blender/animation/always_sample", true);
ADD_OPTION_BOOL("blender/animation/group_tracks", true);

#undef ADD_OPTION_BOOL
#undef ADD_OPTION_ENUM
}

///////////////////////////
Expand Down
10 changes: 10 additions & 0 deletions modules/gltf/editor/editor_scene_importer_gltf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include "editor_scene_importer_gltf.h"

#include "../gltf_defines.h"
#include "../gltf_document.h"

uint32_t EditorSceneFormatImporterGLTF::get_import_flags() const {
Expand All @@ -50,6 +51,10 @@ Node *EditorSceneFormatImporterGLTF::import_scene(const String &p_path, uint32_t
doc.instantiate();
Ref<GLTFState> state;
state.instantiate();
if (p_options.has("meshes/handle_gltf_embedded_images")) {
int32_t enum_option = p_options["meshes/handle_gltf_embedded_images"];
state->set_handle_binary_image(enum_option);
}
Error err = doc->append_from_file(p_path, state, p_flags);
if (err != OK) {
if (r_err) {
Expand All @@ -68,4 +73,9 @@ Node *EditorSceneFormatImporterGLTF::import_scene(const String &p_path, uint32_t
}
}

void EditorSceneFormatImporterGLTF::get_import_options(const String &p_path,
List<ResourceImporter::ImportOption> *r_options) {
r_options->push_back(ResourceImporterScene::ImportOption(PropertyInfo(Variant::INT, "meshes/handle_gltf_embedded_images", PROPERTY_HINT_ENUM, "Discard All Textures,Extract Textures,Embed As Basis Universal", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_UPDATE_ALL_IF_MODIFIED), GLTFState::HANDLE_BINARY_EXTRACT_TEXTURES));
}

#endif // TOOLS_ENABLED
2 changes: 2 additions & 0 deletions modules/gltf/editor/editor_scene_importer_gltf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class EditorSceneFormatImporterGLTF : public EditorSceneFormatImporter {
virtual Node *import_scene(const String &p_path, uint32_t p_flags,
const HashMap<StringName, Variant> &p_options,
List<String> *r_missing_deps, Error *r_err = nullptr) override;
virtual void get_import_options(const String &p_path,
List<ResourceImporter::ImportOption> *r_options) override;
};

#endif // TOOLS_ENABLED
Expand Down
Loading