On Thu, 2 Jan 2020 18:18:21 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> 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) > >> I get the following system test failures on Mac >> ``` >> Shader compile log: ERROR: 0:308: Use of undeclared identifier 'range' >> ERROR: 0:316: Regular non-array variable 'dist' may not be redeclared >> ``` > > I don't have a Mac to test on, but on Ubuntu system tests pass (I ran the > `test` command for systemTests). Does the sample app I attached also fail on > Mac? They both use the same shaders, so I wonder where the issue could be. > Moreover, the error messages are strange. `dist` is not redeclared and > `range` is not undeclared in the shader. The error message seems to originate > from the native function `Java_com_sun_prism_es2_GLContext_nCompileShader` in > `GLContext.c`, not managing to compile the shader, but I can't tell why. I get the same error on Ubuntu 16.04 as on Mac. Did you run the system tests with `-PFULL_TEST=true -PUSE_ROBOT=true`? Also, you can try running the `fx83dfeatures/LightMotion` toy and you should see the same error. I still need to test your sample app on Mac. ------------- PR: https://git.openjdk.java.net/jfx/pull/43
