Update freetype.py to 2.12.1 #18088

HCLJason · 2022-10-21T19:09:10Z

This does introduce a dependency on system zilb to resolve a duplicate symbols issue. There's an open issue in freetype to fix this.

https://gitlab.freedesktop.org/freetype/freetype/-/issues/1146

The changes to the srcs list come from the CMakeLists file.

tools/ports/freetype.py

sbc100 · 2022-10-24T14:25:13Z

tools/ports/freetype.py

                             target=os.path.join('freetype2'))

+    with open(os.path.join(source_path, 'include', 'freetype', 'config', 'ftoption.h'), 'a') as ftheader:
+      ftheader.write('#define FT_CONFIG_OPTION_SYSTEM_ZLIB')


Seems like you don't need these two lines since you added FT_CONFIG_OPTION_SYSTEM_ZLIB to the config below?

I think those might be dead lines. I tried without the highlighted lines, and I got a bunch of duplicate symbol errors when linking.

I'm trying out a build without the config options in the zconf file now.

wasm-ld: error: duplicate symbol: get_crc_table >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(crc32.c.o) wasm-ld: error: duplicate symbol: crc32_z >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(crc32.c.o) wasm-ld: error: duplicate symbol: crc32_combine_op >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(crc32.c.o) wasm-ld: error: duplicate symbol: inflateResetKeep >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(inflate.c.o) wasm-ld: error: duplicate symbol: inflateInit_ >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(inflate.c.o) wasm-ld: error: duplicate symbol: inflateSyncPoint >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(inflate.c.o) wasm-ld: error: duplicate symbol: inflateUndermine >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a(ftgzip.c.o) >>> defined in /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a(inflate.c.o)

Removing the lines from ftconf.h and leaving this line works.

Whats the difference between ftconf.h and ftoption.h? It seems unfortunate to have to patch a shipping header like this.

ftconf.h looks like a dead link. ftoption.h is referenced a lot, but there's nothing that uses ftconf.h. I'm trying out a build without ftconf.h to see if that breaks.

I removed the ftconfig.h write and file and it built cleanly, so we should be able to remove that entirely.

There is a lot of stuff in that existing config file. How do we know we are not changing the configuration by removing it? Does freetype no longer use ftconfig.h?

I double-checked for ftconfig.h and that's actually used in the build, so I reverted that.

The build failure is freetype depending on zlib happening first. How do I denote a dependency?

I tried using the dependency logic referenced in tools/system_libs by adding it to freetype.py, but it didn't seem to work. I see that there's tools/deps_info.py, but that doesn't have any port->port dependencies.

You can add deps = ['zlib'] to this file I believe.

Change made.

We were previously pointing at a version_1 tag in our custom repo. This was basically VER-2.6 which a single patch applied. I can't see anything in that patch that is useful, perhaps this code didn't previously compile cleanly with emcc but today it seems to work just fine: emscripten-ports/FreeType@40a760c This seems like a good intermediate step before we actually do a freetype upgrade in #18088.

sbc100 · 2022-10-24T19:51:14Z

I created #18095 which seems like a good intermediate step, and should make the update smaller.

sbc100 · 2022-10-24T21:14:17Z

Were we previously using an internal version of zlib, or not using zlib at all?

ericoporto · 2022-10-25T01:51:47Z

tools/ports/freetype.py

-            'src/base/ftstream.c',
            'src/base/ftstroke.c',
-            'src/base/ftsynth.c',
            'src/base/ftsystem.c',


it should be builds/unix/ftsystem.c

see:

emscripten/cmake/Modules/Platform/Emscripten.cmake

Line 53 in 8922355

set(UNIX 1)

https://gitlab.freedesktop.org/freetype/freetype/-/blob/VER-2-12-1/CMakeLists.txt#L415

When I use builds/unix/ftsystem.c, the compile is missing the fcntl.h header. It looks like we'll need to pull the #undef HAVE_FCNTL_H from the config file.

/local/emsdk/upstream/emscripten/cache/ports/freetype/freetype-VER-2-12-1/builds/unix/ftsystem.c:258:12: error: call to undeclared function 'open'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] file = open( filepathname, O_RDONLY ); ^ /local/emsdk/upstream/emscripten/cache/ports/freetype/freetype-VER-2-12-1/builds/unix/ftsystem.c:258:32: error: use of undeclared identifier 'O_RDONLY' file = open( filepathname, O_RDONLY ); ^ /local/emsdk/upstream/emscripten/cache/ports/freetype/freetype-VER-2-12-1/builds/unix/ftsystem.c:341:22: error: call to undeclared function 'read'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] read_count = read( file, ^ /local/emsdk/upstream/emscripten/cache/ports/freetype/freetype-VER-2-12-1/builds/unix/ftsystem.c:341:22: note: did you mean 'fread'? /local/emsdk/upstream/emscripten/cache/sysroot/include/stdio.h:106:8: note: 'fread' declared here size_t fread(void *__restrict, size_t, size_t, FILE *__restrict); ^ /local/emsdk/upstream/emscripten/cache/ports/freetype/freetype-VER-2-12-1/builds/unix/ftsystem.c:362:5: error: call to undeclared function 'close'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] close( file );

I did some testing. All 3 settings for HAVE_FCNTL_H had build failures with builds/unix/ftsystem.c.

Did you check HAVE_UNISTD_H too? It was the only other thing of notice reverse engineering the cmake files...

The methods from the error are here: https://github.com/emscripten-core/emscripten/blob/e05e72d9c49fe15578e73934ce525a894d1b712a/system/lib/libc/musl/include/unistd.h

