Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-10-08 Thread Nir Lisker
On Thu, 8 Oct 2020 22:08:16 GMT, Kevin Rushforth wrote: >> In that case, it seems like a generally useful optimization (not just at >> initialization) to send down `maxRange` as 0 >> whenever `ca`, `la`, and `qa` are all at their default values. > > Actually, my above comment is wrong. A

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-10-08 Thread Kevin Rushforth
On Fri, 7 Aug 2020 22:30:39 GMT, Kevin Rushforth wrote: >> The only impact this has is that the range will be maximal instead of 0. >> When these reach the shader, they will run the >> lighting computation as opposed to skipping it. I'm not sure if this will >> have any performance impact

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-19 Thread Kevin Rushforth
On Fri, 4 Sep 2020 23:20:43 GMT, Nir Lisker wrote: >> Thanks, the test runs now, but I've run into an issue with `snapshot`. >> >> `Node#snapshot` uses its scene parameters (like lights) to render the image, >> but if the node is in a subscene then the >> image will be wrong since it takes the

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-04 Thread Nir Lisker
On Fri, 4 Sep 2020 14:34:40 GMT, Nir Lisker wrote: >> If you run it with `--info` you will see something like this: >> >> test.javafx.scene.shape.PointLightAttenuationTest STANDARD_ERROR >> Exception in Application constructor >> Exception in thread "Thread-4"

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-04 Thread Nir Lisker
On Wed, 2 Sep 2020 17:16:00 GMT, Kevin Rushforth wrote: >> I wrote the following test: >> >> import static org.junit.Assert.assertTrue; >> import static org.junit.Assert.fail; >> >> import java.util.concurrent.CountDownLatch; >> import java.util.concurrent.TimeUnit; >> >> import

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-02 Thread Kevin Rushforth
On Wed, 2 Sep 2020 16:57:25 GMT, Nir Lisker wrote: >> It can either be a fully qualified class name (with `.` as separator) or the >> unqualified name of the test class. The >> class name must end with exactly `Test`. So for example, try it with >> `Snapshot1Test`. > > I wrote the following

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-02 Thread Nir Lisker
On Tue, 1 Sep 2020 23:09:39 GMT, Kevin Rushforth wrote: >>> gradle -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests MyTest >> >> What format is `MyTest`? Is it some relative path? > > It can either be a fully qualified class name (with `.` as separator) or the > unqualified name of

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Kevin Rushforth
On Tue, 1 Sep 2020 22:46:44 GMT, Nir Lisker wrote: >> You need to pass a flag to enable system tests (and a second one for Robot >> tests, but if you use snapshot it might not >> need to be). >> gradle -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests MyTest > >> gradle

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Nir Lisker
On Tue, 1 Sep 2020 17:00:10 GMT, Kevin Rushforth wrote: > gradle -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests MyTest What format is `MyTest`? Is it some relative path? - PR: https://git.openjdk.java.net/jfx/pull/43

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Kevin Rushforth
On Tue, 1 Sep 2020 13:49:48 GMT, Nir Lisker wrote: >> I guess you are trying a unit test in the `javafx.graphics` project? That >> won't work. It needs to be a system test in >> `tests/system/...` > > I tried several approaches. When I put it under `tests/system` and run the > tests task, I

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Nir Lisker
On Tue, 1 Sep 2020 11:42:30 GMT, Kevin Rushforth wrote: >> I've written a simple draft for the automated test: >> >> PointLight light = new PointLight(Color.BLUE); >> light.setTranslateZ(-50); >> var box = new Box(100, 100, 1); >> var root = new Group(light,

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Kevin Rushforth
On Tue, 1 Sep 2020 10:54:50 GMT, Nir Lisker wrote: >> The performance tests need a standard GPLv2+classpath copyright header. I >> haven't tested them yet, but will do that >> next week. > > I've written a simple draft for the automated test: > > PointLight light = new

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-09-01 Thread Nir Lisker
On Fri, 7 Aug 2020 22:37:15 GMT, Kevin Rushforth wrote: >> Given that we don't have any automated tests for lighting (we have a couple >> that verify that we can take a snapshot and >> get the same result, but that isn't testing the lighting itself), probably >> the most we can expect is a

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-08-08 Thread Kevin Rushforth
On Fri, 24 Jul 2020 09:16:38 GMT, Ambarish Rapte wrote: > ``` > float attenuatedColor = gLightColor[i].xyz / (ca + la * dist + qa * dist * > dist); > ``` Typo: that should be: float3 attenuatedColor = gLightColor[i].xyz / (ca + la * dist + qa * dist * dist); - PR:

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-08-07 Thread Kevin Rushforth
On Fri, 7 Aug 2020 21:55:25 GMT, Kevin Rushforth wrote: >>> 1. The API docs look good. If you change the public API to `@since 16` then >>> you can also update the CSR and move it to >>> the "Submitted" state. >> >> I moved it to the PROPOSED state. >> >>> 2. I think it would be good to

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-08-07 Thread Kevin Rushforth
On Wed, 29 Jul 2020 09:17:10 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGShape3D.java >> line 187: >> >>> 186: 0, 0, 0, 0, // r g b w >>> 187: 1, 0, 0, 0); // ca la qa maxRange >>> 188: } >>

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-08-07 Thread Kevin Rushforth
On Wed, 29 Jul 2020 23:11:20 GMT, Nir Lisker wrote: >> High-level comments: >> >> 1. The API docs look good. If you change the public API to `@since 16` then >> you can also update the CSR and move it to >> the "Submitted" state. 2. I think it would be good to cleanup the >> performance test

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-29 Thread Nir Lisker
On Fri, 24 Jul 2020 21:41:04 GMT, Kevin Rushforth wrote: > 1. The API docs look good. If you change the public API to `@since 16` then > you can also update the CSR and move it to > the "Submitted" state. I moved it to the PROPOSED state. > 2. I think it would be good to cleanup the

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-29 Thread Nir Lisker
On Fri, 24 Jul 2020 21:25:06 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now >> contains 11 commits: >> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight >> - Attenuation and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-29 Thread Ambarish Rapte
On Tue, 28 Jul 2020 12:44:18 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl/main3Lights.frag >> line 92: >> >>> 91: d += clamp(dot(n,l), 0.0, 1.0) * (lights[0].color).rgb * att; >>> 92: s += pow(clamp(dot(-refl, l), 0.0, 1.0), power)

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-29 Thread Ambarish Rapte
On Tue, 28 Jul 2020 13:28:38 GMT, Nir Lisker wrote: > I'd like to know how you did that. Compiled hlsl shader binaries are stored in folder `rt\modules\javafx.graphics\build\headers\PrismD3D\hlsl`. The `fxc` command that generates these files is in `build.gradle`. We can just make a copy and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-28 Thread Kevin Rushforth
On Tue, 28 Jul 2020 13:15:02 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/PointLightHelper.java >> line 2: >> >>> 1: /* >>> 2: * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights >>> reserved. >>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-28 Thread Nir Lisker
On Fri, 24 Jul 2020 21:41:04 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now >> contains 11 commits: >> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight >> - Attenuation and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-28 Thread Nir Lisker
On Fri, 24 Jul 2020 21:01:08 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now >> contains 11 commits: >> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight >> - Attenuation and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-28 Thread Nir Lisker
On Fri, 24 Jul 2020 21:29:06 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now >> contains 11 commits: >> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight >> - Attenuation and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-28 Thread Nir Lisker
On Thu, 23 Jul 2020 12:06:17 GMT, Ambarish Rapte wrote: >> Nir Lisker has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now >> contains 11 commits: >> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight >> - Attenuation and

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-24 Thread Kevin Rushforth
On Fri, 8 May 2020 12:36:41 GMT, Nir Lisker wrote: >> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264 > > Nir Lisker has updated the pull request with a new target base due to a merge > or a rebase. The pull request now > contains 11 commits: > - Merge branch 'master' into

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-24 Thread Ambarish Rapte
On Fri, 8 May 2020 12:36:41 GMT, Nir Lisker wrote: >> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264 > > Nir Lisker has updated the pull request with a new target base due to a merge > or a rebase. The pull request now > contains 11 commits: > - Merge branch 'master' into

Re: RFR: 8217472: Add attenuation for PointLight [v5]

2020-07-01 Thread Nir Lisker
On Wed, 13 May 2020 23:50:37 GMT, Kevin Rushforth wrote: >>> We should make sure that we aren't seeing any significant performance drop >>> when rendering spheres (at a couple >>> different tessellation levels) or boxes. >> >> I missed this. Do you mean that the test should create a mesh of a