RFR: 8217472: Add attenuation for PointLight [v14]

[Kevin Rushforth]

On Thu, 24 Sep 2020 02:46:31 GMT, Nir Lisker <nlisker at openjdk.org> 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:
> 
>   Change range after clamping

The public API and implementation looks good. I left a few inline comments.

I'll review the CSR in parallel with your fixing the few things I noted.

modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGPointLight.java line 118:

> 116:         }
> 117:     }
> 118: }

Add newline here.

modules/javafx.graphics/src/main/native-prism-d3d/D3DLight.cc line 49:

> 47:     attenuation[1] = 0;
> 48:     attenuation[2] = 0;
> 49:     maxRange = 0;

Same comment as in `NGShape3D` about using infinity here, although if this value is always set from the Java side, then
this native default might not matter.

modules/javafx.graphics/src/main/native-prism-d3d/D3DLight.cc line 64:

> 62: }
> 63:
> 64: /*void D3DLight::setRange(float r) {

Remove this unused function?

modules/javafx.graphics/src/main/native-prism-es2/GLContext.c line 2128:

> 2126:     meshViewInfo->pointLightAttenuation[1] = 0;
> 2127:     meshViewInfo->pointLightAttenuation[2] = 0;
> 2128:     meshViewInfo->pointLightMaxRange = 0;

Same comment as `D3DLight.cc` about the default value.

tests/system/src/test/java/test/javafx/scene/lighting3D/PointLightAttenuationTest.java line 51:

> 49: public class PointLightAttenuationTest {
> 50:
> 51:     private static final double DELTA = 1d/255; // smallest color resolution

This test fails on my MacBook Pro system. Our other robot-based tests use a higher tolerance because of various
problems we run into trying to look for exact values. I recommend a tolerance of `10d/255`, which will still catch
whether or not attenuation is used.

tests/system/src/test/java/test/javafx/scene/lighting3D/PointLightAttenuationTest.java line 105:

> 103:             double attenBlueDiag = snapshot.getPixelReader().getColor(SAMPLE_DIST, 0).getBlue();
> 104:             assertEquals("Wrong color value", nonAttenBlueCenter * attnCenter, attenBlueCenter, DELTA);
> 105:             assertEquals("Wrong color value", nonAttenBlueDiag * attnDiag, attenBlueDiag, DELTA);

This test looks good. Maybe you could also add a test of quadratic attenuation and max range?

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

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