👋 Welcome back nlisker! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Webrevs

• 05: Full - Incremental (ae471f2e)
• 04: Full - Incremental (f063d088)
• 03: Full - Incremental (9b3f0c5e)
• 02: Full - Incremental (3f2994ff)
• 01: Full - Incremental (2646489d)
• 00: Full (7f0e562f)

Reviewer: @arapte

@jayathirthrao you might also want to take a look?

Couple quick comments:

• Executing the LightingSample manually fails with an exception

Caused by: java.lang.NullPointerException: Input stream must not be null
	at javafx.graphics@23-internal/javafx.scene.image.Image.validateInputStream(Image.java:1140)
	at javafx.graphics@23-internal/javafx.scene.image.Image.<init>(Image.java:707)
	at lighting3D.Environment.<clinit>(Environment.java:64)

Steps:
cd tests/performance/3DLighting/src/main/java
javac <compile.args> lighting3D/*.java
java <run.args> lighting3D.LightingApplication

• The copyright year new files should be 2024 only. Some new files have more than one copyright year. If you are moving any existing files then please use git mv so that the files are shown as moved.
• Some files are missing copyright headers.

An observation out of scope of this PR.
The effect of lighting on mesh vs a box is quite different.
Notice the screenshot, left side is a Box and right side is Mesh, both have Magenta SpotLight.
I think the effect of light should be same on a 2D surface, and the effect on Box seems correct to me.
Another observation: The effect of light on Mesh is different with 100 divisions than 200+ divisions.

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

Executing the LightingSample manually fails with an exception

I think the package names of the main and resources weren't aligned.

The copyright year new files should be 2024 only. Some new files have more than one copyright year. If you are moving any existing files then please use git mv so that the files are shown as moved.

For now, for the ease of reviewing, I reverted the package name change.

The effect of lighting on mesh vs a box is quite different.

I think this is the same issue you submitted in the past: https://bugs.openjdk.org/browse/JDK-8269133

Even after reverting package name i continue to see that 3DLighting throws exception when we manually launch it:

javafx.graphics@23-internal/javafx.scene.image.Image.validateInputStream(Image.java:1140)
	at javafx.graphics@23-internal/javafx.scene.image.Image.<init>(Image.java:707)
	at attenuation.Environment.<clinit>(Environment.java:64)

I can't reproduce the exception. The path to the image is correct. Both the class and the image are under the attenuation package. Here is my launch command:

C:\Program Files\Java\jdk-21\bin\javaw.exe "-Djava.library.path=C:\Users\Nir\git\jfx\modules\javafx.graphics\build\module-lib"
--add-modules=javafx.controls,javafx.swing
-Dfile.encoding=UTF-8
-Dstdout.encoding=UTF-8
-Dstderr.encoding=UTF-8
-p "C:\Users\Nir\git\jfx\modules\javafx.controls\bin;C:\Users\Nir\git\jfx\modules\javafx.graphics\bin;C:\Users\Nir\git\jfx\modules\javafx.base\bin;C:\Users\Nir\git\jfx\modules\javafx.swing\bin"
-classpath "C:\Users\Nir\git\jfx\tests\performance\3DLighting\bin"
-XX:+ShowCodeDetailsInExceptionMessages
--add-exports javafx.graphics/test.com.sun.javafx.pgstub=javafx.controls
--add-exports javafx.base/test.com.sun.javafx.binding=javafx.controls
--add-exports javafx.base/test.util.memory=javafx.controls
--add-exports javafx.base/test.javafx.collections=javafx.controls
--add-exports javafx.base/com.sun.javafx.property=javafx.graphics
--add-exports javafx.base/test.javafx.collections=javafx.graphics
--add-exports javafx.base/test.util.memory=javafx.graphics
--add-exports java.base/sun.security.util=javafx.graphics
--add-reads javafx.base=java.management
--add-reads javafx.base=jdk.management attenuation.LightingApplication

Perhaps it's a platform-dependent path issue?

I can reproduce the exception if I compile the test program from the command line in the .../src/main/java dir using javac and run the program from there. The reason that happens is the separation of the resources into a separate directory tree.

One way to solve this, so we can still easily compile and run it from the command line, is to provide an "ant" or "gradle" script as was done for MonkeyTester and FXMediaPlayer

I added the application to the gradle build file so it can be hooked up with its module dependencies. You can now produce a jar that bundles the resource with the classes. You still need to point to the modules at runtime.
Let me know if this is a good enough solution.

The way the project is configured in the main build file is not the best, but it will require large changes to do it better.

Not sure what jcheck wants. It's complaining about empty spaces that have a } appear after them. This is the error: https://github.com/openjdk/jfx/pull/1387/files#annotation_19798318258.

Looks like the line specified by jcheck was off.

@nlisker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@nlisker This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.