RFR: 8234920: Add SpotLight to the selection of 3D light types [v14]

[Ambarish Rapte]

On Thu, 15 Apr 2021 02:21:50 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Added a SpotLight only to the D3D pipeline currently.
>> 
>> ### API discussion points
>> 
>> - [X]  Added `SpotLight` as a subclass of `LightBase`. However, it could also be a subclass of `PointLight` as it's a point light with direction and extra factors. I saw that `scenario.effect.light.SpotLight` extends its respective `PointLight`, but it's not a perfect analogy. In the end, I think it's a questions of whether `PointLight` will be expanded in a way which doesn't not suit `SpotLight`, and I tend to think that the answer is no.
>> 
>> - [X] The inner and outer angles are the "diameter angles" as shown [here](https://docs.microsoft.com/en-us/windows/win32/direct3d9/light-typeshttps://docs.microsoft.com/en-us/windows/win32/direct3d9/light-types).  I, personally, find it more intuitive that these are the "radius angles", so half these angles, as used in the spotlight factor formula. Do you think I can change this or do you prefer the current definition of the angles?
>> 
>> - [ ] The current implementation uses an ad-hoc direction property (using a `Point3D`). It crossed my mind that we could use the rotation transforms of the node to control the direction instead, just like we use the translation/layout of the node to get the position (there is an internal Affine3D transform for lights, not sure why `AmbientLight` needs it). Wouldn't that make more sense? When I rotate the light I would expect to see a change in direction.
>> 
>> ### Implementation discussion points
>> 
>> - [ ] I've gotten advice from a graphics engineer to treat point lights as spot lights with a 360 degrees coverage, which simplifies a few places. We can still try to optimize for a point light by looking at the light parameters: `falloff = 0` and `outerAngle = 180`. These possible optimization exist in `ES2PhongShader.java` and `D3DMeshView.cc`, and in the pixel/fragment shaders in the form of 3 different ways to compute the spotlight factor (the `computeLightN` methods). We need to check which of these give the best results.
>> 
>> ## Performance
>> 
>> Testing 3 point lights and comparing this branch with `master` using a 1000 division sphere, 200 meshes, and 5000 meshes.
>> Using an AMD RX 470 4GB GPU.
>> 
>> In this branch, there is a possible CPU optimization for checking the light type and using precalculated values (in `D3DMeshView.cc` for d3d and `ES2PhongShader.java` for opengl). On the GPU, I tried 3 ways of computing the spotlight factor contributions (`computeSpotlightFactor`, `computeSpotlightFactor2` and `computeSpotlightFactor3`) trying out different branching and shortcuts.
>> 
>> ### Results
>> The CPU "optimizations" made no difference, which is understandable considering it will not be the bottleneck. We can remove these if we want to simplify, though maybe if we allow a large number of lights it could make a difference (I doubt it). I don't have a strong preference either way.
>> 
>> The sphere 1000 tests always gave max fps (120 on Win and 60 on Ubuntu).
>> 
>> **Win 10**
>> Compared with the `master` branch, this patch shows 5-10 fps drop in the mesh 200 test and ~5 in the mesh 5000 test. I repeated the tests on several occasions and got different results in terms of absolute numbers, but the relative performance difference remained more or less the same. Out of the 3 `computeSpotlightFactor` methods, `computeSpotlightFactor3`, which has no "optimizations", gives slightly better performance.
>> 
>> **Ubuntu 18**
>> The mesh 200 test always gave 60 fps because it is locked to this fps, so we can't measure the real GPU performance change.
>> The mesh 5000 test shows 2-6 fps drop from master, with `computeSpotlightFactor` > `computeSpotlightFactor2`  > `computeSpotlightFactor3` at in terms of performance (~2 fps difference each).
>> 
>> **Conclusion**: we can expect a 5 fps drop more or less with 3 point lights. `computeSpotlightFactor3` on d3d and `computeSpotlightFactor` on opengl gave the best performances.
>
> Nir Lisker has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Combined rotation and direction

Provided few comments on documentation. Have to review and test the code.

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 46:

> 44:  * <p>
> 45:  * The light cone is defined by 3 factors: an {@link #innerAngleProperty() inner angle}, an {@link #outerAngleProperty()
> 46:  * outer angle}, and a {@link #falloffProperty() falloff} factor. For a point whose angle to the light is {@code a}, if

->
A {@code SpotLight} is a {@code PointLight} that radiates a cone of light in a specific direction.
< p >
The direction is defined by the {@link #directionProperty() direction} property.
The light cone is defined by 3 properties: an {@link #innerAngleProperty() inner angle}, an {@link #outerAngleProperty() outer angle}, and a {@link #falloffProperty() falloff} property.

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 49:

> 47:  * {@code a < innerAngle} then that point receives maximum illumination, if {@code a > outerAngle} then that point
> 48:  * receives no illumination, and if {@code innerAngle <= a <= outerAngle} then the illumination is determined by the
> 49:  * formula

->
For a point whose direction vector from the source of light forms an angle {@code theta} with the direction of light, 
if {@code theta < innerAngle} then that point receives maximum illumination.
if {@code theta > outerAngle} then that point receives no illumination. and
if {@code innerAngle <= a <= outerAngle} then the illumination received by the point is determined by formula

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 55:

> 53:  * {@code falloff >= 0}; values outside either of these ranges can produce unexpected results.
> 54:  * <p>
> 55:  * <img src="doc-files/spotlight.png" alt="Image of the Spotlight">

Should we have cone depicted in the diagram ?

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 76:

> 74: 
> 75:     {
> 76:         // To initialize the class helper at the beginning each constructor of this class

Minor: missing of: // To initialize the class helper at the beginning **of** each constructor of this class

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 98:

> 96: 
> 97:     /**
> 98:      * The direction the spotlight is facing. The vector need not be normalized.

-> The direction vector of the spotlight. The vector need not be normalized.

modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 130:

> 128:      * The angle of the spotlight's inner cone. Surfaces whose angle to the light's origin is less than this angle
> 129:      * receive the full light's intensity. At larger angles, the light intensity starts to drop. The valid range is
> 130:      * {@code 0 <= innerAngle <= outerAngle}; values outside of this range can produce unexpected results.

We should use Point or vertex instead of word surface. Using Point would be consistent with description of class.

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

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