Fix handling of pthread naming API for LLVM on Windows

[akien-mga]

The previous two-step method relying on std:thread:native_handle() is tricky as it's implementation-defined, and it seems that LLVM's libc++ does not return a pthread_t handle even when using pthread (and the MSVC code path is not an option for LLVM on Windows).

With mingw-gcc as packaged in Linux distros with pthread support it worked fine, but with llvm-mingw it was problematic. It seems like it could also be problematic with clang-cl.

Setting the name in the thread directly as done for Apple platforms is simpler and should work fine.

Co-authored-by: @hpvb

[wolfpld]

This code is a bit obsolete. I now keep the most recent version in https://github.com/wolfpld/tracy/blob/master/common/TracySystem.cpp. I think it would be best to just use it instead, given that Tracy is already a dependency for etcpak. It also brings some order into how things work, i.e. setting thread name is only possible from within the thread itself.

[akien-mga]

The tracy code for this looks a lot more cross-platform indeed, I agree that it seems to be a good replacement.

I'm a bit worried about using Tracy as a dependency directly, it seems like nothing in etcpak actually relies on Tracy currently. The code we're vendoring for Godot is only the "library" bits (not Application.cpp, nor Tracy) and this seems to work well for our use case (we like to keep our library dependencies as small as possible): https://github.com/godotengine/godot/tree/master/thirdparty/etcpak

If you plan to add a tighter integration between etcpak and Tracy in the future, maybe that can be guarded behind a compiler define? For Godot's needs we mostly care about having a fast and reliable VRAM compression library, but not so much inspecting performance of the tool itself beyond "it's fast!". On the other hand for etcpak development I fully understand that Tracy can be extremely useful.

That's maybe more something to consider for the long run, but there could maybe be a clearer separation between the etcpak library, ready to use by downstream applications, the etcpak CLI application which uses it, and the development tooling that lets you and other contributors do efficient research and optimization for fast and reliable compression.

Of course that depends on how big the interest is in using the library itself. We're just getting started with it in Godot and things look good, but it hasn't passed the test of time and a stable release yet :)

[wolfpld]

That's maybe more something to consider for the long run, but there could maybe be a clearer separation between the etcpak library, ready to use by downstream applications, the etcpak CLI application which uses it, and the development tooling that lets you and other contributors do efficient research and optimization for fast and reliable compression.

Typically I'd expect people to just extract the Compress* functions into their own code, as these functions do all the heavy magic here, i.e. they accept an image buffer with a number of blocks and output the compressed data. Proper threading can be then left to the user, if needed, as it involves just a trivial calculation of how many blocks should be processed per each job. This may as well be just what you need.

The rest of code may be a bit hairy, as it has to juggle loading images, generating mips, and at the same time it has to perform the compression. The data is also directly streamed to disk, as it's measurably faster than using intermediate buffers. So, things are a bit involved here, and I don't really see much sense in extracting these functionalities to be easier to use externally.

Let me know how this feels for you.

[akien-mga]

Thanks, that's very helpful advice. Our actual code using the library was implemented by @fire and I hadn't done a deep dive into the API, but I had a look now and could confirm that we only need the Compress* functions, and thus the Process*.cpp files and their direct dependencies.

That enabled me to slim down the vendored code significantly: https://github.com/godotengine/godot/pull/47890 And that solves any portability issue we may have with the rest of the code of the etcpak application.

So I think this is good as is from our point of view. :+1:

My advice for potential other users of the compression code could be to split it better as a self-contained sublibrary, so that it's easier to identify (and guarantee) that it can be used standalone. E.g. a structure like this:

compress/
  Dither.*
  ProcessCommon.hpp
  ProcessDxtc.*
  ProcessRGB.*
common/
  ForceInline.hpp
  Math.hpp
  Tables.*
  Vector.hpp
app/
  Application.*
  Bitmap.*
  BlockData.*
  etc.

So it could be clearer to downstream users that to use the compression API they'd just need compress/ and common/.

[Anutrix]

What's the status on this?