servo / font-kit

A cross-platform font loading library written in Rust
Apache License 2.0
678 stars 100 forks source link

switch to yeslogic-fontconfig-sys instead of servo-fontconfig #192

Closed Be-ing closed 2 years ago

Be-ing commented 2 years ago

servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: https://github.com/slint-ui/slint/issues/88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig:

$ pkg-config fontconfig --static --libs
-lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm

Instead of using a vendored copy, with https://github.com/yeslogic/fontconfig-rs/pull/12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

Be-ing commented 2 years ago

This is a draft until https://github.com/yeslogic/fontconfig-rs/pull/12 is merged upstream.

Be-ing commented 2 years ago

See also: https://github.com/servo/libfontconfig/issues/65 https://github.com/yeslogic/fontconfig-rs/issues/11

Having two different -sys crates for fontconfig has created a messy situation because Cargo will refuse to build if two -sys crates link the same library by specifying link = "fontconfig" in Cargo.toml. It would be great if we could converge on one and deprecate the other. If you agree to drop servo-fontconfig, I'd be happy to make pull request's for servo-fontconfig's other reverse dependencies to switch them to yeslogic-fontconfig-sys.

Be-ing commented 2 years ago

PR to switch Slint to yeslogic-fontconfig-sys from servo-fontconfig: https://github.com/slint-ui/slint/pull/956

Be-ing commented 2 years ago

This is a draft until https://github.com/yeslogic/fontconfig-rs/pull/12 is merged upstream.

That was merged and released upstream. This is ready for review.

Be-ing commented 2 years ago

Hi @jdm are you still maintaining this crate?

jdm commented 2 years ago

This seems like a reasonable path forwards. Thanks! @bors-servo r+

bors-servo commented 2 years ago

:pushpin: Commit 68d461e has been approved by jdm

bors-servo commented 2 years ago

:hourglass: Testing commit 68d461eb205e84f2b52a85976ed8d545e57f73c1 with merge 60812fd2633aad876f4b8723443e18369d622f2c...

bors-servo commented 2 years ago

:broken_heart: Test failed - checks-github

jdm commented 2 years ago

Can you run cargo fmt?

Be-ing commented 2 years ago

Okay, I ran cargo fmt.

jdm commented 2 years ago

@bors-servo r+

bors-servo commented 2 years ago

:pushpin: Commit 4130e14 has been approved by jdm

bors-servo commented 2 years ago

:hourglass: Testing commit 4130e14f0e262cf065f3bc71630492808948f463 with merge 75f99cf648885c180c7ccfb9b6d55eae259e1d71...

bors-servo commented 2 years ago

:sunny: Test successful - checks-github Approved by: jdm Pushing 75f99cf648885c180c7ccfb9b6d55eae259e1d71 to master...

Be-ing commented 2 years ago

Thanks for merging. Could you publish a new release of the crate?

jdm commented 2 years ago

Done.