Skip to content
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

Drop FreeType in favor of the HarfBuzz draw API, but using C, not Python pens #267

Closed
justvanrossum opened this issue Jul 25, 2022 · 17 comments · Fixed by #269
Closed

Drop FreeType in favor of the HarfBuzz draw API, but using C, not Python pens #267

justvanrossum opened this issue Jul 25, 2022 · 17 comments · Fixed by #269
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@justvanrossum
Copy link
Owner

This was already done in #117, but was rejected as it not as performant as the current solution that constructs the CoreGraphics paths in C, using FreeType data structures.

I need to figure out how to link the "turbo" C lib to the HarfBuzz embedded into the uharfbuzz Python extension.

@justvanrossum justvanrossum added enhancement New feature or request help wanted Extra attention is needed labels Jul 25, 2022
@anthrotype
Copy link
Collaborator

anthrotype commented Jul 25, 2022

maybe you could build Harfbuzz as a shared dynamic library (.dylib) and build uharfbuzz from source and tell it to link to that one?
We need to add an option to uharfbuzz setup.py (similar to the one we have on skia-pathops for example) to not build the wrapped library from source but link to an existing one, see harfbuzz/uharfbuzz#45

@justvanrossum
Copy link
Owner Author

Possibly naive idea: uharfbuzz could provide a way to get a (ctypes) pointer to the hb_font_t structure of Font, and a (ctypes) pointer to the hb_font_get_glyph_shape function. Those pointers could be explicitly passed to custom C code, which then won't need to be linked against HB.

@anthrotype
Copy link
Collaborator

hm it might work but sounds a bit hacky/dangerous.
Maybe better to take advantage of cython, since uharfbuzz is written in cython.
Uharfbuzz can export a .pxd file (cython declaration file, equivalent to a C .h header but for cython modules) which another cython module of yours can include and gain access to some symbols from uharfbuzz extension at the C level.
https://cython.readthedocs.io/en/latest/src/tutorial/pxd_files.html#pxd-files

@anthrotype
Copy link
Collaborator

but I think creating a single harfbuzz dylib and linking it from both uharfbuzz and your custom C code would be the easiest for fontgoggles' purposes

@anthrotype
Copy link
Collaborator

You may also have a look at https://cython.readthedocs.io/en/latest/src/userguide/external_C_code.html#using-cython-declarations-from-c where they discuss about two methods for making C declarations from a Cython module available for use by external C code (cdef public vs cdef api). I haven't tried any of them

@justvanrossum
Copy link
Owner Author

but I think creating a single harfbuzz dylib and linking it from both uharfbuzz and your custom C code would be the easiest for fontgoggles' purposes

