RFR: 8217472: Add attenuation for PointLight [v5]

[Kevin Rushforth]

On Fri, 8 May 2020 12:36:41 GMT, Nir Lisker <nlisker at openjdk.org> 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 8217472_Add_attenuation_for_PointLight
>  - Attenuation and range changed internally to floats from doubles
>  - Fixed shader compilation errors for 2 and 3 lights in es2
>  - Addressing review comments
>  - Fixed whitespaces
>  - Correction for indexes
>  - Docs and year update
>  - Merge remote-tracking branch 'nlisker/8217472_Add_attenuation_for_PointLight' into
>    8217472_Add_attenuation_for_PointLight
>  - GL pipeline
>  - Separate range from attenuation
>  - ... and 1 more: https://git.openjdk.java.net/jfx/compare/4ec163df...2e1223ed

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 and make it part of this PR, maybe
in `tests/performance` (which currently only has the not-very-useful `VMPerformance` subdir) or `tests/manual`. 3. We
need some functional tests, ideally automated ones in `tests/system`

I also left a few inline comments. I haven't reviewed the shaders yet, so I do that, in addition to further testing,
next week.

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 NOTICES OR THIS FILE HEADER.

Update the "last modified year" for 2020 or else revert (this applies to all copyright year updates).

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/PointLightHelper.java line 77:

> 76:     }
> 77: }

Minor: missing newline here

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:         }

Minor: maybe use the getDefaultXxxx methods of NGPointLight?

modules/javafx.graphics/src/main/java/javafx/scene/PointLight.java line 51:

> 50:  * results in the light's color being subtracted from the material instead of added to it, thus creating a
> 51:  * "shadow caster".
> 52:  * <p>

Can you think of any problems that might arise by supporting negative coefficients? If not, then this seems fine.

modules/javafx.graphics/src/main/java/javafx/scene/PointLight.java line 108:

> 107:      * @defaultValue {@code Double.POSITIVE_INFINITY}
> 108:      * @since 14
> 109:      */

`@since 16`

-------------

PR: https://git.openjdk.java.net/jfx/pull/43