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 all seven of the javafx modules to JavaBuild.java #856

Merged
merged 4 commits into from
Nov 17, 2024

Conversation

vsquared
Copy link
Contributor

Please add _all seven of the javafx modules so that we may use JavaFX controls in our apps. Thanks.

Please add _all seven of the javafx modules so that we may use JavaFX controls in our apps. Thanks.
@hx2A
Copy link
Collaborator

hx2A commented Nov 16, 2024

I'd like to second this. This change would also make it much easier to use JavaFX with py5.

@SableRaf SableRaf requested a review from Stefterv November 16, 2024 23:05
@SableRaf SableRaf changed the title Update JavaBuild.java Add all seven of the javafx modules to JavaBuild.java Nov 16, 2024
@SableRaf
Copy link
Collaborator

Hi @vsquared and thanks for your suggestion. It looks like the current commit just adds a file to the root of the repository with instructions for modifying JavaBuild.java, instead of directly making the change in the file. Could you update the PR to modify JavaBuild.java directly instead so we can properly review the proposed changes?

@glvjr
Copy link

glvjr commented Nov 16, 2024

Hello,

I am seeing different behavior with Processing 4.3 and 4.3.1 (no changes and as is).

These will run in Processing 4.3.1 with the JavaFX jar files (all or only required) in the sketch folder:
https://discourse.processing.org/t/javafx-controls-preview/41353
https://discourse.processing.org/t/javafx-webview-preview/41343

I added this to the above in the sketch:
import processing.javafx.*;

They (above) will give errors in Processing 4.3

I did a build of Processing 4.3.1 with IntelliJ IDEA 2024.3 (community Edition) with this:

"--add-modules", "javafx.base,javafx.graphics,javafx.swing,javafx.controls,javafx.media,javafx.web,javafx.fxml",
And I still had to add the required jar files to the sketch folder.

This is on a W10 PC.

@vsquared
Copy link
Contributor Author

vsquared commented Nov 17, 2024

After modifying two different Processing editors with the above change my observation is that adding the seven javafx jar files to the sketch folder is no longer necessary. I have no experience using javafx in IntelliJ or testing on a platform other than macos. My impression is that adding all the jar files to the sketch folder sometimes fails when trying to run javafx code in an editor that has not been modified. I cannot be 100% certain about this and it needs further study. Nevertheless, I don't think the average user wants to go to all the bother to find the seven jar files and copy them to the sketch folder just to run a javafx demo. It would be easier for everyone if a simple one line change was made to the editor. I know this works for macos, but have not tried it on Windows or Linux.
Addendum:
Processing 4.3 failed to run a javafx app with all seven jar files in sketch folder, either lying free or inside a 'code' folder. Another copy of a Processing 4.3 editor modified with the changes above will run the javafx demo without the addition of jar files.
Processing 4.3.1 on macos also failed to run a javafx app with all seven jar files in the sketch folder, both lying free and inside a 'code' folder.

@vsquared
Copy link
Contributor Author

vsquared commented Nov 17, 2024

@SableRaf - I did another fork of the main app and edited the JavaBuild.java file and submitted a second pull request.

Please add _all seven of the javafx modules so that we may use JavaFX controls in our apps. Thanks.
Copy link
Collaborator

@Stefterv Stefterv left a comment

Choose a reason for hiding this comment

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

Hi @vsquared,

Please change the things I marked, I will now look into the JavaFX examples provided and see if they work

Looking at the context of this change as something specific and limited to existing cases handling JavaFX code I think it would probably be fine. I just need to test and understand your use case

vsquared-patch-1 Outdated Show resolved Hide resolved
java/src/processing/mode/java/JavaBuild.java Outdated Show resolved Hide resolved
@Stefterv
Copy link
Collaborator

Tested on windows, does indeed fix the issue you're describing, so happy to merge after the requested changes.
If you can find a user friendly way, e.g. not just adding the JavaFX jars to the code folder, please let me know. I would like to make that possible.

Please add _all seven of the javafx modules so that we may use JavaFX controls in our apps. Thanks.
@Stefterv
Copy link
Collaborator

Cool thanks @vsquared!

I'll let you hit the merge button @SableRaf

@vsquared
Copy link
Contributor Author

@Stefterv - If you can find a user friendly way, e.g. not just adding the JavaFX jars to the code folder, please let me know. I would like to make that possible.
On macos if you make the changes I suggested the user should not need to add the jar files to the sketch folder.

@SableRaf SableRaf merged commit 9292eb5 into processing:main Nov 17, 2024
6 checks passed
@vsquared vsquared deleted the patch-1 branch November 17, 2024 14:22
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.

5 participants