Basis{ImageConverter,Importer}: add support for basis_universal 1.16

Fully backwards compatible thanks to the new BASISU_LIB_VERSION/BASISD_LIB_VERSION. The OpenCL dependency is kind of brutal since it hard-links to the shared library. Is there a nicer way to configure this other than CMAKE_DISABLE_FIND_PACKAGE_OpenCL?
mosra · Feb 4, 2022 · 0daabe1 · 0daabe1
1 parent fa77ca4
commit 0daabe1
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 19 deletions.
diff --git a/modules/FindBasisUniversal.cmake b/modules/FindBasisUniversal.cmake
@@ -161,30 +161,60 @@ foreach(_component ${BasisUniversal_FIND_COMPONENTS})
                 # Alternatively, look into creating stubs for the library
                 # functions used by basis_universal.
                 set(BasisUniversalEncoder_SOURCES
-                    ${BasisUniversalEncoder_DIR}/apg_bmp.c
-                    ${BasisUniversalEncoder_DIR}/basisu_astc_decomp.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_backend.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_basis_file.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_bc7enc.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_comp.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_enc.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_etc.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_frontend.cpp
-                    ${BasisUniversalEncoder_DIR}/basisu_global_selector_palette_helpers.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_gpu_texture.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_kernels_sse.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_pvrtc1_4.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_resampler.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_resample_filters.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_ssim.cpp
                     ${BasisUniversalEncoder_DIR}/basisu_uastc_enc.cpp
-                    ${BasisUniversalEncoder_DIR}/jpgd.cpp
-                    ${BasisUniversalEncoder_DIR}/lodepng.cpp)
+                    ${BasisUniversalEncoder_DIR}/jpgd.cpp)
+
+                # Files not present in all supported basis versions, treat them
+                # as optional and do nothing if not found.
+                foreach(_file
+                    # Removed in 1.16
+                    # https://github.com/BinomialLLC/basis_universal/commit/deeb5acb563246f9b747229636205c5b19b99839
+                    apg_bmp.c
+                    basisu_astc_decomp.cpp
+                    basisu_global_selector_palette_helpers.cpp
+                    lodepng.cpp
+                    # Added in 1.16
+                    basisu_opencl.cpp
+                    pvpngreader.cpp)
+                    # Disable the find root path here, it overrides the
+                    # CMAKE_FIND_ROOT_PATH_MODE_INCLUDE setting potentially set in
+                    # toolchains.
+                    find_file(BasisUniversalEncoder_${_file}_SOURCE NAMES ${_file}
+                        HINTS ${BasisUniversalEncoder_DIR} NO_CMAKE_FIND_ROOT_PATH)
+
+                    if(BasisUniversalEncoder_${_file}_SOURCE)
+                        list(APPEND BasisUniversalEncoder_SOURCES
+                            ${BasisUniversalEncoder_${_file}_SOURCE})
+                    endif()
+                endforeach()
 
                 foreach(_file ${BasisUniversalEncoder_SOURCES})
                     _basis_setup_source_file(${_file})
                 endforeach()
 
+                set(BasisUniversalEncoder_DEFINITIONS "BASISU_NO_ITERATOR_DEBUG_LEVEL")
+
+                # Try to find an external OpenCL library and enable support for
+                # it in basis if found.
+                find_package(OpenCL QUIET)
+                if(OpenCL_FOUND)
+                    list(APPEND BasisUniversalEncoder_DEFINITIONS
+                        "BASISU_SUPPORT_OPENCL=1")
+                endif()
+
                 # Disable the find root path here, it overrides the
                 # CMAKE_FIND_ROOT_PATH_MODE_INCLUDE setting potentially set in
                 # toolchains.
@@ -196,8 +226,16 @@ foreach(_component ${BasisUniversal_FIND_COMPONENTS})
                 add_library(BasisUniversal::Encoder INTERFACE IMPORTED)
                 set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
                     INTERFACE_INCLUDE_DIRECTORIES ${BasisUniversalEncoder_INCLUDE_DIR})
+                set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
+                    INTERFACE_COMPILE_DEFINITIONS ${BasisUniversalEncoder_DEFINITIONS})
                 set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
                     INTERFACE_SOURCES "${BasisUniversalEncoder_SOURCES}")
