Initial CMake build support

[yzsolt]

I've been hacking on CMake build support in the last few weeks. It has got to the point where the library, documentation and all demos build and run properly on my Linux desktop. With the GLES3 backend enabled there are some problems with the demos, but I haven't had the time yet to check if it's a driver issue or something with the build.

There are some subtle issues though. First, this initial version requires CMake 3.10 which isn't released yet. The reason is that only 3.10 will have a proper FindOpenGL.cmake built-in which can be used to check for EGL. This requirement can be lowered using a local FindEGL.cmake, if needed. Until then a locally built CMake can be used for testing.
Second, I couldn't chain the NGL filter to extractor through stdin/stdout as CMake doesn't seem to support that. The current workaround is to redirect one's output into a file and feed that into the other instead. This is incorrect as the redirection probably doesn't work on Windows. So either I'm wrong and the stdin/out chaining can be done with CMake or a separate shell/Python script will be needed later.
Third, given as I don't know the codebase in depth, it's possible that I missed some defines/build options or set them incorrectly, so they should be thoroughly checked.

Please review/test the changes. Only new files were added so the change is non-intrusive, the old and messy Makefile system can be thrown out later in a separate step.

[krogueintel]

Hi!

I greatly appreciate this work as adding a cmake build. Below is some feedback for the current version:

• EGL is needed for the demos even if not using GLES. This is because one can use EGL to create a GL context. Depending on how SDL2 is built, it will use (usually) GLX to create a context -period- even for GLES. There are situations where one wants to create via EGL even when GLX is available. On MS-Windows, the demos do NOT use EGL and the NEGL support library is defaulted to not be built.
• Reading the CMakeList.txt, it looks like only one library is built: FastUIDraw which includes either the GL or GLES backend. The backend libraries must be separate libraries, i.e. there should be libFastUIDraw, libFastUIDrawGL and libFastUIDrawGLES. It should be ok for a user to select which (none, one, or both) of the backends they wish to build.
• I strongly prefer having a set of CMakeList.txt files that include to the lower directories for building the FastUIDraw libraries (just as you have done for the demos)
• The Debug build of FastUIDraw should also include the define FASTUIDRAW_DEBUG; this macro makes FASTUIDRAWassert() non-trivial and also does the magic for detecting memory leaks for objects allocated/freed via FASTUIDRAWnew/FASTUIDRAWdelete.
• I'd like to have change the CMakeList.txt file to detect which cmake version, and if the version is too old to then use FindEGL.cmake instead of using built-in.
• Please append to the readme how to use cmake and what variables control what on building it.
• For static library support, one can make it happen but one needs to add the linker option when build the static libs to "keep all symbols"; I cannot remember the command off hand at this point though.
• Testing under Msys2, the scanning of the GL header failed, atleast out of the box with cmake -G "Unix Makefiles" ..It also generated a header with a junk include "#include <path=>". Looking at the makefile, it appears that cmake failed to figure out and of the GL header files or locations for msys2. For reference, on my system, they are located in /mingw64/include/GL.

Some form of MS-Windows support is required, for now I am happy with just msys2, but I would love to have Visual Studio working too. A big benefit of a cmake build system is that it would then give Visual Studio support for almost free.

Thank you for the work, I look forward to reviewing the pull request again after the above is taken into account.

[yzsolt]

Thanks for the in-depth review! I'll try to clean it up this week, and update the pull request with separate commits to make it easier to review.
As for VS: the biggest problem with Windows support usually is the absence of a package management system. In my cross-platform projects I tend to add dependencies as Git submodules and CMake subprojects. It has the benefit of getting full control over the version and build configuration of the dependencies, and it makes Windows support a lot easier, as basically no library installation/preconfiguration is required for the build. For FastUIDraw this would mean adding Freetype2 and SDL2 as submodules. If this is a no-go for FastUIDraw, I'll look into other ways of getting painless Windows support.

[krogueintel]

Did you still want to pursue cmake building for FastUIDraw?

[yzsolt]

Yes! I'm sorry about the huge delay. I've done most of the requested changes in my local branch, but it needs some cleanup before I can rebase and update the pull request. There's a long weekend here in Hungary, so I'm looking forward to send the changes in the next few days.

[krogueintel]

Excellent! I look forward to the pull request and if all requests I made are in the new version, we should see it pulled into master.

[krogueintel]

One more .. thing. There is a new pair of libraries in FastUIDraw: libNEGL_debug.so and libNEGL_release.so. There serve the same function as libGL_debug/release.so but for EGL. For MS-Windows, they are not used or built, but for Linux they are.

[yzsolt]

So I did the GL/GLES backend separation, but before continuing with refactoring the demos, I have a question: can we let go the simultaneous backend build (both GL and GLES at the same time)?
I'm asking this because building two versions of each demo for the two backends greatly complicates the build scripts - basically each demo's add_executable() should be duplicated, and lots of ifs should be introduced. It doesn't look like a scalable solution, and it's neither really cmake-ish. I tried to create a DEMO_BACKEND variable and adding the demos subdirectory twice with DEMO_BACKEND set to GL and GLES respectively, but this workaround doesn't work, since add_subdirectory() can't be called with the same path twice.
If letting this go is not a viable option, at least we could introduce a configuration option for the demos which sets the backend for them to use. Since demos are not installed, this seems a viable alternative to me. What do you think?

[krogueintel]

Isn't it worse because of the _release and _debug issue which actually makes it four versions? Currently, if one builds an application with FastUIDraw, one's debug builds links against a different .so than a release build. Did you already have the debug and release pain sorted out for the library building?

For what it is worth, it wiggs me out some that there is a different .so for debug and release, but their is a reason: the debug version has a number of FASTUIDRAWassert's in the header for reference counted pointer and the release build def's those out. If I remember correctly, the linker rules for inlined functions in a header stipulate that all stuff getting linked together has to have the exact same implementation for such inlined functions.

[yzsolt]

Release and debug isn't a big issue, because they're built separately, depending on the value of CMAKE_BUILD_TYPE. For them it's just an if() somewhere for the name of the library. But the backends can be built simultaneously currently, which means they must be added somehow as separate targets, which complicates things.

However, since you've mentioned it, I think the _debug/_release suffix could be dropped too - I mean, judging by the open source libraries I know, it's not common to note the type of the build in the library name. Some go with appending a "d" to the debug builds, but it's the minority. With CMake, if someone wants to use fastuidraw as a dependency (and assuming they're also using CMake), they just add it with add_subdirectory(), and it will get all the configuration parameters of the parent project, including the build type (debug or release). So there's no way to mess it up, and probably that's why the library name is not really interesting anymore: with CMake, you don't have to set these things manually, everything is figured out by the build system.

[krogueintel]

The _release and _debug suffixes can NOT be dropped; the issue is that under release, FASTUIDRAWassert degenerates into nothing and under debug it is assert-like; the crux is that some headers have FASTUIDRAWassert in them and linking pain rules generally say don't have headers that define templates compile different ways; so the _release and _debug should remain on the libraries,

I think the simple way out is to view GL vs GLES as like debug vs release. It is not ideal, but in all honesty, most users just want GL or GLES but rarely both (indeed some non-Mesa drivers on Linix desktop that support GLES acts a littly wonky too).

[yzsolt]

Short status report: I've figured out GL/GLES and debug/release naming issues, also added separate targets for NEGL, NGL and NGLES, which were missing from the initial version.
The three remaining things are figuring out installation, extending the README with CMake build instructions and checking the build with MSYS2.

[yzsolt]

I'd like to ask if this gets merged, do you intend to remove the current Makefile-based build support? In other words, should I extend the README with the CMake build instructions, or replace the existing build instructions completely?

[krogueintel]

For now extend the README; I don't want to commit to deleting the current GNU-make system until the cmake one has all the features of the GNU-make and is battle tested by users thoroughly.

[zsx]

I just came across this PR after I sent my fixes for MSVC compilation. It's very interesting because CMake happends to be what I used to compile with MSVC, but mine is not as complete, and tailored for building with MSVC only. But I would like to see this merged, so I can drop mine.

[krogueintel]

I would like to merge it too, but I am awaiting an update on the issues I have for it; the latest version of this is from about 6 months ago and files have been added since then along with changes, so it won't work out-of-the-box yet either. I'd rather you did NOT drop yours; the most horribly ugly thing to handle is how to generate ngl_gl.hpp/ngl_gl.cpp given that the generation requires flex.