JetBrains / skija

Java bindings for Skia
Apache License 2.0
2.63k stars 127 forks source link

jigsaw support for natives resource loading #110

Closed comtel2000 closed 3 years ago

comtel2000 commented 3 years ago

I tried to run skija in a modular maven project. But the native resources (.dll, .so) can't be reached from the shared lib via

URL url = Library.class.getResource(resourcePath + fileName); `

The Library.class is part of the skia-shared-x.x.x.jar and the native plattform libs are separated into skia-${platform}-x.x.x.jar

It is necessary to manually extract the native files to the root of the project or to the src.main.resources folder.

A workaround could be to add a empty class file 'Native.class' into each skia-${platform}-x.x.x.jar and add a load from resource method into Library::_extract(..) like:

Native.class.getResourceAsStream(resourcePath + fileName);
tonsky commented 3 years ago

Interesting! Thanks for reporting. I’ll see what I can do

comtel2000 commented 3 years ago

I’m not an expert with python and how to add a compiled class, but I can create a PR for the Library.class changes :)

tonsky commented 3 years ago

That would be great, the only suggestion I have is that I don’t want to have same class in all three jars. The idea was that if your build system is very simple you can just add all three native jars and the only one will be used by runtime. But if all three will have same class it might lead to conflict.

So maybe LibraryMacosX64, LibraryMacosArm64, LibraryWindows, LibraryLinux? And then Class.forName("Library" + platform).getResourceAsStream()?

I am also curious how to reproduce your configuration?

And final question: is there a way to make two jars into the same module somehow?

comtel2000 commented 3 years ago

That would be great, the only suggestion I have is that I don’t want to have same class in all three jars. The idea was that if your build system is very simple you can just add all three native jars and the only one will be used by runtime. But if all three will have same class it might lead to conflict.

So maybe LibraryMacosX64, LibraryMacosArm64, LibraryWindows, LibraryLinux? And then Class.forName("Library" + platform).getResourceAsStream()?

yes, totally aggree.

I am also curious how to reproduce your configuration?

I will push my sample project to github and link it here asap

And final question: is there a way to make two jars into the same module somehow?

with jigsaw, each none modularized jar becomes a "none named" auto module: module A (skia.shared) and a second module b (like: skia.windows)

As I know best practice for platform depependcies is to work with a placeholder jar (the way like openjfx/gluon handle it) The skia.shared becomes an empty placeholder jar and all shared java classes are moved/copied to the native platform jar's. Now, everything is inside one jar (module) file and can be used internally without access restrictions.

comtel2000 commented 3 years ago

here is my test configuration: modularized (native files added to root): https://github.com/comtel2000/lwgl-skia

none modularized (natives can be extracted from jar): https://github.com/comtel2000/jogl-skia

comtel2000 commented 3 years ago

here is a good sample for handling resources with the module classpath: https://github.com/codetojoy/easter_eggs_for_java_9/tree/master/egg_31_stack_overflow_45166757

what is your minimal supported java version for skija?

tonsky commented 3 years ago

what is your minimal supported java version for skija?

Currently 9

comtel2000 commented 3 years ago

some classes uses java10+ features (var) like here:

var textUtf8 = new ManagedString(text);

This can may be switch to lombok compiler magic or just use java11 as defined in the pom.xml?

I also tested a more general solution for the native dependencies. That branch switch to a single module/jar (shared + native) via maven classifier with auto select native platform files by maven. https://github.com/comtel2000/skija/tree/skija-mvn-parent

Unfortunately, this would kill backwards compatibility.

tonsky commented 3 years ago

The skia.shared becomes an empty placeholder jar and all shared java classes are moved/copied to the native platform jar's.

Hm, this approach would cause class duplication if I put all platforms together on a classpath (we actually have a use-case like this in Idea, where build system is not very configurable)

some classes uses java10+ features

I’ve never checked, should be easy to get rid of those.

That branch switch to a single module/jar

As I understand, it downloads zip file and unpacks it somewhere during build? This makes too much assumtions about how this will be used (maven, wget, build dir). I prefer plain old jars, they are universal, work everywhere and the most basic Java tools (javac, java) understand them.

I’m gonna merge #111 now in a branch and finish it up (tests, java 9, maybe some folder reorganization). Thanks for the help!

comtel2000 commented 3 years ago

The skia.shared becomes an empty placeholder jar and all shared java classes are moved/copied to the native platform jar's.

Hm, this approach would cause class duplication if I put all platforms together on a classpath (we actually have a use-case like this in Idea, where build system is not very configurable)

some classes uses java10+ features

I’ve never checked, should be easy to get rid of those.

That branch switch to a single module/jar

As I understand, it downloads zip file and unpacks it somewhere during build? This makes too much assumtions about how this will be used (maven, wget, build dir). I prefer plain old jars, they are universal, work everywhere and the most basic Java tools (javac, java) understand them.

skija-mvn-parent builds also plain jars. The native maven pom uses wget only to extract binary sources (same functionality as in the python build scripts just moved to maven profiles <- caused by my limited python knowledge). The main disadvantage is the code duplication for the 'all platforms together' approach.

I’m gonna merge #111 now in a branch and finish it up (tests, java 9, maybe some folder reorganization). Thanks for the help!

ok, let me know if I can help with testing

tonsky commented 3 years ago

skija-mvn-parent builds also plain jars

Oh, then I didn’t understand what you did. I noticed you renamed skija-shared to skija, and wget. Probably caused by my limited knowledge of Maven :)

ok, let me know if I can help with testing

Absolutely!

tonsky commented 3 years ago

Ok, I did some code reorganization and updated build scripts. There’s no published jars yet, but can you try that these work for you?

Check out modules branch: https://github.com/JetBrains/skija/commit/854d9975731160836b9cd9f3e3149f41bee9786a

Run

python3 script/deploy_shared.py --dry-run
python3 script/deploy_native.py --dry-run

It should produce necessary jars in shared/target/skija-shared-0.92.8.jar and platform/target/skija-windows-0.92.8.jar

I understand you are on Windows, unfortunately I don’t have a Windows machine atm to produce those for you.

You can check that tests work:

python3 script/test.py

and that examples run:

python3 examples/lwjgl/script/run.py
comtel2000 commented 3 years ago

perfect! I compiled and tested it with windows and macos-x64 without any problems. 👍

tonsky commented 3 years ago

Published 0.92.2. Thanks for the PR!