+                if(OpenCL_FOUND)
+                    set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
+                        INTERFACE_INCLUDE_DIRECTORIES ${OpenCL_INCLUDE_DIRS})
+                    set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES ${OpenCL_LIBRARIES})
+                endif()
                 # Explicitly *not* linking this to Threads::Threads because
                 # when done like that, std::thread creation will die on a null
                 # function pointer call (inside __gthread_create, which weakly
@@ -214,8 +252,6 @@ foreach(_component ${BasisUniversal_FIND_COMPONENTS})
                 # itself.
                 set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
                     INTERFACE_LINK_LIBRARIES BasisUniversal::Transcoder)
-                set_property(TARGET BasisUniversal::Encoder APPEND PROPERTY
-                    INTERFACE_COMPILE_DEFINITIONS "BASISU_NO_ITERATOR_DEBUG_LEVEL")
             endif()
         else()
             set(BasisUniversal_Encoder_FOUND TRUE)

diff --git a/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.conf b/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.conf
@@ -77,6 +77,10 @@ rdo_uastc_favor_simpler_modes_in_rdo_mode=true
 ktx2_uastc_supercompression=true
 ktx2_zstd_supercompression_level=6
 
+# OpenCL acceleration. Falls back to CPU encoding if OpenCL isn't supported or
+# fails during encoding.
+use_opencl=false
+
 # Set various fields in the Basis file header
 userdata0=0
 userdata1=0

diff --git a/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.cpp b/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.cpp
@@ -45,6 +45,9 @@
 #include <basisu_enc.h>
 #include <basisu_comp.h>
 #include <basisu_file_headers.h>
