From 0015a9a0e73c9e608757c6c83b30f659fc089936 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 24 Nov 2019 13:07:16 +0530 Subject: [PATCH 1/5] gltfpack: Unify encodeBasisData and encodeBasisFile We now exclusively rely on in-memory encoding and do the necessary file operations ourselves. This makes the flow slightly more straightforward and, more importantly, allows us to reshuffle Basis bits into KTX format without too much effort. --- tools/gltfpack.cpp | 53 ++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/tools/gltfpack.cpp b/tools/gltfpack.cpp index 26577d0da..cf8f75f52 100644 --- a/tools/gltfpack.cpp +++ b/tools/gltfpack.cpp @@ -2740,8 +2740,15 @@ bool writeFile(const char* path, const std::string& data) return result == data.size(); } -bool encodeBasisFile(const char* input, const char* output, bool normal_map, int quality) +bool encodeBasis(const std::string& data, std::string& result, bool normal_map, int quality, const char* output_path) { + std::string temp_name = getFileName(output_path) + ".temp"; + std::string temp_input = getFullPath(temp_name.c_str(), output_path) + ".png"; + std::string temp_output = getFullPath(temp_name.c_str(), output_path) + ".basis"; + + if (!writeFile(temp_input.c_str(), data)) + return false; + std::string cmd = "basisu"; char ql[16]; @@ -2758,9 +2765,9 @@ bool encodeBasisFile(const char* input, const char* output, bool normal_map, int } cmd += " -file "; - cmd += input; + cmd += temp_input; cmd += " -output_file "; - cmd += output; + cmd += temp_output; #ifdef _WIN32 cmd += " >nul"; @@ -2768,22 +2775,13 @@ bool encodeBasisFile(const char* input, const char* output, bool normal_map, int cmd += " >/dev/null"; #endif - return system(cmd.c_str()) == 0; -} + int rc = system(cmd.c_str()); -bool encodeBasisData(const std::string& data, std::string& result, bool normal_map, int quality, const char* output_path) -{ - std::string temp_name = getFileName(output_path) + ".temp"; - std::string temp_input = getFullPath(temp_name.c_str(), output_path) + ".png"; - std::string temp_output = getFullPath(temp_name.c_str(), output_path) + ".basis"; - - bool ok = - writeFile(temp_input.c_str(), data) && - encodeBasisFile(temp_input.c_str(), temp_output.c_str(), normal_map, quality) && - readFile(temp_output.c_str(), result); + bool ok = rc == 0 && readFile(temp_output.c_str(), result); unlink(temp_input.c_str()); unlink(temp_output.c_str()); + return ok; } @@ -2821,7 +2819,7 @@ void writeImage(std::string& json, std::vector& views, const cgltf_i { std::string encoded; - if (encodeBasisData(img_data, encoded, info.normal_map, settings.texture_quality, output_path)) + if (encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality, output_path)) { writeEmbeddedImage(json, views, encoded.c_str(), encoded.size(), "image/basis"); } @@ -2843,15 +2841,28 @@ void writeImage(std::string& json, std::vector& views, const cgltf_i std::string basis_path = getFileName(image.uri) + ".basis"; std::string basis_full_path = getFullPath(basis_path.c_str(), output_path); - if (encodeBasisFile(full_path.c_str(), basis_full_path.c_str(), info.normal_map, settings.texture_quality)) + if (!readFile(full_path.c_str(), img_data)) { - append(json, "\"uri\":\""); - append(json, basis_path); - append(json, "\""); + fprintf(stderr, "Warning: unable to read image %s, skipping\n", image.uri); } else { - fprintf(stderr, "Warning: unable to encode image %s with Basis, skipping\n", image.uri); + std::string encoded; + + if (!encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality, output_path)) + { + fprintf(stderr, "Warning: unable to encode image %s with Basis, skipping\n", image.uri); + } + else if (!writeFile(basis_full_path.c_str(), encoded)) + { + fprintf(stderr, "Warning: unable to save Basis image %s, skipping\n", image.uri); + } + else + { + append(json, "\"uri\":\""); + append(json, basis_path); + append(json, "\""); + } } } else From 27852e051ada8496fe6203b2d501cb79d4de9135 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 20 Nov 2019 11:21:18 +0100 Subject: [PATCH 2/5] demo: Add Basis support to GLTFLoader This change probably doesn't make sense to upstream, but without it it's impossible to use embedded Basis images in glTF content for testing. Eventually KHR_image_ktx2 will replace this. --- demo/GLTFLoader.js | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/demo/GLTFLoader.js b/demo/GLTFLoader.js index dc25a055a..364cf3eff 100644 --- a/demo/GLTFLoader.js +++ b/demo/GLTFLoader.js @@ -14,6 +14,7 @@ THREE.GLTFLoader = ( function () { this.dracoLoader = null; this.ddsLoader = null; + this.basisLoader = null; this.meshoptDecoder = null; } @@ -111,6 +112,13 @@ THREE.GLTFLoader = ( function () { }, + setBasisLoader: function ( basisLoader ) { + + this.basisLoader = basisLoader; + return this; + + }, + setMeshoptDecoder: function ( decoder ) { this.meshoptDecoder = decoder; @@ -222,7 +230,8 @@ THREE.GLTFLoader = ( function () { path: path || this.resourcePath || '', crossOrigin: this.crossOrigin, - manager: this.manager + manager: this.manager, + basisLoader: this.basisLoader, } ); @@ -1970,11 +1979,29 @@ THREE.GLTFLoader = ( function () { var loader = options.manager.getHandler( sourceURI ); + if ( ! loader && textureExtensions[ EXTENSIONS.MSFT_TEXTURE_DDS ] ) { + + loader = parser.extensions[ EXTENSIONS.MSFT_TEXTURE_DDS ].ddsLoader + + } + + if ( ! loader && options.basisLoader ) { + + if ( source.uri !== undefined && source.uri.toLowerCase().endsWith(".basis") ) { + + loader = options.basisLoader; + + } else if ( source.bufferView !== undefined && source.mimeType == "image/basis" ) { + + loader = options.basisLoader; + + } + + } + if ( ! loader ) { - loader = textureExtensions[ EXTENSIONS.MSFT_TEXTURE_DDS ] - ? parser.extensions[ EXTENSIONS.MSFT_TEXTURE_DDS ].ddsLoader - : textureLoader; + loader = textureLoader; } From 02e50e32edce4e5cbb70b95b4f319a4ed751c11d Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 23 Nov 2019 18:45:31 +0530 Subject: [PATCH 3/5] demo: Allow direct loading of .dds textures with DDS loader This means we don't need to use MSFT_texture_dds either. I honestly don't understand why this is not the way these formats are integrated into glTF. --- demo/GLTFLoader.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/demo/GLTFLoader.js b/demo/GLTFLoader.js index 364cf3eff..46d3cfe99 100644 --- a/demo/GLTFLoader.js +++ b/demo/GLTFLoader.js @@ -231,6 +231,7 @@ THREE.GLTFLoader = ( function () { path: path || this.resourcePath || '', crossOrigin: this.crossOrigin, manager: this.manager, + ddsLoader: this.ddsLoader, basisLoader: this.basisLoader, } ); @@ -1985,6 +1986,20 @@ THREE.GLTFLoader = ( function () { } + if ( ! loader && options.ddsLoader ) { + + if ( source.uri !== undefined && source.uri.toLowerCase().endsWith(".dds") ) { + + loader = options.ddsLoader; + + } else if ( source.bufferView !== undefined && source.mimeType == "image/vnd-ms.dds" ) { + + loader = options.ddsLoader; + + } + + } + if ( ! loader && options.basisLoader ) { if ( source.uri !== undefined && source.uri.toLowerCase().endsWith(".basis") ) { From 8b39a362b58e0136ed248c33db87b0c7dab33eac Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 24 Nov 2019 23:33:41 +0530 Subject: [PATCH 4/5] gltfpack: Allow external customization of basisu executable path By default we assume it can be found in PATH but it's now possible to set BASISU_PATH to override this. --- tools/gltfpack.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/gltfpack.cpp b/tools/gltfpack.cpp index cf8f75f52..fa1507766 100644 --- a/tools/gltfpack.cpp +++ b/tools/gltfpack.cpp @@ -2749,7 +2749,8 @@ bool encodeBasis(const std::string& data, std::string& result, bool normal_map, if (!writeFile(temp_input.c_str(), data)) return false; - std::string cmd = "basisu"; + const char* basisu_path = getenv("BASISU_PATH"); + std::string cmd = basisu_path ? basisu_path : "basisu"; char ql[16]; sprintf(ql, "%d", (quality * 255 + 50) / 100); From 9672d2bb6e3f6b3cfd89d11b25736df48b6dc062 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 25 Nov 2019 11:22:33 +0100 Subject: [PATCH 5/5] gltfpack: Use temporary directory for temporary files Instead of writing temporary files side by side with the output file, we write them to the temporary folder instead. This means that if the process crashes, we don't keep files in the output folder. Note that the files need a suffix to work around basisu issue where the output file name has to have a .basis extension. For this to work we need to use mkstemps on POSIX systems to avoid race conditions. --- tools/gltfpack.cpp | 64 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/tools/gltfpack.cpp b/tools/gltfpack.cpp index fa1507766..22378d91e 100644 --- a/tools/gltfpack.cpp +++ b/tools/gltfpack.cpp @@ -34,7 +34,9 @@ #include #include -#ifndef _WIN32 +#ifdef _WIN32 +#include +#else #include #endif @@ -1663,9 +1665,9 @@ StreamFormat writeKeyframeStream(std::string& bin, cgltf_animation_path_type typ const Attr& a = data[i]; float v[3] = { - meshopt_quantizeFloat(a.f[0], bits), - meshopt_quantizeFloat(a.f[1], bits), - meshopt_quantizeFloat(a.f[2], bits)}; + meshopt_quantizeFloat(a.f[0], bits), + meshopt_quantizeFloat(a.f[1], bits), + meshopt_quantizeFloat(a.f[2], bits)}; bin.append(reinterpret_cast(v), sizeof(v)); } @@ -2740,13 +2742,42 @@ bool writeFile(const char* path, const std::string& data) return result == data.size(); } -bool encodeBasis(const std::string& data, std::string& result, bool normal_map, int quality, const char* output_path) +struct TempFile { - std::string temp_name = getFileName(output_path) + ".temp"; - std::string temp_input = getFullPath(temp_name.c_str(), output_path) + ".png"; - std::string temp_output = getFullPath(temp_name.c_str(), output_path) + ".basis"; + std::string path; + int fd; + + TempFile(const char* suffix) + : fd(-1) + { +#ifdef _WIN32 + const char* temp_dir = getenv("TEMP"); + path = temp_dir ? temp_dir : "."; + path += "\\gltfpack-XXXXXX"; + (void)_mktemp(&path[0]); + path += suffix; +#else + path = "/tmp/gltfpack-XXXXXX"; + path += suffix; + fd = mkstemps(&path[0], strlen(suffix)); +#endif + } - if (!writeFile(temp_input.c_str(), data)) + ~TempFile() + { + unlink(path.c_str()); +#ifndef _WIN32 + close(fd); +#endif + } +}; + +bool encodeBasis(const std::string& data, std::string& result, bool normal_map, int quality) +{ + TempFile temp_input(".raw"); + TempFile temp_output(".basis"); + + if (!writeFile(temp_input.path.c_str(), data)) return false; const char* basisu_path = getenv("BASISU_PATH"); @@ -2766,9 +2797,9 @@ bool encodeBasis(const std::string& data, std::string& result, bool normal_map, } cmd += " -file "; - cmd += temp_input; + cmd += temp_input.path; cmd += " -output_file "; - cmd += temp_output; + cmd += temp_output.path; #ifdef _WIN32 cmd += " >nul"; @@ -2778,12 +2809,7 @@ bool encodeBasis(const std::string& data, std::string& result, bool normal_map, int rc = system(cmd.c_str()); - bool ok = rc == 0 && readFile(temp_output.c_str(), result); - - unlink(temp_input.c_str()); - unlink(temp_output.c_str()); - - return ok; + return rc == 0 && readFile(temp_output.path.c_str(), result); } void writeImage(std::string& json, std::vector& views, const cgltf_image& image, const ImageInfo& info, size_t index, const char* input_path, const char* output_path, const Settings& settings) @@ -2820,7 +2846,7 @@ void writeImage(std::string& json, std::vector& views, const cgltf_i { std::string encoded; - if (encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality, output_path)) + if (encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality)) { writeEmbeddedImage(json, views, encoded.c_str(), encoded.size(), "image/basis"); } @@ -2850,7 +2876,7 @@ void writeImage(std::string& json, std::vector& views, const cgltf_i { std::string encoded; - if (!encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality, output_path)) + if (!encodeBasis(img_data, encoded, info.normal_map, settings.texture_quality)) { fprintf(stderr, "Warning: unable to encode image %s with Basis, skipping\n", image.uri); }