On Thu, 17 Jun 2021 00:38:56 GMT, Nir Lisker 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?
>>
>> - [x] 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:
>
> Fixed method call in glsl shaders
I did a full test run on 5 different system, including manual tests on 4 of
them:
Windows 10 w/ Intel graphics
Linux w/ NVIDIA graphics (no manual testing)
Linux VM guest running on Windows 10 host
Mac w/ discrete graphics
Mac w/ integrated graphics
No problems detected. All looks good.
I think there could be some additional tuning done for point lights, but that
could be looked at in a follow-on fix.
I reviewed the CSR and it is ready to be Finalized.
I finished reviewing the shader code, and left a couple comments on the HLSL
shaders.
modules/javafx.graphics/src/main/native-prism-d3d/hlsl/psConstants.h line 28:
> 26: // see