+#if BASISU_LIB_VERSION >= 116
+#include <basisu_opencl.h>
+#endif
 
 namespace Magnum { namespace Trade {
 
@@ -122,7 +125,11 @@ template<UnsignedInt dimensions> Containers::Array<char> convertLevelsToData(Con
     PARAM_CONFIG(quality_level, int);
     PARAM_CONFIG(perceptual, bool);
     PARAM_CONFIG(debug, bool);
+    #if BASISU_LIB_VERSION >= 116
+    PARAM_CONFIG_FIX_NAME(validate_etc1s, bool, "validate");
+    #else
     PARAM_CONFIG(validate, bool);
+    #endif
     PARAM_CONFIG(debug_images, bool);
     PARAM_CONFIG(compute_stats, bool);
     PARAM_CONFIG(compression_level, int);
@@ -225,6 +232,16 @@ template<UnsignedInt dimensions> Containers::Array<char> convertLevelsToData(Con
     keyValue.m_key.append(reinterpret_cast<const uint8_t*>(OrientationKey), sizeof(OrientationKey));
     keyValue.m_key.append(reinterpret_cast<const uint8_t*>(orientationValue), sizeof(orientationValue));
 
+    /* OpenCL */
+    #if BASISU_LIB_VERSION >= 116
+    PARAM_CONFIG(use_opencl, bool);
+
+    if(params.m_use_opencl && !basisu::opencl_is_available()) {
+        Warning{} << "Trade::BasisImageConverter::convertToData(): OpenCL not supported, falling back to CPU encoding";
+        params.m_use_opencl = false;
+    }
+    #endif
+
     /* Set various fields in the Basis file header */
     PARAM_CONFIG(userdata0, int);
     PARAM_CONFIG(userdata1, int);
@@ -341,6 +358,9 @@ template<UnsignedInt dimensions> Containers::Array<char> convertLevelsToData(Con
             /* process() will have printed additional error information to stderr */
             Error{} << "Trade::BasisImageConverter::convertToData(): frontend processing failed";
             return {};
+        case basisu::basis_compressor::error_code::cECFailedFontendExtract:
+            Error{} << "Trade::BasisImageConverter::convertToData(): frontend extraction failed";
+            return {};
         case basisu::basis_compressor::error_code::cECFailedBackend:
             Error{} << "Trade::BasisImageConverter::convertToData(): encoding failed";
             return {};
@@ -356,15 +376,27 @@ template<UnsignedInt dimensions> Containers::Array<char> convertLevelsToData(Con
             return {};
 
         /* LCOV_EXCL_START */
-        case basisu::basis_compressor::error_code::cECFailedFontendExtract:
-            /* This error will actually never be raised from basis_universal code */
+        #if BASISU_LIB_VERSION >= 116
+        case basisu::basis_compressor::error_code::cECFailedInitializing:
+            /* This error is returned by the parallel compression API if
+               basis_compressor::init() returns false */
+        #endif
         case basisu::basis_compressor::error_code::cECFailedWritingOutput:
             /* We do not write any files, just data */
         default:
             CORRADE_INTERNAL_ASSERT_UNREACHABLE();
         /* LCOV_EXCL_STOP */
     }
 
+    #if BASISU_LIB_VERSION >= 116
+    /* If OpenCL fails at any stage of encoding, Basis falls back to encoding
+       on the CPU. It also spams stderr but printing to Warning is still useful
+       for anyone parsing or redirecting it. */
+    if(params.m_use_opencl && basis.get_opencl_failed()) {
+        Warning{} << "Trade::BasisImageConverter::convertToData(): OpenCL encoding failed, fell back to CPU encoding";
+    }
+    #endif
+
     const basisu::uint8_vec& out = params.m_create_ktx2_file ? basis.get_output_ktx2_file() : basis.get_output_basis_file();
 
     Containers::Array<char> fileData{NoInit, out.size()};
@@ -389,7 +421,17 @@ template<UnsignedInt dimensions> Containers::Array<char> convertLevelsToData(Con
 }
 
 void BasisImageConverter::initialize() {
+    #if BASISU_LIB_VERSION >= 116
+    basisu::basisu_encoder_init(true);
+    #else
     basisu::basisu_encoder_init();
+    #endif
+}
+
+void BasisImageConverter::finalize() {
+    #if BASISU_LIB_VERSION >= 116
+    basisu::basisu_encoder_deinit();
+    #endif
 }
 
 BasisImageConverter::BasisImageConverter(Format format): _format{format} {

diff --git a/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.h b/src/MagnumPlugins/BasisImageConverter/BasisImageConverter.h
@@ -238,6 +238,16 @@ class MAGNUM_BASISIMAGECONVERTER_EXPORT BasisImageConverter: public AbstractImag
          */
         static void initialize();
 
+        /**
+         * @brief Finalize Basis encoder
+         * @m_since_latest
+         *
+         * If the class is instantiated directly (not through a plugin
+         * manager), this function has to be called explicitly after
+         * destroying the last instance.
+         */
+        static void finalize();
+
         /**
          * @brief Default constructor
          *

diff --git a/src/MagnumPlugins/BasisImageConverter/Test/BasisImageConverterTest.cpp b/src/MagnumPlugins/BasisImageConverter/Test/BasisImageConverterTest.cpp
@@ -806,7 +806,7 @@ void BasisImageConverterTest::convert2DMipmaps() {
     CORRADE_COMPARE_WITH(*levels[1].result,
         Utility::Directory::join(BASISIMPORTER_TEST_DIR, "rgba-31x13.png"),
         /* There are moderately significant compression artifacts */
-        (DebugTools::CompareImageToFile{_manager, 81.0f, 14.33f}));
+        (DebugTools::CompareImageToFile{_manager, 81.0f, 14.651f}));
     CORRADE_COMPARE_WITH(*levels[2].result,
         Utility::Directory::join(BASISIMPORTER_TEST_DIR, "rgba-15x6.png"),
         /* There are moderately significant compression artifacts */
@@ -869,10 +869,10 @@ void BasisImageConverterTest::convert2DArray() {
         (DebugTools::CompareImage{97.25f, 7.89f}));
     CORRADE_COMPARE_WITH(image->pixels<Color4ub>()[1], imageViewSlice(ImageView3D(originalImage), 1),
         /* There are moderately significant compression artifacts */
-        (DebugTools::CompareImage{97.25f, 7.735f}));
+        (DebugTools::CompareImage{97.25f, 7.778f}));
     CORRADE_COMPARE_WITH(image->pixels<Color4ub>()[2], imageViewSlice(ImageView3D(originalImage), 2),
         /* There are moderately significant compression artifacts */
-        (DebugTools::CompareImage{96.5f, 6.928f}));
+        (DebugTools::CompareImage{98.5f, 7.022f}));
 }
 
 void BasisImageConverterTest::convert2DArrayOneLayer() {
@@ -986,14 +986,14 @@ void BasisImageConverterTest::convert2DArrayMipmaps() {
         CORRADE_COMPARE_WITH(levels[0].result->pixels<Color4ub>()[i],
             imageViewSlice(ImageView3D(*levels[0].originalImage), i),
             /* There are moderately significant compression artifacts */
-            (DebugTools::CompareImage{97.25f, 7.914f}));
+            (DebugTools::CompareImage{98.5f, 7.914f}));
     }
     for(Int i = 0; i != levels[0].originalImage->size().z(); ++i) {
         CORRADE_ITERATION("level 1, layer" << i);
         CORRADE_COMPARE_WITH(levels[1].result->pixels<Color4ub>()[i],
             imageViewSlice(ImageView3D(*levels[1].originalImage), i),
             /* There are moderately significant compression artifacts */
-            (DebugTools::CompareImage{87.0f, 14.453f}));
+            (DebugTools::CompareImage{87.0f, 14.73f}));
     }
     for(Int i = 0; i != levels[0].originalImage->size().z(); ++i) {
         CORRADE_ITERATION("level 2, layer" << i);
@@ -1049,7 +1049,7 @@ void BasisImageConverterTest::convertToFile2D() {
     CORRADE_COMPARE_WITH(*level1,
         Utility::Directory::join(BASISIMPORTER_TEST_DIR, "rgba-31x13.png"),
         /* There are moderately significant compression artifacts */
-        (DebugTools::CompareImageToFile{_manager, 81.0f, 14.31f}));
+        (DebugTools::CompareImageToFile{_manager, 81.0f, 14.709f}));
 
     /* The format should get reset again after so convertToData() isn't left
        with some random format after */

diff --git a/src/MagnumPlugins/BasisImporter/BasisImporter.cpp b/src/MagnumPlugins/BasisImporter/BasisImporter.cpp
@@ -130,8 +130,11 @@ template<> struct ConfigurationValue<Magnum::Trade::BasisImporter::TargetFormat>
 namespace Magnum { namespace Trade {
 
 struct BasisImporter::State {
+    /* Basis 1.16 got rid of global selector palettes */
+    #if BASISD_LIB_VERSION < 116
     /* There is only this type of codebook */
     basist::etc1_global_selector_codebook codebook;
+    #endif
 
     /* One transcoder for each supported file type, and of course they have
        wildly different interfaces. ktx2_transcoder is only defined if
@@ -161,8 +164,11 @@ struct BasisImporter::State {
     bool noTranscodeFormatWarningPrinted = false;
     UnsignedInt lastTranscodedImageId = ~0u;
 
-    explicit State(): codebook(basist::g_global_selector_cb_size,
-        basist::g_global_selector_cb) {}
+    explicit State()
+    #if BASISD_LIB_VERSION < 116
+        : codebook(basist::g_global_selector_cb_size, basist::g_global_selector_cb)
+    #endif
+    {}
 };
 
 void BasisImporter::initialize() {
@@ -253,7 +259,11 @@ void BasisImporter::doOpenData(Containers::Array<char>&& data, DataFlags dataFla
 
     #if BASISD_SUPPORT_KTX2
     if(isKTX2) {
-        state->ktx2Transcoder.emplace(&state->codebook);
+        state->ktx2Transcoder.emplace(
+            #if BASISD_LIB_VERSION < 116
+            &state->codebook
+            #endif
+        );
 
         /* init() handles all the validation checks, there's no extra function
            for that */
@@ -316,7 +326,11 @@ void BasisImporter::doOpenData(Containers::Array<char>&& data, DataFlags dataFla
     #endif
     {
         /* .basis file */
-        state->basisTranscoder.emplace(&state->codebook);
+        state->basisTranscoder.emplace(
+            #if BASISD_LIB_VERSION < 116
+            &state->codebook
+            #endif
+        );
 
         if(!state->basisTranscoder->validate_header(state->in.data(), state->in.size())) {
             Error{} << "Trade::BasisImporter::openData(): invalid basis header";