kyamagu
Skparagraph binding
Binding skparagraph and adding text decoration. Fixes #225 and #224
@kyamagu this is operational in running https://github.com/HinTak/skia-python-examples/blob/main/shape_text.py
which is a python port of upstream's
https://github.com/google/skia/blob/main/example/external_client/src/shape_text.cpp
Transplanted to build standalone against libskia as a shared library in
https://github.com/HinTak/skia-c-examples/blob/main/shape_text.cpp
Important note: https://issues.skia.org/358587937 and its windows equivalent https://issues.skia.org/307357528
- basically NONE of skparagraph is meant to be exposed outside of google's family of software, let alone stable API!
And other issues in https://github.com/HinTak/skia-building-fun/blob/main/ReportedIssues.md
Missing a lot of documentations and tests, but operational - in running that particular example. Since this is google's own example, we can at least hope that if they break it, they'll update the example and points towards a modification/update of the example code too.
The skia-python example gives output which are pixels identical to the c++ example, if you take into account of how they load the same font differently. I haven't figured out how to use skparagraph with an exact font file, but that's a limitation of my python knowledge about sub-class'ing, rather than this pull. We might be able to get around it by adding a python FontMgr.OneFontMgr(filename) class static method to do the equivalent of the c++ subclassing on this line : https://github.com/HinTak/skia-c-examples/blob/15b3b2b9381df682fb84b1605cc447af88d46c3f/shape_text.cpp#L63 to give python code the ability of the c++ example of using a specific font file.
@kyamagu how do you feel about indentation, regarding the "class OneFontStyleSet and OneFontMgr" code copied from upstream? If you feel we should re-indent it to ours, I'll do that. I did a look around, trying to make a python class deriving from a c++ class and being able to pass it back to c++ which takes the derived class is hard, and needs some helper c++ code anyway. So I thought it is probably simpler just to offer that as a static method which returns the derived class "OneFontMgr". Usage as in the newly added tests.
The example is called shaped_text, and is able to do multi-line paragraphs in Arabic correctly.
Looks good to me. No need to care too much about indentation if that complicates code migration from the upstream.
Hi, I'm trying to implement a text to image function using skia-python.
Here is a demo I wrote with rust-skia: https://gist.github.com/MeetWq/c56635fbf886c2c511c2ac1ec8e0aa48
It seems to be some features missing in the current skparagraph binding.
The main ones are ParagraphBuilder's pushStyle and pop functions to realize multiple TextStyle in a Paragraph; and Paragraph's properties like width, height, longestLine, etc.
Hope these can be added in the skparagraph binding.
@MeetWq thanks for the interesting demo. Yes, the current skparagraph binding in skia-python is mainly geared towards it being sufficient to run google's own example - this is somewhat intentional, as none of skparagraph is declared API stable nor documented ( https://issues.skia.org/358587937 , https://issues.skia.org/307357528 ). That said, I'll add those bits you mentioned in the next round of updates. I am concerned about using skparagraph bits which are still subjected to change though, so I think the rust-skia people are perhaps somewhat reckless in providing too much of skparagraph, without asking upstream for clarification (I.e basically filing or commenting on https://issues.skia.org/358587937 and https://issues.skia.org/307357528 and equivalents.
What I mean is that, while I'll add those, I would also update those issues upstream to push upstream to doing something about formally declaring those API as public symbols and etc.
@MeetWq I added all the bits, and also ported your code to skia-python: https://github.com/HinTak/skia-python-examples/blob/main/skparagraph-example.py
Took me some effort to match your output (mainly unsetting my FC_LANG="zh-TW" and set it to FC_LANG=en temporarily):
Just saving you the trouble of eye-balling mine against yours, this is from ImageMagick's compare - You can see mine is pixel-level identical to yours, even the Arabic section, until it gets to the Devanagari section and the Color Emoji:
FWIW, if I keep my default FC_LANG="zh-TW", this is the result - you can see all the non-English sections are broken, because it overwhelmingly prefer a chinese font:
So this code is hightly sensitive to LANG/FC_LANG. I did try a pure Arabic example earlier, and it did the correct thing (see my comment above; it was able to do Arabic run-directions, shaping and line breaks), so it is probably a case it get confused if there are more than one language, too.
@MeetWq A few items from the rust->python porting:
- You may want to report to the rust-skia people - the 2-arg constructor ParagraphBuilder.make is marked
SK_DISABLE_LEGACY_PARAGRAPH_UNICODEin the upstream header (i.e. indicating it is scheduled for removal later). Hence I am not adding it to skia-python, and just change the code to use the 3-arg constructor. cloneForPlaceholder/clone,style_underline.setDecoration*,paragraph.paint(canvas, x, y)/paragraph.paint(canvas, Point()), I choose to stay close to upstream or the equivalent c++ code, the rust-people seems to want to think different. I kind of see their point, so I might put some rust-like alias in in the future.- I checked upstream,
TextDecorationMode.kThroughis the default; that rust seems to default tokGaps. although it is nice (and I agree it is an improvement), you might want to ask the rust-skia people to properly document it as a "different default behavior from upstream", because it is surprising to people who are familiar to skia via a different binding, or actually use skia directly in c/c++. - I learned something from your code
style.setForegroundPaintis preferred overstyle.setForegroundColor(which I added earlier, but marked DEPRECATED; this is used by the google example!).
I think from the first item and the last item, you can see why I am cautious about binding too much of skparagraph, which is itself a changing target.
@kyamagu m130 is out, and it is quite small. I intend to pull #265 (and therefore also #260) and #256 into this, and do the README.m129 / m130 here (and mention this as a m130 change although it was post m128 against m128). Do you want to review this pre-pull of those, or post? Or doesn't matter? The pulls are all updating separate area of code, so it is fairly obvious which is which, even if I join all of them together before review.
Still missing are some tests from this. Upstream hasn't changed much documentation wise.
@HinTak It's good m130 update is small. At a glance, the code change looks good to me. It doesn't matter whether to review before or after the pull. Thanks!
@kyamagu ready to go - as usual, read Readme.m130 first is probably a good idea. This is inclusive of #265 , #260 and #256.
Okay, test_paragraph.py failed on windows. I suspect it is the icu code which isn't used on windows; so let's just skip it for windows and see whether test_unicode.py fails too.
It appears that on windows, icudtl.dat needs to be copied to a load path. There are fairly recent reports about using skparagraph on windows (with rust or c#, or just c) that needs this.
Apparently the canonical place to get icudtl.dat is from ihttps://github.com/unicode-org/icu/releases , one of the data-bin-{l,b}.zip files. L/B for little / big endian, and it is versioned.
Okay, the failure on test_unicode.py is clearer - it just fails to construct. This looks to be a generic skia problem and has been reported against other users of skparagraph on windows quite recently (flutter, rust-skia, skia-sharp), and they seems to suggest people to get the icudtl.dat file from somewhere. rust-skia seems to have an answer of embedding the file within rust-skia. For now I am just going to put an additional paragraph in readme.m130 , and just skip the tests on windows. @kyamagu