SFML dependency wrapper code cleanup #551

hobyst · 2023-12-08T12:39:27Z

Remove remnant of SFML <= 2.4 support
Add prefix to variable names on loop
Fix a typo in a comment

- Remove remnant of SFML <= 2.4 support - Add prefix to variable names on loop - Fix a typo in a comment

mikke89 · 2023-12-11T22:08:43Z

Thanks for the follow-up!

- Remove remnant of SFML <= 2.4 support - Add prefix to variable names on loop - Fix a typo in a comment

@hobyst

Partial rewrite originally written by @hobyst in #446 and #551. Remaining code and changes written by @mikke89. These changes should generally be feature-complete with the previous CMake code, with some exceptions to legacy and certain Apple-specific behavior. Please see the changelog for breaking changes, in particular, several options, CMake targets, and file names have been changed. The following lists some noteworthy changes: - CMake presets now available. - Major changes to CI automation and tests. - Most tests on Appveyor are moved to Github actions. This includes a completely new packaging workflow on GitHub. - Packaging now uses Visual Studio 2019 instead of 2017. Keep Appveyor only for testing with Visual Studio 2017. - Use CMake scripts for some packaging tasks to avoid avoid duplicate workflow code. - `RmlUi::RmlUi` is now an include-all library target, while components like `RmlUi::Core` and `RmlUi::Debugger` can be chosen individually. - Add `contributing.md` guidelines for CMake. - Backends can now be changed without recompiling the whole library. - Set CMake `policy_max` to a recent version. This opts in to a number of CMake improvements for users that can take advantage of that. - Sample bitmapfont now available without FreeType. - Runtime outputs are now placed in the binary root directory. - Runtime dependencies can now be installed automatically, this includes dependencies from vcpkg. - Update .editorconfig and format all CMake files consistently. - Rename `harfbuzzshaping` sample to `harfbuzz`. - Fix harfbuzz compatibility for older versions. - Remove CMake warning on FreeType version. In the following, some working notes and lessons learned are written down (authored by @mikke89): - Avoid separate libraries for subparts of Core Originally, parts of core such as Elements, Layout and FontEngineDefault, were added as interface libraries. Defining these as interface libraries cause issues during export. I believe we would have to export them as separate targets, but that doesn't relly make sense from a client standpoint and causes new issues with exported source files and dependencies. It seems to be a lot less troublesome to use a single CMake library and set properties and attach sources to that directly. - Avoid interface libraries for plugins and Lua dependencies Using interface libraries to add plugins causes problems when installing targets. Even trying to split the interface libraries into private and public parts did not work suitably. An issue arises when compiling the project as static libraries. Consider when we are linking targets to rmlui_core. Even when they are added as private dependencies, CMake adds it as a transient link-time dependency. This even applies to interface dependencies, which causes problems for our private interface libraries representing the plugins. In particular, CMake wants us to export these libraries, as now they are referenced from rmlui_core, but we don't want to export them, as that was the whole point of making them private and would cause new issues. Instead, add the SVG and Lottie source files and imported dependencies directly to Core. This simplifies things quite a bit, and seems to work reliably. For similar reasons, use imported interface targets instead of a pure interface library for the custom Lua target. - Common options for CMake targets Using CMake presets to specify compiler flags, warnings in particular, is not a great experience. Presets require one to set all the flags at the same time (in a single variable). For example, we can't easily set just /WX in one profile and /MP in another. There are workarounds, but being more pragmatic, setting these flags in a cmake file is much more straightforward with proper conditionals. - On CMake presets I also experimented with more detailed, platform-dependent presets, but found that it is not really that useful. In particular, we want users themselves to use whichever compilers and generators they want to rather than to constrain the choices. We strive to be agnostict toward both generators and platforms. This way, we we can also give more common build instructions regardless of platform. Also tested with making presets for e.g. vcpkg and mingw toolchains. However, this doesn't really make sense in presets. First of all, the choice of such a toolchain is not mutually exclusive with the current presets, so we would have to replicate most of them for each toolchain we wanted to support. Further, the exact specifics of these toolcahins, like version, platform, install location, and so on, is very specific to each case. Thus, if we wanted to handle these it would create additional dimensions to the presets, and we'd end up with a huge list of similar presets we would have to mainatain and which users would have to choose from. Or alternatively, users would have to provide a lot of their setup in any case, thereby dismissing some of the initially considered usefulness. Instead, many tools already allow you to setup a toolchain first which can then be used with a given preset. Thus, making the choice of toolchains and cmake presets orthogonal makes a lot more sense. One unfortunate issue from a user-friendliness perspective is that the cmake gui doesn't allow you to select a preset without a specified generator. There's an issue for this here: https://gitlab.kitware.com/cmake/cmake/-/issues/23341 I figured accepting this for now is better than duplicating all our presets to specify different generators. But the situations is not ideal, hopefully it will be fixed soon. - Recommended compiler flags The compiler flags are enabled by default to ensure users get a good experience when building the library without fiddling with options. Especially on MSVC, the default compiler flags means really slow builds (no /MP), and we also risk exposing more warnings. Co-authored-by: Michael Ragazzon <michael.ragazzon@gmail.com>

@hobyst

Partial rewrite originally written by @hobyst in #446 and #551. Remaining code and changes written by @mikke89. These changes should generally be feature-complete with the previous CMake code, with some exceptions to legacy and certain Apple-specific behavior. Please see the changelog for breaking changes, in particular, several options, CMake targets, and file names have been changed. The following lists some noteworthy changes: - CMake presets now available. - Major changes to CI automation and tests. - Most tests on Appveyor are moved to Github actions. This includes a completely new packaging workflow on GitHub. - Packaging now uses Visual Studio 2019 instead of 2017. Keep Appveyor only for testing with Visual Studio 2017. - Use CMake scripts for some packaging tasks to avoid avoid duplicate workflow code. - `RmlUi::RmlUi` is now an include-all library target, while components like `RmlUi::Core` and `RmlUi::Debugger` can be chosen individually. - Add `contributing.md` guidelines for CMake. - Backends can now be changed without recompiling the whole library. - Set CMake `policy_max` to a recent version. This opts in to a number of CMake improvements for users that can take advantage of that. - Sample bitmapfont now available without FreeType. - Runtime outputs are now placed in the binary root directory. - Runtime dependencies can now be installed automatically, this includes dependencies from vcpkg. - Update .editorconfig and format all CMake files consistently. - Rename `harfbuzzshaping` sample to `harfbuzz`. - Fix harfbuzz compatibility for older versions. - Remove CMake warning on FreeType version. In the following, some working notes and lessons learned are written down (authored by @mikke89): - Avoid separate libraries for subparts of Core Originally, parts of core such as Elements, Layout and FontEngineDefault, were added as interface libraries. Defining these as interface libraries cause issues during export. I believe we would have to export them as separate targets, but that doesn't relly make sense from a client standpoint and causes new issues with exported source files and dependencies. It seems to be a lot less troublesome to use a single CMake library and set properties and attach sources to that directly. - Avoid interface libraries for plugins and Lua dependencies Using interface libraries to add plugins causes problems when installing targets. Even trying to split the interface libraries into private and public parts did not work suitably. An issue arises when compiling the project as static libraries. Consider when we are linking targets to rmlui_core. Even when they are added as private dependencies, CMake adds it as a transient link-time dependency. This even applies to interface dependencies, which causes problems for our private interface libraries representing the plugins. In particular, CMake wants us to export these libraries, as now they are referenced from rmlui_core, but we don't want to export them, as that was the whole point of making them private and would cause new issues. Instead, add the SVG and Lottie source files and imported dependencies directly to Core. This simplifies things quite a bit, and seems to work reliably. For similar reasons, use imported interface targets instead of a pure interface library for the custom Lua target. - Common options for CMake targets Using CMake presets to specify compiler flags, warnings in particular, is not a great experience. Presets require one to set all the flags at the same time (in a single variable). For example, we can't easily set just /WX in one profile and /MP in another. There are workarounds, but being more pragmatic, setting these flags in a cmake file is much more straightforward with proper conditionals. - On CMake presets I also experimented with more detailed, platform-dependent presets, but found that it is not really that useful. In particular, we want users themselves to use whichever compilers and generators they want to rather than to constrain the choices. We strive to be agnostict toward both generators and platforms. This way, we we can also give more common build instructions regardless of platform. Also tested with making presets for e.g. vcpkg and mingw toolchains. However, this doesn't really make sense in presets. First of all, the choice of such a toolchain is not mutually exclusive with the current presets, so we would have to replicate most of them for each toolchain we wanted to support. Further, the exact specifics of these toolcahins, like version, platform, install location, and so on, is very specific to each case. Thus, if we wanted to handle these it would create additional dimensions to the presets, and we'd end up with a huge list of similar presets we would have to mainatain and which users would have to choose from. Or alternatively, users would have to provide a lot of their setup in any case, thereby dismissing some of the initially considered usefulness. Instead, many tools already allow you to setup a toolchain first which can then be used with a given preset. Thus, making the choice of toolchains and cmake presets orthogonal makes a lot more sense. One unfortunate issue from a user-friendliness perspective is that the cmake gui doesn't allow you to select a preset without a specified generator. There's an issue for this here: https://gitlab.kitware.com/cmake/cmake/-/issues/23341 I figured accepting this for now is better than duplicating all our presets to specify different generators. But the situations is not ideal, hopefully it will be fixed soon. - Recommended compiler flags The compiler flags are enabled by default to ensure users get a good experience when building the library without fiddling with options. Especially on MSVC, the default compiler flags means really slow builds (no /MP), and we also risk exposing more warnings. Co-authored-by: Michael Ragazzon <michael.ragazzon@gmail.com>

SFML dependency wrapper code cleanup

88edcfa

- Remove remnant of SFML <= 2.4 support - Add prefix to variable names on loop - Fix a typo in a comment

mikke89 added the build Build system and compilation label Dec 10, 2023

mikke89 merged commit c40d18a into mikke89:cmake Dec 11, 2023
11 of 12 checks passed

mikke89 pushed a commit that referenced this pull request Mar 31, 2024

SFML dependency wrapper code cleanup (#551)

6f3a14f

- Remove remnant of SFML <= 2.4 support - Add prefix to variable names on loop - Fix a typo in a comment

mikke89 pushed a commit that referenced this pull request Apr 6, 2024

SFML dependency wrapper code cleanup (#551)

efbeaab

- Remove remnant of SFML <= 2.4 support - Add prefix to variable names on loop - Fix a typo in a comment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SFML dependency wrapper code cleanup #551

SFML dependency wrapper code cleanup #551

hobyst commented Dec 8, 2023

mikke89 commented Dec 11, 2023

SFML dependency wrapper code cleanup #551

SFML dependency wrapper code cleanup #551

Conversation

hobyst commented Dec 8, 2023

mikke89 commented Dec 11, 2023