On Wed, 20 Nov 2019 00:32:36 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Kevin, Ambarish,
>>
>> You can start the review, especially the API. I will hunt that specific
>> values bug this week.
>>
>> I'll need to know what kind of tests are needed in terms of functionality
>> and performance.
>
> The bug I mentioned above is not a bug actually. There are large changes over
> a small distance that make it looks like a jump in the lighting values, but
> when working with a finer scale the lighting dynamics seem correct.
I think this is on the right track. The API looks like it is in good shape.
This will need a fair bit of testing to ensure that there are no regressions
either in functionality or (especially) performance, in addition to tests for
the new functionality. On the performance aspect, the inner loop of the
lighting calculation now has an additional if test for the max range and
additional arithmetic calculations for the attenuation. What we will need is a
test program that we can run on Mac and Windows to measure the performance of
rendering in a fill-rate-limited case. Ideally, we would not pay much of a
performance hit in the default case where `ca == 1, la == 0, qa == 0`, but we
first need to be able to measure the drop in performance before we can say
whether it is acceptable.
Speaking of testing, I took the current patch for a test drive on Mac and
Windows. I get the following system test failures on Mac, and also the same
failure using fx83dfeatures/LightMotion in toys.
Shader compile log: ERROR: 0:308: Use of undeclared identifier 'range'
ERROR: 0:316: Regular non-array variable 'dist' may not be redeclared
test.robot.test3d.MeshCompareTest > testSnapshot3D[3] STANDARD_ERROR
java.lang.RuntimeException: Error creating fragment shader
at
javafx.graphics/com.sun.prism.es2.ES2Shader.createFromSource(ES2Shader.java:141)
at
javafx.graphics/com.sun.prism.es2.ES2PhongShader.getShader(ES2PhongShader.java:177)
...
test.robot.test3d.MeshCompareTest > testSnapshot3D[3] FAILED
java.lang.IllegalArgumentException: Unrecognized image loader: null
at
javafx.graphics/javafx.scene.image.WritableImage.loadTkImage(WritableImage.java:278)
at
javafx.graphics/javafx.scene.image.WritableImage$1.loadTkImage(WritableImage.java:53)
at javafx.graphics/javafx.scene.Scene.doSnapshot(Scene.java:1340)
at javafx.graphics/javafx.scene.Scene.doSnapshot(Scene.java:1372)
at javafx.graphics/javafx.scene.Scene.snapshot(Scene.java:1462)
at
test.robot.test3d.MeshCompareTest.lambda$testSnapshot3D$0(MeshCompareTest.java:315)
test.robot.test3d.Snapshot3DTest > testSnapshot3D[3] FAILED
(same failure as above)
test.robot.test3d.Snapshot3DTest > testSnapshot3D[7] FAILED
(same failure as above)
-------------
PR: https://git.openjdk.java.net/jfx/pull/43
