kyamagu / skia-python

Python binding to Skia Graphics Library
https://kyamagu.github.io/skia-python/
BSD 3-Clause "New" or "Revised" License
245 stars 43 forks source link

Skparagraph on windows, and icudtl.dat #268

Closed HinTak closed 4 days ago

HinTak commented 1 month ago

The icu module fails to load - looking online it seems to be a generic problem across skia on windows, with rust , c/c++, and java:

https://github.com/rust-skia/rust-skia/blob/master/skia-safe/README.md#embed-icudtl-enabled-by-default

https://groups.google.com/g/skia-discuss/c/FpL3Q0XK8Ag

https://stackoverflow.com/questions/60048968/build-errors-with-ninja-and-skia-unable-to-find-icudtl-dat-even-though-the-file

HinTak commented 1 month ago

I set up my skia-python-example ci to run the skparagraph example directly on windows, and got a direct message:

Run python skparagraph-example.py
  python skparagraph-example.py
  shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
SkIcuLoader: datafile missing: C:\hostedtoolcache\windows\Python\3.9.13\x64\icudtl.dat.
SkIcuLoader: datafile missing: C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\site-packages\icudtl.dat.
D:\a\skia-python\skia-python\skia\modules\skparagraph\src\ParagraphBuilderImpl.cpp([2](https://github.com/HinTak/skia-python-examples/actions/runs/11003814842/job/30553585753#step:5:2)52): fatal error: "check(fUnicode)"
Error: Process completed with exit code 1.
HinTak commented 1 month ago

Tested successfully copying icudtl.dat : https://github.com/HinTak/skia-python-examples/commit/6ff966825e0cd3c21fe7e013c4d68c186ac07ead

Basically a one-liner:

python -c 'import shutil, site; shutil.copy2("icudtl.dat", site.getsitepackages()[0])'

Where 'icudtl.dat' is unpacked from https://github.com/unicode-org/icu/releases/download/release-75-1/icu4c-75_1-data-bin-l.zip and renamed.

HinTak commented 1 month ago

Okay the programmatic way of download, unzip and copy with rename is (see testing in https://github.com/HinTak/skia-python-examples/blob/main/.github/workflows/windows.yml):

python -c 'import urllib.request; urllib.request.urlretrieve("https://github.com/unicode-org/icu/releases/download/release-75-1/icu4c-75_1-data-bin-l.zip", "icudatal.zip")'
unzip icudatal.zip
python -c 'import os, shutil, site; shutil.copy2("icudt75l.dat", os.path.join(site.getsitepackages()[0], "icudtl.dat"))'

I guess we can insert this as CIBW_BEFORE_BUILD_WINDOWS , for testing purpose, but this still leaving windows users having to do the same thing on his computer.

HinTak commented 1 month ago

Adding a "good first issue" label, as there is a clear answer, just somebody needs to figure out how to do it neatly..., either in CI or for user download, or both.

HinTak commented 1 month ago

Hmm, the rust-skia solution seems horrible - they patch skia itself to load from alternative location, as well as putting/bundling the icudtl.dat into a rust location at build time. (If I understand the pull that solves the icudtl not found issue in rust-skia)

HinTak commented 1 month ago

Hmm, the warning is seen as early as 126b8.

HinTak commented 4 days ago

279 merged.