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

Add compatibility with Java 13 #400

Merged
merged 19 commits into from
Feb 4, 2021
Merged

Add compatibility with Java 13 #400

merged 19 commits into from
Feb 4, 2021

Conversation

igorpisarev
Copy link
Contributor

@igorpisarev igorpisarev commented Mar 19, 2020

Currently, Paintera works only with Java 8. If running with Java 9+, NoClassDefFoundException will be printed as reported in #398.

Some work has been made in this PR to add compatibility with Java 13, which also allows us to use JavaFX 13 and its new API, such as PixelBuffer for writing into JavaFX images directly instead of having to use reflection to do it.

The PR is not ready yet and there is still a few things to do:

  • Serialization/deserialization needs to be able to access JavaFX classes with reflection, and to make it work properly the packages need to be listed in the project configuration file: https://github.com/saalfeldlab/paintera/blob/java13/pom.xml#L702-L724
  • There are problems with certain shortcuts: for example, Ctrl+Shift+Scroll doesn't work in 3D viewer anymore for some reason.
  • label-utilities dependency and possibly others need to be updated to support Java 13.
  • Need to rebase and update the PR with respect to the most recent master branch.

@hanslovsky
Copy link
Collaborator

That's awesome!!

@hanslovsky
Copy link
Collaborator

I don't know if it is relevant for us but people saw bad performance when migrating from Java 8 to 11:
https://twitter.com/shipilev/status/1242078941659217920

@igorpisarev
Copy link
Contributor Author

For anyone continuing to work on this PR, this is the command that I used to run and debug the application:

mvn javafx:run

(it's provided by maven-javafx-plugin which was added to pom.xml in this PR)

@igorpisarev
Copy link
Contributor Author

The distribution of the application will probably need to change with this PR too. Since JavaFX 13 is available through Maven directly and the users will not need to install a runtime dependency, it may be worthwhile to switch to building and distributing an uber jar that contains all dependencies. This will require only a standard JRE installation to run Paintera on users' machines without any other prerequisites such as JDK and Maven.

@cmhulbert
Copy link
Collaborator

cmhulbert commented Jan 13, 2021

  • There are problems with certain shortcuts: for example, Ctrl+Shift+Scroll doesn't work in 3D viewer anymore for some reason.

Addressed by a3bc181. JavaFx now interprets Shift+Scroll as a horizontal scroll, so querying for the DeltaY was always 0.0. Solution uses the JavaFx ControlUtils to get scroll axis with the largest delta magnitude. This is consistent with how the orthographic slice scrolling works.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@cmhulbert cmhulbert requested a review from axtimwalde January 28, 2021 15:10
Copy link
Contributor

@axtimwalde axtimwalde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some formatting comments and question about jdk 13.
I assume we need to push for a jgo release

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@cmhulbert cmhulbert marked this pull request as ready for review February 3, 2021 21:23
README.md Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@axtimwalde axtimwalde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@axtimwalde axtimwalde merged commit 5e07ad4 into master Feb 4, 2021
@cmhulbert cmhulbert deleted the java13 branch September 27, 2021 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants