misc cmake update

[cathaysia]

this pr makes some changes:

• add RMLUI_FREETYPE_EXTERNAL option
• add RMLUI_RLOTTIE_EXTERNAL option
• enable CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS and CMAKE_POSITION_INDEPENDENT_CODE

RMLUI_FREETYPE_EXTERNAL makes rmlui use existed freetype target. such as:

add_subdirectory(freetype)
add_subdirectory(rlottie)
set(RMLUI_FREETYPE_EXTERNAL ON CACHE BOOL "use existed freetype target")
set(RMLUI_RLOTTIE_EXTERNALON ON CACHE BOOL "use existed rlottie target")
add_subdirectory(RmlUi)

the reason use freetype_interface instead of freetype is freetype has not set -fpie, so it make a link error.

[cathaysia]

Hi, friend. Don't say more thanks. we all hope this project be better. It's you give the project life!

[mikke89]

Hah, well, I do appreciate the contributions!

Do you intend to submit more changes to the CMake file? If so, it would be nice if you could submit them together in here as a single pull request.

[cathaysia]

ok, I will do this later.

…

________________________________ From: Michael R. P. Ragazzon ***@***.***> Sent: Saturday, December 24, 2022 8:30:45 AM To: mikke89/RmlUi ***@***.***> Cc: DragonBillow ***@***.***>; Author ***@***.***> Subject: Re: [mikke89/RmlUi] misc cmake update (PR #397) Hah, well, I do appreciate the contributions! Do you intend to submit more changes to the CMake file? If so, it would be nice if you could submit them together in here as a single pull request. ― Reply to this email directly, view it on GitHub<#397 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKBJ6ALGREYTKDIDQ3FLUP3WOY73LANCNFSM6AAAAAATFGWCSE>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[cathaysia]

lua not support cmake, did you can accept make it as a submodule? just like this: https://github.com/luvit/luv/tree/master/deps

[mikke89]

I would prefer not to include dependencies as submodules. Instead though, it would be nice to allow users to optionally fetch them using FetchContent as discussed in #198. I would want this functionality to be behind an explicit opt-in option though.

[cathaysia]

sorry replay too late.

maybe we can provide a option, which provide lua51 as default lua, just like this:

option (USE_BUND_LUA OFF)
if(${USE_BUND_LUA}
    add_subdirectory(extern/lua)
endif()

can you accept this?

[mikke89]

Hm, how would this work from the user perspective?

Do you mean we should bundle the Lua source code inside our repository?

[cathaysia]

I want add lua as a submodule, but just use it when USE_BUND_LUA is true.

[mikke89]

As mentioned previously, I would prefer not to include dependencies as submodules.

[cathaysia]

ok, can you accept that use RMLUI_LUA_EXTERNAL option? for this option, just use lua::lua as target name, then user can define any lua version as this alias.

I can do a commit later, but it need sone time.

[mikke89]

Yeah, that sounds reasonable to me. Preferably, mark all the *_EXTERNAL options as advanced.

[cathaysia]

That's should be OK now. I add .editorconfig file, because I use nvim edit file, it default use space but not tab, so it's troublesome for me to edit CMakeLists.txt.

Then I add RMLUI_LUA_EXTERNAL, when this flag be true, rmlui will try link RmlUi::Lua.

addition: if target_link_libraries RmlUi::Lua, then RmlUi::Lua's include directories will be included automaticly.

[mikke89]

Hey, I have some comments here. Generally I'm a bit confused about some of the external library names.

Here is an alternative idea: Instead of adding the new options, see if e.g. ${LUA_INCLUDE_DIR} and ${LUA_LIBRARIES} are already set, and if so, use them directly and skip the find_package. Similarly for rlottie and freetype. What do you think about this approach?

@hobyst Perhaps you have some thoughts on this PR or my comments here?

[hobyst]

Hey, I have some comments here. Generally I'm a bit confused about some of the external library names.

Here is an alternative idea: Instead of adding the new options, see if e.g. ${LUA_INCLUDE_DIR} and ${LUA_LIBRARIES} are already set, and if so, use them directly and skip the find_package. Similarly for rlottie and freetype. What do you think about this approach?

@hobyst Perhaps you have some thoughts on this PR or my comments here?

The proper way to define paths to dependencies is, although not often noticed, to use the CMake variable CMAKE_PREFIX_PATH and/or environment variable CMAKE_PREFIX_PATH. Albeit some exceptions like hint paths used by some find modules, this is the standard way of specifying which paths should be looked for when find functions are called.

CMake prefers CMAKE_PREFIX_PATH over CMAKE_SYSTEM_PREFIX_PATH, meaning if the consumer defines CMAKE_PREFIX_PATH to specify where the external dependencies they want to use are, find_package() and alike functions will prefer those over the packages provided by the system without any more interaction from the consumer being required. Although it must be noted that some find modules might not follow this, in whose case they will go their way using their own hint paths (Freetype and its FREETYPE_DIR hint variable might be an example of such exception).

For rlottie, it might be more complex to go for this approach as it doesn't get distributed as a pre-compiled package but not impossible to make a proper integration since it supports CMake as a build system and a CMake package config file could be generated when building it so that it can be consumed as a pre-compiled dependency by other CMake projects. Adding it as a submodule could ease the integration as the build script could default to do a source build from the included rlottie via add_subdirectory() in case the consumer didn't provide a pre-compiled package without the consumer having to do it by themselves. That way, they would only have to go through the hassle of manually build it themselves if they actually want to.

In the event that rlottie can't be integrated using its CMake build option, then either a find module would have to be written for it or investigate the use of Meson's CMake module in their mesonfile so that it generates the CMake config module for us.

[cathaysia]

Hey, I have some comments here. Generally I'm a bit confused about some of the external library names.

Here is an alternative idea: Instead of adding the new options, see if e.g. ${LUA_INCLUDE_DIR} and ${LUA_LIBRARIES} are already set, and if so, use them directly and skip the find_package. Similarly for rlottie and freetype. What do you think about this approach?

@hobyst Perhaps you have some thoughts on this PR or my comments here?

• find_package may not set LUA_LIBRARIES
• pkg_module_check will set LUA_LIBRARIES
• In the way of source code integration, the path of the library is written in the attribute of target
  so I don't think it be a good idea use LUA_LIBRARIES but not target. becase create a target will be easy, but it is cumbersome to get attributes from target and check

[cathaysia]

For rlottie, it might be more complex to go for this approach as it doesn't get distributed as a pre-compiled package but not impossible to make a proper integration since it supports CMake as a build system and a CMake package config file could be generated when building it so that it can be consumed as a pre-compiled dependency by other CMake projects. Adding it as a submodule could ease the integration as the build script could default to do a source build from the included rlottie via add_subdirectory() in case the consumer didn't provide a pre-compiled package without the consumer having to do it by themselves. That way, they would only have to go through the hassle of manually build it themselves if they actually want to.

In the event that rlottie can't be integrated using its CMake build option, then either a find module would have to be written for it or investigate the use of Meson's CMake module in their mesonfile so that it generates the CMake config module for us.

yes, this pr is for this case. In this case, rmlui needs to leave some interfaces for external use

[mikke89]

@hobyst Alright, thanks for chiming in. As I understand it, here we're considering cases where users have already included the external libraries through add_subdirectory. I guess it is best to be explicit about this using options as is done here.

• find_package may not set LUA_LIBRARIES
• pkg_module_check will set LUA_LIBRARIES
• In the way of source code integration, the path of the library is written in the attribute of target
  so I don't think it be a good idea use LUA_LIBRARIES but not target. becase create a target will be easy, but it is cumbersome to get attributes from target and check

@cathaysia Yeah, let's drop that suggestion, but I hope we can find some better names.

Could you perhaps write some basic examples of how you would include RmlUi from a parent scope together with the external libraries? That would be great, so we could possibly include this in the documentation.

[cathaysia]

sorry replay too late. :(
I give some commit, and rename RmlUi::Lua to RmlUi_External::Lua

then I add a repo for a sample:

https://github.com/cathaysia/rmlui_externallib_example

please let me know if you have any question about this. :)

and here is a video about this:

Peek.2023-02-12.20-58.mp4

[cathaysia]

You may find I use ${LUA_LIBRARIES} after find_package https://github.com/cathaysia/rmlui_externallib_example/blob/33e3acf2d8aa0f8d6e5506400d49d407b1dac95b/extern/CMakeLists.txt#L12

Generally speaking, find_package will look for libraries from two places: Find_xxx.cmake or xxxConfig.cmake

In the second form, the general library uses the domain name to define the header file path and library path. The XXX_LIBRARIES variable is not set in this case.

In the first form, it is up to the writer whether to set the XXX_LIBRARIES variable or not. Some people set this variable, others don't

Use find_package(XXX CONFIG) to force cmake to use the second form to find libraries.

[mikke89]

That example repo is great, except that it doesn't actually work for me. See the full log output below. It would be nice if we could make this work by just following the commands with minimal prerequisites. Ideally, this should include building the samples too. In any case, this is a good outline for something I can add to the documentation and link to.

I give some commit, and rename RmlUi::Lua to RmlUi_External::Lua

This seems good to me now. We are getting closer, there are still a couple of comments from the previous review, and a conflict that needs to be solved.

Command line output

PS C:\Projects\TestRmlUi\rmlui_externallib_example> cmake -B build
-- Building for: Visual Studio 17 2022
-- Selecting Windows SDK version 10.0.22000.0 to target Windows 10.0.19045.
-- The C compiler identification is MSVC 19.34.31937.0
-- The CXX compiler identification is MSVC 19.34.31937.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.34.31933/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.34.31933/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
-- The following HarfBuzz libraries were not found:
--  HarfBuzz (required)
-- Could NOT find HarfBuzz (missing: HarfBuzz_LIBRARY _HarfBuzz_REQUIRED_LIBS_FOUND) (found suitable version "4.3.0", minimum required is "2.0.0")
-- Found ZLIB: C:/msys64/mingw64/lib/libz.dll.a (found version "1.2.12")
-- Could NOT find PNG (missing: PNG_LIBRARY) (found version "1.6.37")
-- Could NOT find BZip2 (missing: BZIP2_LIBRARIES) (found version "1.0.8")
-- Could NOT find BrotliDec (missing: BROTLIDEC_LIBRARIES)
-- The ASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.34.31933/bin/Hostx64/x64/cl.exe
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - not found
-- Found Threads: TRUE
CMake Error at C:/Program Files/CMake/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Lua (missing: LUA_LIBRARIES) (found version "5.4.3")
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  C:/Program Files/CMake/share/cmake-3.25/Modules/FindLua.cmake:236 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  extern/CMakeLists.txt:9 (find_package)


-- Configuring incomplete, errors occurred!
See also "C:/Projects/TestRmlUi/rmlui_externallib_example/build/CMakeFiles/CMakeOutput.log".
See also "C:/Projects/TestRmlUi/rmlui_externallib_example/build/CMakeFiles/CMakeError.log".

[cathaysia]

I made some changes, it should work for now. But I haven't test it on windows. this question and others will be done later.

[cathaysia]

Did you notice? In the latest repo, I commit a FindLua.cmake file. This can be used with find_package(lua). Maybe I should remove RMLUI_LUA_EXTERNAL.
Similarly, other dependencies can do the same. Perhaps this pr should be closed.

I'll explore this later :)

[mikke89]

I tested this a bit more (on Windows), and have some feedback:

• It didn't work for me out-of-the-box, this time due to FreeType needing additional libraries (e.g. zlib).
• There is already a FindLua module included with CMake, so I'm not particularly a big fan of creating a custom one. Can we instead find the library by providing the correct options to the built-in find module?

I did make it work by disabling external libraries in FreeType. I also tested it with the RmlUi samples. Here are the commands I used:

cmake -B build -D BUILD_SAMPLES=ON -D FT_DISABLE_ZLIB=ON -D FT_DISABLE_BZIP2=ON -D FT_DISABLE_PNG=ON -D 
FT_DISABLE_HARFBUZZ=ON -D FT_DISABLE_BROTLI=ON
cmake --build build

This built successfully. But the samples won't launch because they can't find the RmlUi/Samples directory.

[cathaysia]

Ok, thanks. I am very sorry that this PR has been delayed for so long. Next, I will try my best to spare time to advance this pr.
As you say, I haven't fully tested this library on Windows. I'll get this done asap.

There is already a FindLua module included with CMake, so I'm not particularly a big fan of creating a custom one. Can we instead find the library by providing the correct options to the built-in find module?

Yes, it can be done surely. But I need to consider how to integrate with existing systems as much as possible. while not affecting previous code.

[cathaysia]

OK, I had made some changes in https://github.com/cathaysia/rmlui_externallib_example and https://github.com/cathaysia/RmlUi/tree/cmake .

• I removed ENABLE_LOTTIE_PLUGIN because it can be detected with TARGET::rlottie::rlottie : https://github.com/cathaysia/RmlUi/blob/2ce4f73cfd210161753fad80e42e25132dba6e03/CMakeLists.txt#L380
• I removed RMLUI_FREETYPE_EXTERNAL, because I can use FindFreetype.cmake replace it: https://github.com/cathaysia/rmlui_externallib_example/blob/72074d1986c251ac6d4b21bdb51f902b32c37525/cmake/FindFreetype.cmake#L4
• I removed RMLUI_LUA_EXTERNAL. Reason just like above

In these case. find_package(Lua REQUIRED) while call my FindLua.cmake file, then set some envs for rmlui.

[cathaysia]

What do you think of this approach? This code is also less intrusive.
If you prefer to change the cmake module in rmlui. Then we can use our own Findxxx.cmake to replace the system's.

[mikke89]

I like that it is less intrusive, and I'd be okay with adding rlottie like that.

I don't think we should override the built-in Lua find module inside RmlUi. Of course, I don't mind if users do it in a parent directory.

[cathaysia]

yes, I don't override build-in Lua inside RmlUi. The best practice about lua is don't handle it. If possible, user can override it by themeself.

[cathaysia]

all in all. What I have done just two things:

• disable export rule if rmlui is a submodule.
• add support for find rlottie by rlottie::rlottie.

[cathaysia]

I remove previous useless commits, and merge it to one. Merge it if you decide it's ok

[mikke89]

Yup, this looks good to me!