libgdx / gdx-jnigen

jnigen is a small library that can be used with or without libGDX which allows C/C++ code to be written inline with Java source code.
Apache License 2.0
59 stars 26 forks source link

feat: Introduce RISC-V support #48

Closed SimonIT closed 8 months ago

SimonIT commented 10 months ago

Not tested yet, love to hear some comments

EDIT: Works flawless

Frosty-J commented 10 months ago

Looks alright, though I won't be able to test it with the hardware I currently have. I might get a RISC-V SBC at some point, though don't see much reason to get one over ARM at this time.

I wouldn't worry about 32-bit RISC-V (but worth supporting if it's trivial to). The is64Bit boolean isn't very ready for 128-bit RISC-V, but I don't know if any 128-bit hardware even exists yet (it's in the specification).

While I think about it, there's a lot of natives downloaded and bundled into libGDX game JARs (x86, x64, arm32, arm64... potentially RISC-V in future – Windows, macOS, Linux) – maybe an opt-in solution could be realised for that? E.g. if someone is only interested in 64-bit Windows (not unheard of for Steam titles), don't bother with the rest. But this is probably a discussion for elsewhere, and not the most important thing in the world.

SimonIT commented 10 months ago

It works🎉

➜  gdx-jnigen git:(riscv) ✗ ./gradlew build
> Task :gdx-jnigen:compileJava UP-TO-DATE
> Task :gdx-jnigen:processResources UP-TO-DATE
> Task :gdx-jnigen:classes UP-TO-DATE
> Task :gdx-jnigen:jar UP-TO-DATE
> Task :gdx-jnigen:javadoc UP-TO-DATE
> Task :gdx-jnigen:javadocJar UP-TO-DATE
> Task :gdx-jnigen:sourcesJar UP-TO-DATE
> Task :gdx-jnigen:assemble UP-TO-DATE
> Task :gdx-jnigen-loader:compileJava UP-TO-DATE
> Task :gdx-jnigen:compileTestJava UP-TO-DATE
> Task :gdx-jnigen:processTestResources NO-SOURCE
> Task :gdx-jnigen:testClasses UP-TO-DATE
> Task :gdx-jnigen-loader:processResources NO-SOURCE
> Task :gdx-jnigen-loader:classes UP-TO-DATE
> Task :gdx-jnigen-loader:jar UP-TO-DATE
> Task :gdx-jnigen:test

boolean: true
byte: 1
char: 
short: 3
int: 4
long: %
float: 6.000000
double: 0
byteBuffer: 8
bool[0]: false
char[0]:
short[0]: 10
int[0]: 11
long[0]: %
float[0]: 13.000000
double[0]: 14.000000
string: Hurray fuck this nuts

> Task :gdx-jnigen:check
> Task :gdx-jnigen:build
> Task :gdx-jnigen-gradle:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

> Task :gdx-jnigen-gradle:processResources
> Task :gdx-jnigen-gradle:classes
> Task :gdx-jnigen-gradle:jar
> Task :gdx-jnigen-gradle:javadoc
> Task :gdx-jnigen-gradle:javadocJar
> Task :gdx-jnigen-gradle:sourcesJar
> Task :gdx-jnigen-gradle:assemble
> Task :gdx-jnigen-gradle:compileTestJava NO-SOURCE
> Task :gdx-jnigen-gradle:processTestResources NO-SOURCE
> Task :gdx-jnigen-gradle:testClasses UP-TO-DATE
> Task :gdx-jnigen-gradle:test NO-SOURCE
> Task :gdx-jnigen-gradle:check UP-TO-DATE
> Task :gdx-jnigen-gradle:build
> Task :gdx-jnigen-loader:javadoc
> Task :gdx-jnigen-loader:javadocJar
> Task :gdx-jnigen-loader:sourcesJar
> Task :gdx-jnigen-loader:assemble
> Task :gdx-jnigen-loader:compileTestJava NO-SOURCE
> Task :gdx-jnigen-loader:processTestResources NO-SOURCE
> Task :gdx-jnigen-loader:testClasses UP-TO-DATE
> Task :gdx-jnigen-loader:test NO-SOURCE
> Task :gdx-jnigen-loader:check UP-TO-DATE
> Task :gdx-jnigen-loader:build

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.3/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 1h 37m 19s
19 actionable tasks: 10 executed, 9 up-to-date

But the gradle build is freaking slow on the board... I guess the jdk is not well optimized for riscv or something.

I agree with you about choosing the architecture. Maybe it's something for the gdx-setup app? Or do we add Gradle tasks that generate one jar per architecture?

Berstanio commented 10 months ago

I agree with you about choosing the architecture. Maybe it's something for the gdx-setup app? Or do we add Gradle tasks that generate one jar per architecture?

I'm not sure, whether this isn't a bit bloated. We already have 6 jars produced with natives, if now desktop gets split up too we end up with like around 23 jars if we split on os and arch. Only splitting on arch would be around 11 jars, which is still a lot in my opinion.

PokeMMO commented 9 months ago

At first glance, we should consider replacing isARM and isRISCV with a more generic arch enum (or some other extendable argument)

While there, it might also make sense to turn is64 into an enum of potential bitness.

I think this should all be able to do be done while ensuring backwards compatibility too.

SimonIT commented 9 months ago

Done, do you think we should deprecate methods with boolean is64Bit, boolean isARM? Those constants are then pretty useless:

    public static final boolean x32 = false;
    public static final boolean x64 = true;
    public static final boolean ARM = true;
    public static final boolean RISCV = true;

Should we also change the shared library loader? I also think the extra handling for android and ios is a bit ugly.

PokeMMO commented 9 months ago

Done, do you think we should deprecate methods with boolean is64Bit, boolean isARM?

Yeah, the legacy methods should be marked as deprecated.

Those constants are then pretty useless:

I was assuming we could replace them with references to the enums in a backwards compatible manner, but I haven't experimented to check if that's actually possible.

Should we also change the shared library loader?

I think so, but doing so would be very breaking, and might be worthwhile keeping the old ones as accurate as we can.

SimonIT commented 9 months ago

Done, except for the shared library loader. The reason is that I'm not sure where to put the enums because, in the current position, it's not able to access them

SimonIT commented 8 months ago

Do you have an idea? Should we move the enum to SharedLibraryLoader? Or do we want to leave it as it is?

PokeMMO commented 8 months ago

I think move it to SharedLibraryLoader, and make a new static field for the arch. Also leave the old isARM, isRISCV, is64Bit but mark them as deprecated.

SimonIT commented 8 months ago

Everything is moved

PokeMMO commented 8 months ago

Merged!

Next step should be PR with -SNAPSHOT to libgdx to triple check before we make a new release.