Merge pull request #5062 from nulano/fribidi-link

Dynamically link FriBiDi instead of Raqm

Author: wiredfool

.github/workflows/test-windows.yml

@@ -137,14 +137,11 @@ jobs:
       if: steps.build-cache.outputs.cache-hit != 'true'
       run: "& winbuild\\build\\build_dep_harfbuzz.cmd"
 
+    # Raqm dependencies
     - name: Build dependencies / FriBidi
       if: steps.build-cache.outputs.cache-hit != 'true'
       run: "& winbuild\\build\\build_dep_fribidi.cmd"
 
-    - name: Build dependencies / Raqm
-      if: steps.build-cache.outputs.cache-hit != 'true'
-      run: "& winbuild\\build\\build_dep_libraqm.cmd"
-
     # trim ~150MB x 9
     - name: Optimize build cache
       if: steps.build-cache.outputs.cache-hit != 'true'

setup.py

@@ -29,6 +29,8 @@ def get_version():
 NAME = "Pillow"
 PILLOW_VERSION = get_version()
 FREETYPE_ROOT = None
+HARFBUZZ_ROOT = None
+FRIBIDI_ROOT = None
 IMAGEQUANT_ROOT = None
 JPEG2K_ROOT = None
 JPEG_ROOT = None
@@ -228,6 +230,19 @@ def _find_library_file(self, library):
     return ret
 
 
+def _find_include_dir(self, dirname, include):
+    for directory in self.compiler.include_dirs:
+        _dbg("Checking for include file %s in %s", (include, directory))
+        if os.path.isfile(os.path.join(directory, include)):
+            _dbg("Found %s in %s", (include, directory))
+            return True
+        subdir = os.path.join(directory, dirname)
+        _dbg("Checking for include file %s in %s", (include, subdir))
+        if os.path.isfile(os.path.join(subdir, include)):
+            _dbg("Found %s in %s", (include, subdir))
+            return subdir
+
+
 def _cmd_exists(cmd):
     return any(
         os.access(os.path.join(path, cmd), os.X_OK)
@@ -267,6 +282,7 @@ class feature:
             "jpeg",
             "tiff",
             "freetype",
+            "raqm",
             "lcms",
             "webp",
             "webpmux",
@@ -276,6 +292,7 @@ class feature:
         ]
 
         required = {"jpeg", "zlib"}
+        vendor = set()
 
         def __init__(self):
             for f in self.features:
@@ -287,6 +304,9 @@ def require(self, feat):
         def want(self, feat):
             return getattr(self, feat) is None
 
+        def want_vendor(self, feat):
+            return feat in self.vendor
+
         def __iter__(self):
             yield from self.features
 
@@ -296,6 +316,10 @@ def __iter__(self):
         build_ext.user_options
         + [(f"disable-{x}", None, f"Disable support for {x}") for x in feature]
         + [(f"enable-{x}", None, f"Enable support for {x}") for x in feature]