I tried #define HAVE_UNISTD_H and HAVE_STDINT_H in the ftconfig.h, and it doesn't work with any of them.

It seems to be working if I put them in ftoption.h, however.

Revising CMake, the full ftconfig.h file should be:

#ifndef FTCONFIG_H_ #define FTCONFIG_H_ #include <ft2build.h> #include FT_CONFIG_OPTIONS_H #include FT_CONFIG_STANDARD_LIBRARY_H #define HAVE_UNISTD_H 1 #define HAVE_FCNTL_H 1 #include <freetype/config/integer-types.h> #include <freetype/config/public-macros.h> #include <freetype/config/mac-support.h> #endif /* FTCONFIG_H_ */

I'm not sure where the ftconfig.h file in the build script comes from. I looked at the files from 2.6.1 and didn't get anywhere. It doesn't look like the other header files were inlined.

I tried this, and HAVE_UNISTD_H and HAVE_FCNTL_H don't seem to work from ftconfig.h. I'm going to put them back into ftoption.h and see what happens.

Leaving them in ftoption.h seems to work.

tools/ports/freetype.py

HCLJason · 2022-10-25T14:26:47Z

Were we previously using an internal version of zlib, or not using zlib at all?

I believe that it's using the internal version of zlib.

The defines don't work in ftconfig.h, but they work in ftoption.h.

HCLJason · 2022-10-26T15:59:04Z

I'm looking at the test-other failures. How do we move forward on these?

On my local system, test_freetype failed because it can't find the file LiberationSansBold.ttf.

test_freetype_with_pthreads and test_sdl2_ttf failed because it's looking for the symbol in the wrong place. This is probably a consequence of the zlib change.
error: undefined symbol: inflate (referenced by top-level compiled C/C++ code)

We were previously pointing at a version_1 tag in our custom repo. This was basically VER-2.6 which a single patch applied. I can't see anything in that patch that is useful, perhaps this code didn't previously compile cleanly with emcc but today it seems to work just fine: emscripten-ports/FreeType@40a760c This seems like a good intermediate step before we actually do a freetype upgrade in #18088.

Also, switch from a fork to using the upstream github repo. We had a single patch in our fork: emscripten-ports/FreeType@40a760c However, this was upstreamed in: freetype/freetype@2f4b740 This change is based on emscripten-core#18088.

sbc100 · 2024-09-18T22:45:30Z

I revived this change recently in the form of #22585.

@HCLJason, what do you think about the updated expected output of the freetype test? It looks like the rendering got a little narrower, but maybe that is normal/expected?

Also, switch from a fork to using the upstream github repo. We had a single patch in our fork: emscripten-ports/FreeType@40a760c However, this was upstreamed in: freetype/freetype@2f4b740 This change is based on emscripten-core#18088.

Also, switch from a fork to using the upstream github repo. We had a single patch in our fork: emscripten-ports/FreeType@40a760c However, this was upstreamed in: freetype/freetype@2f4b740 This change is based on emscripten-core#18088. Fixes: emscripten-core#22571

Also, switch from a fork to using the upstream github repo. We had a single patch in our fork: emscripten-ports/FreeType@40a760c However, this was upstreamed in: freetype/freetype@2f4b740 This change is based on #18088. Fixes: #22571

sbc100 · 2024-09-20T00:08:45Z

Closing this now that #22585 has landed

HCLJason added 2 commits October 21, 2022 15:04

Update freetype.py to 2.12.1 (#13)

719312e

Fixing flake8 messages

8b95490

HCLJason mentioned this pull request Oct 21, 2022

Invalid cache assertion building freetype port #18046

Open

sbc100 reviewed Oct 24, 2022

View reviewed changes

HCLJason added 3 commits October 24, 2022 13:42

Removed extra ftconf.h lines moved VER- into tag

9e27ee2

Removing unneeded ftconfig.h creation

2dd8897

Reverting the ftconfig.h removal

0002fc7

sbc100 mentioned this pull request Oct 24, 2022

Switch to upstream freetype repo #18095

Merged

ericoporto reviewed Oct 25, 2022

View reviewed changes

Compiling right svg file

70dbd90

HCLJason added 4 commits October 25, 2022 10:47

Adding explicit deps for zlib

63cf5f3

Using builds/unix/ftsystem.c

291d5eb

The defines don't work in ftconfig.h, but they work in ftoption.h.

Removing the unix defines from ftconfig.h

48de39f

Using ftconfig.h from CMake

2a4432c

curiousdannii mentioned this pull request May 24, 2024

Randomly occurring libpng bug in png_read_IDAT_data #21930

Closed

sbc100 mentioned this pull request Sep 18, 2024

Upgrade freetype port from 2.6 to 2.13.3 #22585

Merged

sbc100 closed this Sep 20, 2024

Update freetype.py to 2.12.1 #18088

Update freetype.py to 2.12.1 #18088

Uh oh!

Conversation

HCLJason commented Oct 21, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HCLJason Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 commented Oct 24, 2022

Uh oh!

sbc100 commented Oct 24, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericoporto Oct 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HCLJason Oct 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericoporto Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HCLJason commented Oct 25, 2022

Uh oh!

HCLJason commented Oct 26, 2022

Uh oh!

sbc100 commented Sep 18, 2024

Uh oh!

sbc100 commented Sep 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HCLJason Oct 24, 2022 •

edited

Loading

ericoporto Oct 25, 2022 •

edited

Loading

HCLJason Oct 25, 2022 •

edited

Loading

ericoporto Oct 26, 2022 •

edited

Loading