Bindings are a bit outdated #181
Conversation
|
Thanks for bringing this up. Updating internal skia involves a lot of work other than adapting the binding implementation. The general steps are:
The documentation part is actually the hardest, as the change in documentation is hard to identify, and the current docstring is hard-coded for each API. I would request completing the above for this PR to continue. |
|
Y, I also realized that (your, quite nice) documentation-style is a lot of effort. On the other hand, the I could work overtime "a bit" on that. But to be honest, I don't have the manpower/time Concerning the build: This also changed a bit. You have to call some batch-file on windows. |
|
Suggestion: Branch the repo and work together on this with more than a single pull request. |
|
Alright, I would set up a development branch. Is there any specific skia branch you are considering? If no, the latest milestone m98 would be the target. |
|
If I'm not mistaken, it's using m97 here. m98 sounds good. |
|
@0lru I just created the The CI failure is a bit different issue and might be better to fix in another PR or branch. I understand splitting skia build would make CI efficient. This python package depends on |
|
Commented in #192 (comment) , I need m103+ 😞 |
|
I think this pull is rather unsatisfactory, actually - it goes by disabling a hell lot of api's, then gradually re-enabling them. I have tried a different approach - not doing skia build myself but just reuse the pre-built headers + libraries from the jetbrain folks, and see how far I get. Trying m110 and things were breaking left right and center. So I backtracked and just do the next one - m88 is when the SVGDOM class become non-experimental so the header moved. The diff between m87 and m88 is close to 1000 lines! I haven't bothered with updating the docstring, just looking at Skia API changes and updating the binding. That's already a whole day's work. Would be a bit painful to do the same 20 times to get to m108. (Ideally I need m103 at least) |
|
@HinTak As you've noticed, this repository is unsuitable for keeping up with the latest Skia development. Presumably, everything should be automatically generated including the docstring via AST, although I don't have any time and resources to do that. |
|
@kyamagu strangely enough, now that I can build current skia as a shared library in about 40 minutes (https://github.com/HinTak/skia-building-fun), and having filed an issue with skia and got some response from skia developers about symbol resolution between the core and optional modules like the SVG one, I am convinced that @0lru did the right thing. The thing is: (1) skia developers actually mark some routines / classes as So I think actually removing a lot of material (accessing private symbols not part of SK_API) in current skia-python is the correct approach, actually. The problem is that skia python has been around for too long and people are getting used to coding in certain way... anyway, I have a m116 fork of skia-python with a lot of material removed, so it probably is terribly broken for practical use, but I intend to add back stuff as I need it, and hope that eventually it is good as a replacement and you can merge it. So I have a m116 fork that builds. |
|
|
|
My m116 fork gets a bit better now - 115 failed, 1972 passed, 34 skipped, 8 xfailed, 66 errors. Stock currrent m87 on my computer, 2 failed, 2169 passed, 28 skipped, 8 xfailed. So My passes about 90% of tests, the non-passed ones are half failed half error. There is a small complication: when routines have similar purpose but done internally differently, do you keep the old name, switch to new name, or both? I also found skia-python touches/covers about 1000 of the 2400 routines; about 200 of them changed between m87 & m116. :-(. |
|
Anyway, I intend to put a fork out soon, and put it to "production" use, while fixing the rest. At the moment it passes 90% of tests, so it probably is useful to most already. |
|
@HinTak Great!
Any example? |
|
The whole image IO - "encodeToData" is gone. It is recommended to use the actual individual graphic format encoder api directly. Canvas.drawBitmap / drawBitmapRect are gone. They re-implemented with drawImage/drawImageRect in my fork... but I wonder how much effort should I go towards "emulating" old APIs. |
|
@HinTak Got it. I would say those breaking changes should be reflected in the Python API and documented in the release note. In other words, keep a record of which API is gone. Anyway, this should be a huge effort. Great thanks! |
|
I'd like to set up CI on that pull (and possibly on my detached mirror too). Is there any help/instruction somewhere on that? |
|
@HinTak I've invited you as a collaborator. Let me see if there is any setup to enable CI |
|
I think I have incorporated all the changes that @0lru did (it is useful to see somebody else's work going half of the way), except I have decided not to expose all the interiors of SamplingOptions - in my m116 fork only the bare no-arg constructor is exposed. My idea is that we shouldn't add API which might change in the future and nobody actually uses :-). And get into a nightmare of having to update it as upstream moves! |
|
If somebody wants to play with the inside of SamplingOptions, they should supply an example use and a test case. |
Overwriting with https://github.com/kyamagu/skia-python/raw/83463dfbae671e14341c6d2be2d5ee663ab58e00/src/skia/SamplingOptions.cpp from kyamagu#181 and djusted for already-committed code
The bindings weren't touched for some time. I've updated these to the latest version.
Two major things changed in the API:
I'm compiling these bindings by myself. I mean, this merge request does not contain "clean code"
(please don't just merge it). but could be the basis for the next version.
Using it here already: https://github.com/0lru/p3ui with d3d12 (d3d12 is also the reason for the update)