+        + [
+            (f"vendor-{x}", None, f"Use vendored version of {x}")
+            for x in ("raqm", "fribidi")
+        ]
         + [
             ("disable-platform-guessing", None, "Disable platform guessing on Linux"),
             ("debug", None, "Debug logging"),
@@ -310,6 +334,8 @@ def initialize_options(self):
         for x in self.feature:
             setattr(self, f"disable_{x}", None)
             setattr(self, f"enable_{x}", None)
+        for x in ("raqm", "fribidi"):
+            setattr(self, f"vendor_{x}", None)
 
     def finalize_options(self):
         build_ext.finalize_options(self)
@@ -334,18 +360,40 @@ def finalize_options(self):
                     raise ValueError(
                         f"Conflicting options: --enable-{x} and --disable-{x}"
                     )
+                if x == "freetype":
+                    _dbg("--disable-freetype implies --disable-raqm")
+                    if getattr(self, "enable_raqm"):
+                        raise ValueError(
+                            "Conflicting options: --enable-raqm and --disable-freetype"
+                        )
+                    setattr(self, "disable_raqm", True)
             if getattr(self, f"enable_{x}"):
                 _dbg("Requiring %s", x)
                 self.feature.required.add(x)
+                if x == "raqm":
+                    _dbg("--enable-raqm implies --enable-freetype")
+                    self.feature.required.add("freetype")
+        for x in ("raqm", "fribidi"):
+            if getattr(self, f"vendor_{x}"):
+                if getattr(self, "disable_raqm"):
+                    raise ValueError(
+                        f"Conflicting options: --vendor-{x} and --disable-raqm"
+                    )
+                if x == "fribidi" and not getattr(self, "vendor_raqm"):
+                    raise ValueError(
+                        f"Conflicting options: --vendor-{x} and not --vendor-raqm"
+                    )
+                _dbg("Using vendored version of %s", x)
+                self.feature.vendor.add(x)
 
-    def _update_extension(self, name, libraries, define_macros=None, include_dirs=None):
+    def _update_extension(self, name, libraries, define_macros=None, sources=None):
         for extension in self.extensions:
             if extension.name == name:
                 extension.libraries += libraries
                 if define_macros is not None:
                     extension.define_macros += define_macros
-                if include_dirs is not None:
-                    extension.include_dirs += include_dirs
+                if sources is not None:
+                    extension.sources += sources
                 if FUZZING_BUILD:
                     extension.language = "c++"
                     extension.extra_link_args = ["--stdlib=libc++"]
@@ -374,6 +422,8 @@ def build_extensions(self):
             TIFF_ROOT=("libtiff-5", "libtiff-4"),
             ZLIB_ROOT="zlib",
             FREETYPE_ROOT="freetype2",
+            HARFBUZZ_ROOT="harfbuzz",
+            FRIBIDI_ROOT="fribidi",
             LCMS_ROOT="lcms2",
             IMAGEQUANT_ROOT="libimagequant",
         ).items():
@@ -659,6 +709,39 @@ def build_extensions(self):
                     if subdir:
                         _add_directory(self.compiler.include_dirs, subdir, 0)
 
+        if feature.freetype and feature.want("raqm"):
+            if not feature.want_vendor("raqm"):  # want system Raqm
+                _dbg("Looking for Raqm")
+                if _find_include_file(self, "raqm.h"):
+                    if _find_library_file(self, "raqm"):
+                        feature.raqm = "raqm"
+                    elif _find_library_file(self, "libraqm"):
+                        feature.raqm = "libraqm"
+            else:  # want to build Raqm from src/thirdparty
+                _dbg("Looking for HarfBuzz")
+                feature.harfbuzz = None
+                hb_dir = _find_include_dir(self, "harfbuzz", "hb.h")
+                if hb_dir:
+                    if isinstance(hb_dir, str):
+                        _add_directory(self.compiler.include_dirs, hb_dir, 0)
+                    if _find_library_file(self, "harfbuzz"):
+                        feature.harfbuzz = "harfbuzz"
+                if feature.harfbuzz:
+                    if not feature.want_vendor("fribidi"):  # want system FriBiDi
+                        _dbg("Looking for FriBiDi")
+                        feature.fribidi = None
+                        fribidi_dir = _find_include_dir(self, "fribidi", "fribidi.h")
+                        if fribidi_dir:
+                            if isinstance(fribidi_dir, str):
+                                _add_directory(
+                                    self.compiler.include_dirs, fribidi_dir, 0
+                                )
+                            if _find_library_file(self, "fribidi"):
+                                feature.fribidi = "fribidi"
+                                feature.raqm = True
+                    else:  # want to build FriBiDi shim from src/thirdparty
+                        feature.raqm = True
+
         if feature.want("lcms"):
             _dbg("Looking for lcms")
             if _find_include_file(self, "lcms2.h"):
@@ -754,9 +837,25 @@ def build_extensions(self):
         # additional libraries
 
         if feature.freetype:
+            srcs = []
             libs = ["freetype"]
             defs = []
