Re: RFR: 8217472: Add attenuation for PointLight [v16]
On Fri, 9 Oct 2020 18:11:14 GMT, Kevin Rushforth wrote:
>> Nir Lisker has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed test
>
> tests/performance/3DLighting/attenuation/LightingSample.java line 62:
>
>> 60: environment.setStyle("-fx-background-color: teal");
>> 61:
>> 62: var subdivisionSlider = new Slider(10, 200, 60);
>
> I might recommend setting the max value of the slider to something like 1000,
> since even on a slow system, it runs
> quite nicely at that tessellation.
I forgot to reply to this and the other comment. Since we're going to implement
more light types, this small test app
is going to be updated anyway, so I will change the values then.
-
PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8217472: Add attenuation for PointLight [v16]
On Fri, 9 Oct 2020 16:33:28 GMT, Nir Lisker wrote: >> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264 > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed test Looks good to me. There seems to be an existing issue with PointLight. It is reported here [JDK-8255015](https://bugs.openjdk.java.net/browse/JDK-8255015) - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8217472: Add attenuation for PointLight [v16]
On Fri, 9 Oct 2020 16:33:28 GMT, Nir Lisker wrote: >> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264 > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed test I think this is ready to go in once it gets a second reviewer and once the CSR is approved. I left two very minor comments on the test. I don't mind whether you fix them now or as a follow-up. Marked as reviewed by kcr (Lead). - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8217472: Add attenuation for PointLight [v16]
On Fri, 9 Oct 2020 16:33:28 GMT, Nir Lisker wrote:
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264
>
> Nir Lisker has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed test
tests/performance/3DLighting/attenuation/LightingSample.java line 72:
> 70: sphere.setOnAction(e -> switchTo(environment.createSphere((int)
> subdivisionSlider.getValue(;
> 71:
> 72: var quadSlider = new Slider(500, 10_000, 1000);
I was doing some final testing, and ran this on a few different systems. I
think the min and max values are too large
(especially the min value). I recommend something more like min=100 and
max=5000.
tests/performance/3DLighting/attenuation/LightingSample.java line 62:
> 60: environment.setStyle("-fx-background-color: teal");
> 61:
> 62: var subdivisionSlider = new Slider(10, 200, 60);
I might recommend setting the max value of the slider to something like 1000,
since even on a slow system, it runs
quite nicely at that tessellation.
-
PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8217472: Add attenuation for PointLight [v16]
> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264 Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Fixed test - Changes: - all: https://git.openjdk.java.net/jfx/pull/43/files - new: https://git.openjdk.java.net/jfx/pull/43/files/e1fdf8e2..475e1bd2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=43=15 - incr: https://webrevs.openjdk.java.net/?repo=jfx=43=14-15 Stats: 7 lines in 1 file changed: 6 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/43.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/43/head:pull/43 PR: https://git.openjdk.java.net/jfx/pull/43
