CMake rework, CIs added, and some ideas

[mattparks]

I don't expect this to be merged, but this PR allows basis universal to be used as a CMake library. I'll make changes to this PR as requested.

Changed:

• Moved main code into src and the tool code into tool
• Renamed the executable basisu to basisuTool
• Shared build enabled, avoided an export solution for now...
• Removed Visual Studio project files, the CMake project can be loaded instead.
• Removed a load of GCC arguments, adding flags though the compiler works better
• Build paths can be set in Visual Studio using a CMakeSettings.json
• Added simple AppVeyor and Travis CI with multiple platforms and architectures, these will need to be setup for the main repo and change the ReadMe badges
• I don't know what headers will be exposed, BASISU_HEADER_FILES can be changed accordingly

Ideas:

• Currently no unit tests, these would be very good for development
• Integrating the current Emscripten CMake file into the main CMake pipeline would be great
• The API is a bit daunting for developers, and no examples
• You are currently missing templates for issues and pull requests
• Some sort of Slack or Gitter channel would be great
• The repo is 150MB+, plenty of old binary files floating around in the history

[richgel999]

Hi - Thanks, I'll check this out! We have unit tests internally, but I need to get them checked into the repo. We'll be adding a C-style API and examples for the compressor/transcoder which should hopefully make the encoder easier to use, as well as API documentation. Also, we'll be moving the encoder to a library, and removing OpenMP support.

-Rich

[oscarbg]

Hi,
sorry for joining the discussion, but why removing OpenMP support? has been threaded in another way? also remember reading on your twitter you were working on ISPC support.. can share status ISPC? has been or will be open sourced too, not ready,etc..

[playmer]

I'm also interested in seeing work done in the CMake on this, and this looks like a great start. A few things I noticed:

• I would change BUILD_TESTS to be BASICU_BUILD_TESTS, just because someone wants basicu's tests off, doesn't mean they want all tests off if they're building via add_subdirectory. (BUILD_TESTS is a common but not standardized Option, for these sorts of things.)

• I would not default BUILD_SHARED_LIBS to ON, I suspect many will want this as a static library. Also I don't see anything being exported, so I'm unsure if it supports being a shared library right now? @richgel999? Apologies if I'm missing something here.

• There's no library just for just the transcoder, so you'd have to link to all of basisu. This seems against the intent of "To use .basis files in an application, you only need the files in the "transcoder" directory. The entire transcoder lives in a single .cpp file". Ideally, we'd expose enough CMake options to only expose the transcoder project, which I think at this point would just be adding a target just for the transcoder, and adding an option that turns off the encoder.

• I would wrap include(CTest) in the if for BUILD_TESTS. No reason to add stuff not being used, it just clutters generated VS solutions.

• I see you're setting some FOLDER properties, but upon running I see they're not functional. This is due to USE_FOLDERS not being set on. This will do it: set_property(GLOBAL PROPERTY USE_FOLDERS ON)

Version related stuff:

• cmake_minimum_required(VERSION 3.0 FATAL_ERROR) is quite low, 3.0 having released in mid 2014, I obviously don't know what the requirements for the customers of basisu are, but I'd recommend at least 3.9, which came out in 2017. I believe most Linux distros are at least up to that in their repos. if not further along as CMake is always adding improvements. I see that it's currently 3.0 in the master, so I suppose @sehurlburt or @richgel999 would have to chime in about it.

  One thing to note about this is we're already using a feature from 3.1, target_compile_features in this branch, and along with it a feature from 3.8, the meta language feature which allows you to specify a standard ( cxx_std_11).

• This isn't doable in 3.0, but if >= 3.1 becomes available, rather than BASISU_SOURCES, I would recommend target_sources

I'm happy to address these and put it in as a PR to @mattparks branch so it can show up here if folks are interested in some/all of these. I've already been playing around locally so it wouldn't be hard to wrap this stuff up.

[mattparks]

Thanks for the suggestions

• BUILD_TESTS is defined in CTests
• I was not entirely sure what should be split in separate libraries, having the transcoder and bacicu as libraries sounds like a good idea
• I was not sure what cmake was being targeted, I'm assuming 3.8 would be fine, maybe 3.11 if they want to use c++17

I'd love to see your changes to my start, I'm by far not the best with CMake, this or was my start of improvements.

[playmer]

• BUILD_TESTS is defined in CTests

Oh, interesting! Apologies then! I looked it up in their docs but didn't see it, must have missed it. It would still be nice to find a way to not hit this pain point though, I'll look further into it!

• I was not sure what cmake was being targeted, I'm assuming 3.8 would be fine, maybe 3.11 if they want to use c++17

I'm fairly sure they have no need for C++17, so for now on this branch I'll assume 3.8.2 and C++11, and when they get back to us I can adjust backwards.

• I was not entirely sure what should be split in separate libraries, having the transcoder and bacicu as libraries sounds like a good idea

Right now I'm playing with putting the libs and exe in src and naming them uni formally (basisu_*). I'll put a PR for you to look at a little later, still looking through the notes implementing stuff. I'll likely need some of your help as well, as I've never made an install target before. Usually just use add_subdirectory, hence me noticing the other thing.

[richgel999]

sorry for joining the discussion, but why removing OpenMP support?

Some engines don't support OpenMP, and I believe clang on OSX doesn't support OpenMP "out of the box" yet. So the plan is to replace it by using a simple C++0x11 thread pool/job system, and lambdas. I've done this on another project before. It should be a simple switch over and it'll give basisu better thread utilization.

I'll check out what I can merge from this PR into master. I'm not really a cmake person, so seeing alternative ways of using it is valuable.

Thanks,
-Rich

[playmer]

For sure, and I'm happy to make a new PR if you have specific things in mind but don't want to muck around too much in CMake, I know it can be annoying. I have a version where it detects OpenMP support right now as well. The main idea is that if you have a CMake, ideally we don't need to also store Visual Studio project files. Plus we can get libraries for the compressor and the transcoder, and a separate executable for the tool.

[jherico]

Given the movement of files in this PR, keeping it up to date with changes in master is going to be challenging long term.

I've done a rebase on to the current master here but it wasn't trivial and I had to do with a number of merge conflicts. I'm doing this because I'd ultimately like to create a PR that makes basisu available as a vcpkg port, and without this cmake re-work such a port would be significantly harder.

@mattparks if you'd like I can create a new PR with my rebase, or you can inspect it if it's helpful in performing your own rebase.

[jherico]

One suggestion I would make is to remove the lodepng source code from encoder folder and put it into a something like a third-party folder, as well as make some minimal attempt to find the library prior to using the included source code version.

Since we don't want to generate either a static or dynamic for lodepng, if finding an existing one on the build system fails, we could incorporate it as an object library, which basically gives you the same behavior as if he source were included directly in the encoder project, but doesn't eliminate the option of using an found copy of the library instead.

Again, this goes back to my work on trying to make this a port for vcpkg, where lodepng already exists as a port and therefore making basisu depend on lodepng would be preferable to directly incorporating lodepng here.

[jherico]

Added a pull request to vcpkg to add basisu here: microsoft/vcpkg#6995

Basis Universal doesn't seem to have a version number scheme, so I've tagged it as 0.0.1 in lieu of something better. Additionally I've made the changes regarding lodepng and I've disable the code that sets the iterator debug level to 1 instead of 2 in the code, since virtually every other package out there uses 2.

[richgel999]

Hi - There's a lot being talked about/changed here, and as I am currently working on the next milestone goals I have to juggle that with trying to follow this. I'm totally open to improving the cmake files and moving towards a vcpkg.

"I've disable the code that sets the iterator debug level to 1 instead of 2 in the code, since virtually every other package out there uses 2."

We set level 1 because level 2 enables full iterator debugging. Unfortunately level 2 is exceptionally slow, and it'll make debugging the library a very frustrating (if not nearly impossible) experience.

[jherico]

I am currently working on the next milestone goals I have to juggle that with trying to follow this. I'm totally open to improving the cmake files and moving towards a vcpkg.

If you have any feedback on another route to get there that might make you more comfortable merging, please let us know.

It should be possible to do all this without re-organizing the files into folders, but that path would likely lead to a single extremely large cmake script that would be much harder to maintain over time.

While this PR could be redone as a series of smaller changes, the file re-organization is inevitably going to still need to be the first step, otherwise it's going to be even harder to get the project into it's final state.

We set level 1 because level 2 enables full iterator debugging. Unfortunately level 2 is exceptionally slow, and it'll make debugging the library a very frustrating (if not nearly impossible) experience.

As a package author that's valuable, but once in the package system, the vast majority of debug builds will be because someone downstream is doing a debug build, rather than debugging iterators in this package. Additionally, since lodepng exists inside vcpkg already, a vcpkg basis build should be using that, not the embedded version. Unless we use level 2 debugging, the lodepng debug build is incompatible and will generate link errors.

I will restore the level 1 iterator debugging in my fork but wrap it in a preprocessor macro so that it depends on the use of cmake arguments.

[richgel999]

This is an old PR, and we weren't able to take it because it changed the build too much. However - there are some good ideas in here, which we will merge next year. I'll leave this PR up as a reference. Thanks again!