-            self._update_extension("PIL._imagingft", libs, defs)
+            if feature.raqm:
+                if not feature.want_vendor("raqm"):  # using system Raqm
+                    defs.append(("HAVE_RAQM", None))
+                    defs.append(("HAVE_RAQM_SYSTEM", None))
+                    libs.append(feature.raqm)
+                else:  # building Raqm from src/thirdparty
+                    defs.append(("HAVE_RAQM", None))
+                    srcs.append("src/thirdparty/raqm/raqm.c")
+                    libs.append(feature.harfbuzz)
+                    if not feature.want_vendor("fribidi"):  # using system FriBiDi
+                        defs.append(("HAVE_FRIBIDI_SYSTEM", None))
+                        libs.append(feature.fribidi)
+                    else:  # building FriBiDi shim from src/thirdparty
+                        srcs.append("src/thirdparty/fribidi-shim/fribidi.c")
+            self._update_extension("PIL._imagingft", libs, defs, srcs)
+
         else:
             self._remove_extension("PIL._imagingft")
 
@@ -803,13 +902,20 @@ def summary_report(self, feature):
             print(f"             [{v.strip()}")
         print("-" * 68)
 
+        raqm_extra_info = ""
+        if feature.want_vendor("raqm"):
+            raqm_extra_info += "bundled"
+            if feature.want_vendor("fribidi"):
+                raqm_extra_info += ", FriBiDi shim"
+
         options = [
             (feature.jpeg, "JPEG"),
             (feature.jpeg2000, "OPENJPEG (JPEG2000)", feature.openjpeg_version),
             (feature.zlib, "ZLIB (PNG/ZIP)"),
             (feature.imagequant, "LIBIMAGEQUANT"),
             (feature.tiff, "LIBTIFF"),
             (feature.freetype, "FREETYPE2"),
+            (feature.raqm, "RAQM (Text shaping)", raqm_extra_info),
             (feature.lcms, "LITTLECMS2"),
             (feature.webp, "WEBP"),
             (feature.webpmux, "WEBPMUX"),
@@ -819,10 +925,10 @@ def summary_report(self, feature):
         all = 1
         for option in options:
             if option[0]:
-                version = ""
+                extra_info = ""
                 if len(option) >= 3 and option[2]:
-                    version = f" ({option[2]})"
-                print(f"--- {option[1]} support available{version}")
+                    extra_info = f" ({option[2]})"
+                print(f"--- {option[1]} support available{extra_info}")
             else:
                 print(f"*** {option[1]} support not available")
                 all = 0

src/PIL/features.py

@@ -118,6 +118,8 @@ def get_supported_codecs():
     "webp_mux": ("PIL._webp", "HAVE_WEBPMUX", None),
     "transp_webp": ("PIL._webp", "HAVE_TRANSPARENCY", None),
     "raqm": ("PIL._imagingft", "HAVE_RAQM", "raqm_version"),
+    "fribidi": ("PIL._imagingft", "HAVE_FRIBIDI", "fribidi_version"),
+    "harfbuzz": ("PIL._imagingft", "HAVE_HARFBUZZ", "harfbuzz_version"),
     "libjpeg_turbo": ("PIL._imaging", "HAVE_LIBJPEGTURBO", "libjpeg_turbo_version"),
     "libimagequant": ("PIL._imaging", "HAVE_LIBIMAGEQUANT", "imagequant_version"),
     "xcb": ("PIL._imaging", "HAVE_XCB", None),
@@ -274,6 +276,11 @@ def pilinfo(out=None, supported_formats=True):
                     # this check is also in src/_imagingcms.c:setup_module()
                     version_static = tuple(int(x) for x in v.split(".")) < (2, 7)
                 t = "compiled for" if version_static else "loaded"
+                if name == "raqm":
+                    for f in ("fribidi", "harfbuzz"):
+                        v2 = version_feature(f)
+                        if v2 is not None:
+                            v += f", {f} {v2}"
                 print("---", feature, "support ok,", t, v, file=out)
             else:
                 print("---", feature, "support ok", file=out)