Skip to content

Conversation

@MarkCallow
Copy link
Contributor

@MarkCallow MarkCallow commented Nov 5, 2025

With these changes KTX-Software now incorporates basis_universal as a CMake subproject, building it using this fork which is the source of the changes in this PR.

Via KTX-Software's CI, basis_universal with these fixes is known to build on Android (gcc), Emscripten (Clang 20), iOS (AppleClang 17), Linux(Ubuntu 22.04/gcc13 and 24.04/gcc15, x86_64 and arm64), minGW (gcc 15), macOS (AppleClang 17, x86_64 & arm64) and Windows (MSVC CL and CLangCL via VisualStudio 2022, x86_64 and arm64). Via manual check, it is also known to build with gcc15 for macOS. KTX-Software's test suite, including various encode and transcode tests, passes on all the platforms it is run on, that is all the above except Android and iOS.

KTX-Software's CI tests all combinations of the SSE and OPENCL options on the 3 major platforms.

You can check the results in the PR for adding the updated encoder to KTX-Software - KhronosGroup/KTX-Software#1078.

There are 3 distinct sets of changes:

  • updates to the CMake project;
  • fixes or additional ignores for compiler warnings.
  • fixes to pass reuse lint

CMake Project Updates

  • Make it usable as a subproject:

    • Don't use CMAKE_ global variables for compiler options.
    • Prefix config options with BASISU_ to avoid polluting the namespace.
    • Expose libbasisu_encoder's dependencies and link options in its target interface so they are automatically exported to any application that links with it.
  • Enhancements

    • Add header files to source files list so they are visible in project.
    • Support building on Android and minGW by handling potential absence of libpthread.
    • Support building with CLangCL.
  • Bug Fixes

    • Make work with any multi-config CMake generator, not just Visual Studio.
    • Only set c++ compile options when compiling c++ files.
    • The STATIC option, renamed BASISU_STATIC, is now set to TRUE by default because the library is always built
      as a static library. STATIC is a keyword for the add_library command so the STATIC option was ignored there. (The BASISU_ prefix now makes it clear they are different.) Hence the previous default option of FALSE was incorrect.
    • The libraries added when STATIC is TRUE are only needed for MinGW and are now only added for MinGW.
    • The rpath setting done when STATIC is FALSE is pointless with a static library but was happening due to the incorrect value of STATIC.
    • .gitignore files have been added to ignore build files and binaries in build and bin, but to not ignore the handful of files in there that are tracked by git.
  • Bugs found but not fixed

    • BASISU_SSE is set if (MSVC) so it is incorrectly set on Windows ARM and not set in many other cases where it probably should be.
    • Building a dynamic library never happens so BASISU_STATIC is pointless.

Warning fixes

Many compile warnings in the code have been fixed. Two I particularly want to note are:

  • tinyexr.h appears to have had the arguments to a call to calloc swapped. The original code is shown below. The first argument is supposed to be the count of items to allocate space for and the second the size of those items.
    exr_image->tiles = static_cast<EXRTile*>(
    calloc(sizeof(EXRTile), static_cast<size_t>(num_tiles)));
  • There are many instances in the code ignoring a GCC warning in the way shown below.
    #ifndef __EMSCRIPTEN__
    #ifdef __GNUC__
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wclass-memaccess"
    #endif
    #endif
    This generates a warning when compiling with any Clang, not just Emscripten. I have changed all occurrences to only set this pragma for real GCC by using #if defined(__GNUC__) && !defined(__clang__).

reuse lint Fixes

  • Corrected spelling of BSD-3-Clause.txt license text file.
  • Removed unused Zlib license text file.
  • Updated from deprecated dep5 to REUSE.toml.

* Prefix option names with `BASISU_`.
* Add include files to source list for library project.
* Fix build of basisu with multi-config generators.
The change at line 13 to use a genex accomplishes the same thing.
using INTERFACE to export OpenCL lib and includes from basisu_encoder
to the applications.
Fix OpenCL configuration issues.
basis_universal CMakeLists sets compiler options that disable these
warnings when they are included there.

Two warnings:

* The pragma to ignore -Wmem-access is ifdef'ed for __GNUC__ but clang
  also defines __GNUC__ and does not have a mem-access warnings. Add
  !defined(__clang__)to fix.
* Comment out an unused typedef in basisu_enc.h.
* move deployment target setting to where it will work;
* move ALLOW_MEMORY_GROWTH to link options;
* set c++ only options only for c++;
* move include directories to BUILD_INTERFACE.
* swapped parameters to calloc in tinyexr.h;
* unneeded unnsigned < 0 comparisons.
Rendered unnecessary by addition of !defined(clang) to subsequent #if
to support wider clang family.
Note that setting BASISU_SSE TRUE for MSVC needs fixing. It
breaks the build on Windows Arm64.
@richgel999
Copy link
Contributor

Thanks! Once our new build settles I'll merge this in.

richgel999 added a commit that referenced this pull request Nov 6, 2025
PR 408's cmakelists.txt file has too many issues (OpenCL breaks on Windows/OSX) and changing our options to have the "BASISU_" prefix will break upstream builds.
@richgel999
Copy link
Contributor

richgel999 commented Nov 6, 2025

I've merged all code warning fixes from this PR. I added the header files to our CMakeLists.txt file. Unfortunately, OpenCL building under Windows is broken in this PR, and OpenCL is not working correctly on OSX, so I can't merge the CMakeLists changes.
Also, if we add the BASISU_ prefix to the CMakeLists options, we break upstream builds. I may merge this change in for our January 2026 release, however.

@Pandapip1
Copy link

It seems this is missing installation rules:

       > buildPhase completed in 1 minutes 16 seconds
       > Running phase: installPhase
       > install flags: -j8 SHELL=/nix/store/ciarnmsx8lvsrmdbjddpmx0pqjrm8imb-bash-5.3p3/bin/bash install
       > make: *** No rule to make target 'install'.  Stop.

If you could rebase your branch on master, I'll rebase my own branch on top of this and pull out the installation rules I made. Alternatively, just pick and choose whatever you want from Pandapip1@e2f294d. Thank you so much for this work!

@MarkCallow
Copy link
Contributor Author

Unfortunately, OpenCL building under Windows is broken in this PR, and OpenCL is not working correctly on OSX, so I can't merge the CMakeLists changes.

It is building fine under windows with OpenCL in the KTX-Software CI. Does the Windows build in your CI use OpenCL? It too is green. What errors are you seeing? I want to fix the problem.

How are you determining that OpenCL is not working correctly on macOS. Please send me a link to the test(s) that show the problem.

Also, if we add the BASISU_ prefix to the CMakeLists options, we break upstream builds. I may merge this change in for our January 2026 release, however.

There is no way to avoid that if you want to avoid polluting the global variable namespace. Please merge the change in your January 2026 release.

@MarkCallow
Copy link
Contributor Author

It seems this is missing installation rules:

There were no installation rules last time I pulled from upstream. I will merge the latest changes from master as soon as I have time.

@MarkCallow
Copy link
Contributor Author

Replacing this with PR #410 with the reuse and .gitignore fixes and PR #411 with just the CMake related fixes. The former is based on the latest master. The latter is based on the former.

@MarkCallow MarkCallow closed this Nov 17, 2025
@MarkCallow MarkCallow deleted the combined_cmake_warnings branch November 18, 2025 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants