Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom name for JNI_OnLoad #34

Closed
nadrolinux opened this issue Jan 10, 2022 · 11 comments
Closed

Custom name for JNI_OnLoad #34

nadrolinux opened this issue Jan 10, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@nadrolinux
Copy link

nadrolinux commented Jan 10, 2022

Hello,
I'm not sure if this functionality doesn't exist in Scapix, but I couldn't find it, so thats why I write this message. In our project we have a lot of JNI code and we can't replace all of them by Scapix in an easy way, but we want to use Scapix for new screens. At this point I found a problem related to JNI_OnLoad. It would be nice to tell a Scapix to generate 'JNI_OnLoad' with a custom name and separate it to *.h and *.cpp file if custom name is provided eg:

[H]
jint customname_JNI_OnLoad(JavaVM* pVM, void* pReserved);
[CPP]
jint customname_JNI_OnLoad(JavaVM* pVM, void* pReserved)
{
    // Scapix code
}

With this feature we will be able to just execute 'customname_JNI_OnLoad' from our 'JNI_OnLoad'. What do you think about this feature?
Patryk,

@Boris-Rasin Boris-Rasin added the enhancement New feature or request label Jan 10, 2022
@Boris-Rasin
Copy link
Member

Sounds good.
Why would you need a custom name, why not always use scapix_JNI_OnLoad()?

When you specify in CMake set(SCAPIX_CUSTOM_JNI_ONLOAD ON) you have to define JNI_OnLoad() and call scapix_JNI_OnLoad() yourself.

@nadrolinux
Copy link
Author

nadrolinux commented Jan 11, 2022

You're right fully flexible custom name isn't required. A static-custom name like, a scapix_JNI_OnLoad() + simple boolean ON/OFF like at set(SCAPIX_CUSTOM_JNI_ONLOAD ON) will be 100% enough for our problem.

At now, as a temporary workaround I added a step for a build.gradle (replace text in a file) to replace JNI_OnLoad to scapix_JNI_OnLoad() after a successful build. Not the prettiest solution, but works until official support will be available.

@nadrolinux
Copy link
Author

nadrolinux commented Jan 12, 2022

Small update from me. Maybe static-custom name is not enough. In my project I have separate library builded with Scapix (JNI_OnLoad is generated) and application project which also require scapix_bridge_headers (second JNI_OnLoad is generated) to build properly, even If I don't generate any bridges at this step, because I use bridges generated in mentioned, separate library. At now I replace JNI_OnLoad from my library to scapix_JNI_OnLoad and JNI_OnLoad from my application to dummy_JNI_OnLoad and never use it. Of course the best option would be to use Scapix in application project just as dependency (headers + required libs for a linker) instead of use scapix_bridge_headers, because as I mentioned I don't generate bridges at this step, but I didn't find any option to do that. Of course I could copy some stuff from Scapix CMakeLists to my CMakeLists to emulate "dependency" bahaviour, but I'm not too experienced enough at CMake topic to do that in relative quick time.

@Boris-Rasin
Copy link
Member

Boris-Rasin commented Jan 12, 2022

To use Scapix (headers+lib) as a dependency:

find_package(Scapix REQUIRED)
target_link_libraries(example PRIVATE scapix)

It may be better to do it the other way around: your static library could use target_link_libraries(scapix) and your shared library (application) could use scapix_bridge_headers() specifying both it's own (if any) and static library headers to bridge.

Here is a small example project, doing exactly that:
double_static.zip

Right now, you are supposed to call scapix_bridge_headers() only once per shared library (application), irrespective of how many static libraries are linked into it (but you can pass headers to bridge from all dependent static libraries).

I will consider supporting separate scapix_bridge_headers() calls for multiple dependent static libraries. This may indeed require different names for JNI_OnLoad() functions, like lib1_scapix_JNI_OnLoad(), lib2_scapix_JNI_OnLoad(), etc.

@nadrolinux
Copy link
Author

nadrolinux commented Jan 12, 2022

Thanks a lot for these tips. I applied those changes to a project and I'm really enjoyed with final results. Both projects (library + application) looks clean and works properly.

BTW. I see that with -DSCAPIX_BRIDGE=java all bridges (java, c#, js, python, objc) are generated. For me it's not a problem, but maybe it's a bug, so I report it. If not, just ignore that. Second, more important thing. If we have few C++ classes in one file Scapix generate those classes as package-private, thats why to use all of them we have to use the same package and sometimes it's not possible. I think that solution for this issue may be a following: if C++ file contains 2 classes wrapped in namespaces eg:

[Source.cpp]
namespace x
{
    class A

    namespace y
    {
        class B
    }
}

class C

Scapix could generate a something like that (in short, public static classes with prefixes for a namespaces):

[Source.java] - file name exactly like now
public class Source
{
    static public class x_A
    static public class x_y_B
    static public class C
}

In this way we don't need to match a package name. This feature will greatly improve integration of Scapix into relative big, existing projects like mine. At now I can use one matched package in my project, but in a future I would need a 2-3 more packages.

Anyway after a first, working integration in Android part I can say that Scapix works really good! Thanks again! At now I just need to finish iOS integration part.

@Boris-Rasin
Copy link
Member

Perhaps a better approach would be to generate each Java class in it's own source file, so they can all be public?

@nadrolinux
Copy link
Author

This will be also a good solution. Yes, maybe even better than mine, however if I'm not wrong (just imagination, because I didn't looked at Scapix sources too much) it will require little more work to do.

@nadrolinux
Copy link
Author

Hello,
How looks a situation with this issue?
Patryk

@Boris-Rasin
Copy link
Member

Added SCAPIX_CUSTOM_JNI_ONLOAD feature:

By default Scapix generates JNI_OnLoad() function for target library.
You can also use your own JNI_OnLoad(), which then needs to call scapix_JNI_OnLoad().

target_compile_definitions(scapix PUBLIC SCAPIX_CUSTOM_JNI_ONLOAD)
#include <scapix/bridge/java/on_load.h>

JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved)
{
	// ... your additional JNI init code ...

	return scapix_JNI_OnLoad(vm, reserved);
}

@nadrolinux
Copy link
Author

nadrolinux commented May 2, 2022

Great, thanks a lot. I tested this stuff and it works without problems. It looks like this issue may be closed. We talked here about namespaces and private-package problem, but it may be better to shift that into another thread or just not start issue for that, because as I understand you agreed that this feature will be good and you will add support for it in future, so "official" ticket is not important too much. Namespaces handling (eg. as prefixes glued with underscores will be useful for other languages like a obj-c too).

@Boris-Rasin
Copy link
Member

Opened a separate issue for generating Java classes public:
#40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants