fabmax / physx-jni

Java JNI bindings for Nvidia PhysX
MIT License
85 stars 8 forks source link

Incompatibility with Java 9 Modules #51

Closed Frooastside closed 1 year ago

Frooastside commented 1 year ago

In my build.gradle file, i both included the base library and natives with

  implementation "de.fabmax:physx-jni:2.0.5"
  runtimeOnly "de.fabmax:physx-jni:2.0.5:natives-linux"

and when running just the initialization:

    int physicsVersion = PxTopLevelFunctions.getPHYSICS_VERSION();
    int physicsVersionMajor = physicsVersion >> 24;
    int physicsVersionMinor = (physicsVersion >> 16) & 0xff;
    int physicsVersionMicro = (physicsVersion >> 8) & 0xff;
    PxDefaultAllocator physicsAllocator = new PxDefaultAllocator();
    PxDefaultErrorCallback physicsErrorCallback = new PxDefaultErrorCallback();
    PxFoundation physicsFoundation = PxTopLevelFunctions.CreateFoundation(physicsVersion, physicsAllocator, physicsErrorCallback);
    PxTolerancesScale physicsTolerances = new PxTolerancesScale();
    PxPhysics physics = PxTopLevelFunctions.CreatePhysics(physicsVersion, physicsFoundation, physicsTolerances);
    System.out.printf("PhysX loaded, version: %d.%d.%d%n", physicsVersionMajor, physicsVersionMinor, physicsVersionMicro);
    int amountOfThreadsForPhysics = Math.max(1, (int) (Runtime.getRuntime().availableProcessors() / 2f));
    PxDefaultCpuDispatcher physicsCpuDispatcher = PxTopLevelFunctions.DefaultCpuDispatcherCreate(amountOfThreadsForPhysics);
    System.out.printf("PhysX CPU Dispatcher created, threads: %d%n", amountOfThreadsForPhysics);

I get the error Caused by: java.lang.IllegalStateException: Failed loading native PhysX libraries for platform LINUX and the cause of that is Caused by: java.lang.ClassNotFoundException: de.fabmax.physxjni.NativeMetaLinux. I think this is because of the native library not having a module-info.class. I am also not able to manually import the NativeMetaLinux class due to that.

my module-info.class looks like that

  ...

  requires de.fabmax.physxjni;

full error log:

Exception in thread "main" java.lang.ExceptionInInitializerError
    at <CENSORED>.main(<CENSORED>.java:52)
Caused by: java.lang.IllegalStateException: Failed loading native PhysX libraries for platform LINUX
    at de.fabmax.physxjni/de.fabmax.physxjni.Loader.load(Loader.java:31)
    at de.fabmax.physxjni/physx.NativeObject.<clinit>(NativeObject.java:7)
    ... 1 more
Caused by: java.lang.ClassNotFoundException: de.fabmax.physxjni.NativeMetaLinux
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
    at de.fabmax.physxjni/de.fabmax.physxjni.Platform.getMeta(Platform.java:17)
    at de.fabmax.physxjni/de.fabmax.physxjni.Loader.load(Loader.java:22)
    ... 2 more
Frooastside commented 1 year ago

I just confirmed that it only occurs, if I use a module-info and the issue is not based on different problems

Frooastside commented 1 year ago

I also noticed that the physx.vehicle2 package is not included in the module-info

Frooastside commented 1 year ago

Also as it is not possible to declare the same package in separate modules (de.fabmax.physicsjni where the NativeMeta classes are located), I thought of putting them in a sub directory like de.fabmax.physicsjni.linux.NativeMetaLinux. Also, is it actually necessary to have that class separate, loaded by Reflection? Because I would like to be able to load the libraries (.so, .dylib, .dll) from an arbitrary location and not from the class path if that would be possible.

fabmax commented 1 year ago

Ah yes, I'm not using java modules myself so I haven't noticed that this actually isn't working, but I was able reproduced your problem.

Putting the native classes in separate packages / modules should not be a problem but I will keep the reflection based loading. I think that is the easiest method to maintain drop-in compatibility between the regular and cuda versions and also to check if the versions of the main lib and the native lib match.

However, it should already be possible to specify a custom path by setting the system properties physx.nativeLibLocation=/path/to/libraries/ and physx.skipHashCheck=true (I will probably change these in the next version to something a bit more intuitive).

Frooastside commented 1 year ago

Ok thank you, I noticed the code for physx.nativeLibLocation to but didn't realize that it never actually tried to use that because of the different error.

Doesn't this code require it to be in the class path, like even if it allows me to load it from an arbitrary location, isn't it still required to be there?

    private static void loadLibsFromResources(List<String> libResourceNames) throws IOException {
        ...

        for (String libResource : libResourceNames) {
            InputStream libIn = Loader.class.getClassLoader().getResourceAsStream(libResource);
            if (libIn == null) {
                throw new IllegalStateException("Failed loading " + libResource + " from resources");
            }
fabmax commented 1 year ago

Yes you are right, nevermind 😃

Anyway, now it should actually work by setting physxjni.loadFromResources=false and specifying the library path with physxjni.nativeLibLocation=path/to/libraries/ (see here). The module stuff should be fixed as well.

I published a new snapshot containing the changes: https://oss.sonatype.org/content/repositories/snapshots/de/fabmax/physx-jni/2.0.6-SNAPSHOT/

Frooastside commented 1 year ago

Works now, thank you. Some thing to remember is that to include it in the module-info it needs to be implementation instead of runtimeOnly or it will not be recognizable and they always need to be all in the gradle configuration at the same time but if you intend to bundle it and only include the necessary binaries, these few extra NativeLib classes are not a problem. But I also noticed that the libraries are now duplicated. For me that's not really a problem just wanted to mention it, because it's probably not intended.

Screenshot

fabmax commented 1 year ago

Some thing to remember is that to include it in the module-info it needs to be implementation instead of runtimeOnly

Yes I noticed that as well. Apparently there is no requiresRuntime or something like that in the module-info.

But I also noticed that the libraries are now duplicated.

Thanks, I missed that. Will remove it with the next build.

Frooastside commented 1 year ago

I think i will close this now, as my main issue is resolved, thanks a lot!