libktx: Update to 4.3.1

[Chubercik]

https://github.com/KhronosGroup/KTX-Software/releases/tag/v4.3.1

Also updated thirdparty/README.md to be more precise and regenerated thirdparty/libktx/godot.patch.

Edit: Bundled miniz lib from upstream.

[Chubercik]

Seems like the guards employed in miniz_wrapper.cpp aren't working properly which results in a few multiply defined symbols - not sure how to fix that, haven't been exposed to such issues in a while..

[Chubercik]

Not sure what to do here, failed builds are the result of one include statement in miniz_wrapper.cpp - if the file is loaded as is, the linker complains about multiple definitions, but when only the header part of the file is loaded, template builds fail due to missing definitions.

I added two define statements in miniz_wrapper.cpp, so that basisu_miniz.h is loaded similarly as it is in the original basis_universal repository - this way at least editor builds compile.

[fire]

Please squash the commits https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html but I am generally ok with library upgrades.

Probably want to test a basisu ktx from gltf from https://github.com/KhronosGroup/glTF-Sample-Models/tree/main/2.0/StainedGlassLamp

[Chubercik]

This compiles on my end, both editor and template build; I'm gonna hold on before squishing commits for someone to approve the PR, if everything is safe and sound I'll push a rebase.

[akien-mga]

Changes look good overall!

The miniz situation can be solved in a better way with this extra diff:

diff --git a/modules/ktx/SCsub b/modules/ktx/SCsub
index 3a37ef93ee..887ffa5d14 100644
--- a/modules/ktx/SCsub
+++ b/modules/ktx/SCsub
@@ -12,11 +12,11 @@ thirdparty_obj = []
 thirdparty_dir = "#thirdparty/libktx/"
 thirdparty_sources = [
     "lib/basis_transcode.cpp",
-    "lib/miniz_wrapper.cpp",
     "lib/checkheader.c",
     "lib/filestream.c",
     "lib/hashlist.c",
     "lib/memstream.c",
+    "lib/miniz_wrapper.cpp",
     "lib/swap.c",
     "lib/texture.c",
     "lib/texture1.c",
@@ -35,7 +35,10 @@ env_ktx.Prepend(CPPPATH=[thirdparty_dir + "include"])
 env_ktx.Prepend(CPPPATH=[thirdparty_dir + "utils"])
 env_ktx.Prepend(CPPPATH=[thirdparty_dir + "lib"])
 env_ktx.Prepend(CPPPATH=[thirdparty_dir + "other_include"])
+
 env_ktx.Prepend(CPPPATH=["#thirdparty/basis_universal"])
+# We already build the miniz implementation in the basis_universal module.
+env_ktx.Append(CPPDEFINES=["MINIZ_HEADER_FILE_ONLY"])
 
 if env["vulkan"]:
     env_ktx.Prepend(CPPPATH=["#thirdparty/vulkan/include"])
diff --git a/thirdparty/libktx/lib/miniz_wrapper.cpp b/thirdparty/libktx/lib/miniz_wrapper.cpp
index 3707c1a806..cbd7da540a 100644
--- a/thirdparty/libktx/lib/miniz_wrapper.cpp
+++ b/thirdparty/libktx/lib/miniz_wrapper.cpp
@@ -22,7 +22,7 @@
 
 #include <assert.h>
 
-#ifndef TOOLS_ENABLED
+#if !KTX_FEATURE_WRITE
 // The reader does not link with the basisu components that already include a
 // definition of miniz so we include it here explicitly.
 #ifdef __GNUC__
diff --git a/thirdparty/libktx/patches/godot.patch b/thirdparty/libktx/patches/godot.patch
index 61a7997b97..28db86cf9b 100644
--- a/thirdparty/libktx/patches/godot.patch
+++ b/thirdparty/libktx/patches/godot.patch
@@ -36,18 +36,9 @@ index 85d53202a5..25c7a2c238 100644
      int channels[] = {0,1,2,3}; int bits[] = {5,5,5,1};
      return createDFDPacked(0, 4, bits, channels, s_UNORM);
 diff --git a/thirdparty/libktx/lib/miniz_wrapper.cpp b/thirdparty/libktx/lib/miniz_wrapper.cpp
-index 07920c4809..3707c1a806 100644
+index 07920c4809..cbd7da540a 100644
 --- a/thirdparty/libktx/lib/miniz_wrapper.cpp
 +++ b/thirdparty/libktx/lib/miniz_wrapper.cpp
-@@ -22,7 +22,7 @@
- 
- #include <assert.h>
- 
--#if !KTX_FEATURE_WRITE
-+#ifndef TOOLS_ENABLED
- // The reader does not link with the basisu components that already include a
- // definition of miniz so we include it here explicitly.
- #ifdef __GNUC__
 @@ -30,7 +30,7 @@
  #pragma GCC diagnostic ignored "-Wextra"
  #pragma GCC diagnostic ignored "-Wmisleading-indentation"

Basically passing MINIZ_HEADER_FILE_ONLY solves the issue - the header should only be included once without that define, to compile the implementation (equivalent to what a .cpp file should be for a normal two-file library). Yes, single header libraries sounded like a cool idea but they're dumb in practice.

[Chubercik]

Seems like we're back to square one:

Not sure what to do here, failed builds are the result of one include statement in miniz_wrapper.cpp - if the file is loaded as is, the linker complains about multiple definitions, but when only the header part of the file is loaded, template builds fail due to missing definitions.

The #ifndef TOOLS_ENABLED guard bypassed this issue by loading the definitions in the case of a template build.

[akien-mga]

That's because we don't build the basis universal encoder in non-editor builds:

godot/modules/basis_universal/SCsub

Lines 52 to 54 in 16d6142

	if env.editor_build:
	env_thirdparty.Append(CPPDEFINES=["BASISU_NO_IMG_LOADERS"])
	env_thirdparty.add_source_files(thirdparty_obj, encoder_sources)

So miniz is not compiled.

So we should pass MINIZ_HEADER_FILE_ONLY conditionally (if env.editor_build: # Already implemented by basis encoder.).

[akien-mga]

Thanks!