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

Add support for using system skia #257

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jopejoe1
Copy link

I'm currently working on packaging skia-python for NixOS and i want to compile it against our version of skia. This patch makes it possible for me to-do that.

@HinTak
Copy link
Collaborator

HinTak commented Jul 31, 2024

No, it does not. I actually do this kind of thing for myself while doing the monthly pulls, and the static libraries needs to be compiled with -rtti, etc. Otherwise it will build and link, but show runtime errors at load time. Convince yourself by actually running your copy of skia-python built your way.

FWIW, my build script to do it separately, and the monthly releases of shared and static libraries, are at:
https://github.com/HinTak/skia-building-fun

@HinTak
Copy link
Collaborator

HinTak commented Jul 31, 2024

And why are you modifying the non-NixOS code, also? You don't need to touch the apple or Windows code, really.

@HinTak
Copy link
Collaborator

HinTak commented Jul 31, 2024

Just launch python, (into the REPL prompt), then type import skia and see if you can load it.

@HinTak
Copy link
Collaborator

HinTak commented Jul 31, 2024

I'll let this go through CI, but I am not convinced it will pass... anyway, let's see.

@HinTak
Copy link
Collaborator

HinTak commented Jul 31, 2024

You'll need a bit more change to setup.py if you use your own libskia, since it inter-depends on freetype / harfbuzz /libjpeg which we bundled in the public wheels. I have the local diff in my private repo, but since it isn't a config usable by a random person who doesn't know how they interdepend, I haven't posted it, nor ever intend to. Anyway, give import skia a try and see if it can convince you not to do this.

@jopejoe1
Copy link
Author

I mainly build it as a dependencies for building noto-font (which did build after this patch), but i did not test it standalone, but doing the import like you said indeed fails with an import error.

ImportError: /nix/store/kc6api80a4zxv2bqyxxk7kxbc9qwsva1-skia-124-unstable-2024-05-22/lib/libsvg.so: undefined symbol: _Z11SkStrSearchPKPKciS0_m

And why are you modifying the non-NixOS code, also? You don't need to touch the apple or Windows code, really.

We also have MacOS support and are working on getting windows support (very early states (mainly cross))

Will need to do a more in dpeth look into this will draft the pr till then.

@jopejoe1 jopejoe1 marked this pull request as draft July 31, 2024 22:52
@HinTak
Copy link
Collaborator

HinTak commented Aug 1, 2024

import skia (or equivalent from skia import ...) is the first line of any python script using skia-python... so if it doesn't work, it means you mis-built it :-). It is a lesson I learned last summer.

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