Skip to content

cmake: make possible to build both game and engine against the engine Freetype submodule #1617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented Mar 24, 2025

Make possible to build both game and engine against the engine Freetype submodule.

Extracted from #1585:

Without the controversial change about default building.

See also:

Some libraries may be built statically before being linked
against game dll, so -fPIC would be required, and then we
better compile everything with -fPIC.
@slipher
Copy link
Member

slipher commented Mar 24, 2025

LGTM

@slipher
Copy link
Member

slipher commented Mar 24, 2025

Wait, what is FREETYPE_INTERNAL_ZLIB supposed to mean? We have the submodule configured so that there is no zlib dependency at all, right? The zlib dependency only comes in now for our poorly built Windows or Mac external_deps Freetype.

@illwieckz
Copy link
Member Author

Freetype ships with an internal zlib, this is good enough for the nacl game because this is the only usage of zlib in the nacl game, so we use FREETYPE_INTERNAL_ZLIB, it prevents us to require a zlib submodule just to build the nacl game.

But then the engine may share zlib with freetype and libpng, so we don't use the internal freetype zlib to avoid having two zlib in the engine binary (they don't conflict as far as I know but it's just stupid to have zlib twice).

@illwieckz
Copy link
Member Author

Hmm, rereading the code, we may use FREETYPE_INTERNAL_ZLIB only with NACL then.

@slipher
Copy link
Member

slipher commented Mar 24, 2025

We disable Brotli, bzip2, Harfbuzz, and PNG which means that we won't have a zlib dependency according to the comment on https://stackoverflow.com/a/29456684.

@illwieckz
Copy link
Member Author

Yes but the engine itself links against libpng to open PNG images, so it already links against zlib.

Do you mean we completely avoid zlib when building Freetype? I'm afraid we cannot avoid that, unlike FT_DISABLE_BZIP2 that disables bzip2 entirely, FT_DISABLE_ZLIB switches to internal Freetype zlib. The Freetype cmake doesn't give options to disable ZLIB entirely.

@illwieckz
Copy link
Member Author

The configure's --with-zlib=no option also doesn't allow to entirely disable zlib:

$ cd libs/freetype
$ ./autogen.sh
$ ./configure -h
…
 --with-zlib=[yes|no|auto]
                          use system zlib instead of internal library
                          [default=auto]
…

@illwieckz
Copy link
Member Author

I added the commit to only use the Freetype internal zlib when building an NaCl game.

@slipher
Copy link
Member

slipher commented Mar 24, 2025

I read the Freetype CMakeLists.txt and it doesn't have internal zlib. I think it only implements a subset of the options of the configure script. When using CMake, if you disable zlib, there is really no zlib at all. make VERBOSE=1 freetype seems to confirm this.

@slipher
Copy link
Member

slipher commented Mar 25, 2025

I was wrong, it does implement internal zlib. I couldn't see it being built because it works by #including many .c source files. No, they couldn't just build it normally, gotta make hundreds of lines of patches to zlib so we can #include it!

There is an FT_CONFIG_OPTION_USE_ZLIB that we could theoretically use to turn off zlib. I guess we'd have to make our own customized ftoptions.h.

@slipher
Copy link
Member

slipher commented Mar 25, 2025

LGTM

@illwieckz illwieckz merged commit d6fdafd into master Mar 25, 2025
9 checks passed
@illwieckz illwieckz deleted the illwieckz/freetype/sync branch March 25, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants