On Sun, 13 Jun 2021 23:08:22 GMT, Nir Lisker <nlis...@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?
>> 
>> - [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:
> 
>   Addressed review comments

I retested on Linux Ubuntu 20 with an AMD RX 470, all 3 lights show correctly:

![Screenshot from 2021-06-14 
02-52-54](https://user-images.githubusercontent.com/37422899/121825552-c6220100-ccbb-11eb-8854-18c1bc86c87f.png)

@arapte If you run the test app without the spotlight patch, but with the 
attenuation patch, does it work better?

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

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

Reply via email to