Nothing about this sounds "easy" to me :(

@anthrotype
Copy link
Collaborator

I can take care of allowing to build uharfbuzz extension module without also bulding the wrapped harfbuzz library from source, but dynamically linking to an existing one (I've done for skia-pathops already).

Then, we need to build a shared library for harfbuzz (preferably one that is compatible with a given uharfbuzz version), using hb's own build system (meson). You'd build uharfbuzz from source and pass some environment variable to give it the directory where this harfbuzz.dylib is located -- so that setuptools can add that to the library search paths when linking the uharfbuzz extension module.

Finally, you write that C code that uses the harfbuzz API to what you want to do, and tell gcc or clang to link with -lharfbuzz -L directory/where/dylib/is/located -I directory/where/hb/headers/are

@justvanrossum
Copy link
Owner Author

While I appreciate your offer for help, I'm not keen on building HB as part of FontGoggles.

I don't have a problem with a more hack-ish approach, as that's pretty much what I'm doing now. Which is possible (and super easy) because freetype-py is made with ctypes, so all underlying pointers are exposed. All I need is two HB pointers.

@anthrotype
Copy link
Collaborator

ok then, if ctypes does the trick and it's easier to integrate, try that

@khaledhosny
Copy link
Contributor

I’m wondering if we return a list of paths from uharfbuzz, something like:

(
  ("m", 0, 0),
  ("l", 100, 100),
  ("c", 200, 200, 300, 300, 400, 400)
  ("z").
)

Would it allow fast creating for NSBezierPath’s at FG side?

@justvanrossum
Copy link
Owner Author

Hm, I would love to avoid Python objects altogether. It is a frequently called inner loop, so having zero Python code there is the ideal situation.

I'm exploring some options now. Maybe the FG C lib could export some hb_draw_*_func_t function pointers, and I can add a method to uharfbuzz to take these in the form of ctypes (function) pointers. Or perhaps uharfbuzz's DrawFuncs.set_*_func() methods can be enhanced to take ctypes function pointers in a addition to Python function objects.

@khaledhosny
Copy link
Contributor

Sounds good to me, passing ctypes function pointers to DrawFuncs.set_*_func() seems the cleanest.

@khaledhosny
Copy link
Contributor

khaledhosny commented Jul 25, 2022

I tried to get this to work, but I’m probably doing some stupid mistake:

diff --git a/src/uharfbuzz/_harfbuzz.pyx b/src/uharfbuzz/_harfbuzz.pyx
index 60dd6a2..c7bb90a 100644
--- a/src/uharfbuzz/_harfbuzz.pyx
+++ b/src/uharfbuzz/_harfbuzz.pyx
@@ -1,9 +1,11 @@
 #cython: language_level=3
 import os
 import warnings
+import ctypes
 from enum import IntEnum
 from .charfbuzz cimport *
 from libc.stdlib cimport free, malloc, calloc
+from libc.stdint cimport uintptr_t
 from libc.string cimport const_char
 from collections import namedtuple
 from typing import Callable, Dict, List, Sequence, Tuple, Union
@@ -1085,6 +1087,31 @@ cdef class DrawFuncs:
         hb_draw_funcs_set_close_path_func(
             self._hb_drawfuncs, _close_path_func, <void*>user_data, NULL)

+    def set_move_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef uintptr_t ptr = <uintptr_t>ctypes.addressof(func)
+        hb_draw_funcs_set_move_to_func(
+            self._hb_drawfuncs, <hb_draw_move_to_func_t>ptr, <void*>user_data, NULL)
+
+    def set_line_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef uintptr_t ptr = <uintptr_t>ctypes.addressof(func)
+        hb_draw_funcs_set_line_to_func(
+            self._hb_drawfuncs, <hb_draw_line_to_func_t>ptr, <void*>user_data, NULL)
+
+    def set_cubic_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef uintptr_t ptr = <uintptr_t>ctypes.addressof(func)
+        hb_draw_funcs_set_cubic_to_func(
+            self._hb_drawfuncs, <hb_draw_cubic_to_func_t>ptr, <void*>user_data, NULL)
+
+    def set_quadratic_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef uintptr_t ptr = <uintptr_t>ctypes.addressof(func)
+        hb_draw_funcs_set_quadratic_to_func(
+            self._hb_drawfuncs, <hb_draw_quadratic_to_func_t>ptr, <void*>user_data, NULL)
+
+    def set_close_path_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef uintptr_t ptr = <uintptr_t>ctypes.addressof(func)
+        hb_draw_funcs_set_close_path_func(
+            self._hb_drawfuncs, <hb_draw_close_path_func_t>ptr, <void*>user_data, NULL)
+
 cdef class HBObject:
     cdef hb_object_t* _hb_obj_list
     cdef unsigned int _num

draw.py:

from uharfbuzz import DrawFuncs, Blob, Face, Font
import ctypes
import sys

lib = ctypes.cdll.LoadLibrary("draw.dylib")

funcs = DrawFuncs()
funcs.set_move_to_func_ctypes(lib.move_to)
funcs.set_line_to_func_ctypes(lib.line_to)
funcs.set_cubic_to_func_ctypes(lib.cubic_to)
funcs.set_close_path_func_ctypes(lib.close_path)

blob = Blob.from_file_path(sys.argv[1])
face = Face(blob)
font = Font(face)

funcs.get_glyph_shape(font, 0)

draw.c (compiled with cc -shared -o draw.dylib $(pkg-config --cflags harfbuzz) draw.c):

#include <stdlib.h>
#include <stdio.h>
#include <hb.h>

void move_to (hb_draw_funcs_t *dfuncs,
              void            *draw_data,
              hb_draw_state_t *st,
              float            to_x,
              float            to_y,
              void            *user_data)
{
  fprintf(stderr, "m: (%g, %g)\n", to_x, to_y);
}

void line_to (hb_draw_funcs_t *dfuncs,
              void            *draw_data,
              hb_draw_state_t *st,
              float            to_x,
              float            to_y,
              void            *user_data)
{
  fprintf(stderr, "l: (%g, %g)\n", to_x, to_y);
}


void cubic_to (hb_draw_funcs_t *dfuncs,
               void            *draw_data,
               hb_draw_state_t *st,
               float            to_x,
               float            to_y,
               float            control1_x,
               float            control1_y,
               float            control2_x,
               float            control2_y,
               void            *user_data)
{
  fprintf(stderr, "c: (%g, %g) (%g, %g) (%g, %g)\n",
          to_x, to_y, control1_x, control1_y, control2_x, control2_y);
}
void close_path (hb_draw_funcs_t *dfuncs,
                 void            *draw_data,
                 hb_draw_state_t *st,
                 void            *user_data)
{
  fprintf(stderr, "z\n");
}
$ python draw.py Amiri-Regular.ttf
Bus error: 10

@justvanrossum
Copy link
Owner Author

I'm trying, but I'm not getting further than you. I don't see an (obvious) error with your code.

@justvanrossum
Copy link
Owner Author

Ha, I got it! The function pointers need to be dereferenced:

diff --git a/src/uharfbuzz/_harfbuzz.pyx b/src/uharfbuzz/_harfbuzz.pyx
index 60dd6a2..f588288 100644
--- a/src/uharfbuzz/_harfbuzz.pyx
+++ b/src/uharfbuzz/_harfbuzz.pyx
@@ -1,9 +1,11 @@
 #cython: language_level=3
 import os
 import warnings
+import ctypes
 from enum import IntEnum
 from .charfbuzz cimport *
 from libc.stdlib cimport free, malloc, calloc
+from libc.stdint cimport uintptr_t
 from libc.string cimport const_char
 from collections import namedtuple
 from typing import Callable, Dict, List, Sequence, Tuple, Union
@@ -1085,6 +1087,32 @@ cdef class DrawFuncs:
         hb_draw_funcs_set_close_path_func(
             self._hb_drawfuncs, _close_path_func, <void*>user_data, NULL)
 
+    def set_move_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef void** ptr = <void**><size_t>ctypes.addressof(func)
+        hb_draw_funcs_set_move_to_func(
+            self._hb_drawfuncs, <hb_draw_move_to_func_t>ptr[0], <void*>user_data, NULL)
+
+    def set_line_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef void** ptr = <void**><size_t>ctypes.addressof(func)
+        hb_draw_funcs_set_line_to_func(
+            self._hb_drawfuncs, <hb_draw_line_to_func_t>ptr[0], <void*>user_data, NULL)
+
+    def set_cubic_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef void** ptr = <void**><size_t>ctypes.addressof(func)
+        hb_draw_funcs_set_cubic_to_func(
+            self._hb_drawfuncs, <hb_draw_cubic_to_func_t>ptr[0], <void*>user_data, NULL)
+
+    def set_quadratic_to_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef void** ptr = <void**><size_t>ctypes.addressof(func)
+        hb_draw_funcs_set_quadratic_to_func(
+            self._hb_drawfuncs, <hb_draw_quadratic_to_func_t>ptr[0], <void*>user_data, NULL)
+
+    def set_close_path_func_ctypes(self, func: object, user_data: object = None) -> None:
+        cdef void** ptr = <void**><size_t>ctypes.addressof(func)
+        hb_draw_funcs_set_close_path_func(
+            self._hb_drawfuncs, <hb_draw_close_path_func_t>ptr[0], <void*>user_data, NULL)
+
+
 cdef class HBObject:
     cdef hb_object_t* _hb_obj_list
     cdef unsigned int _num

I also fiddled with the cast, not sure to what extent that helped. Maybe there's a better way to write this.

@khaledhosny
Copy link
Contributor

This works indeed, I’ll experiment and see of there is nicer way to write it.

@justvanrossum
Copy link
Owner Author

justvanrossum commented Jul 26, 2022

Some notes so I don't forget:

  • objc objects have a method named __c_void_p__() which returns a ctypes pointer to the object. This works both for NSBezierPath and for CGPath. This is ideal to pass these object types to the ctypes-wrapped custom C lib.
  • The opposite is done with objc.objc_path(c_void_p=ctypespointer): this makes a Python wrapper for a ctypes pointer to an objc object.
  • The C lib is currently responsible for creating the path object, but I can easily see doing that in Python, and pass the initialized object to the C lib instead.
  • I would like FG to switch over to using CGPath instead of NSBezierPath, and I would like to be able to pass a CGPath-producer from FG to blackrenderer, so its CoreGraphics backend can benefit from fast path creation, too. (COLRv1 rendering is currently quite slow in FG). Perhaps this should be seen as separate from the FT vs HB issue, perhaps not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants