On Mon, 26 Oct 2020 00:43:02 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Added a SpotLight only to the D3D pipeline currently. >> >> ### API >> >> 1. 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. >> >> 2. 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? >> >> CSR will be created when the API review is finished. >> >> ### Implementation >> >> I've gotten advice from a graphics engineer to treat point lights as spot >> lights with a 360 degrees coverage, which simplifies the shader a lot and >> removes branching over the light type. This is covered anyway by a possible >> optimization in the pixel shader, which I've noted in an inline comment, >> that skips the spotlight computation if `falloff` is 0 (this is the case for >> non-spotlights). >> >> ### Performance >> >> Testing 3 point lights in order to compare with the [pre-patch >> performance](https://github.com/openjdk/jfx/pull/43#issuecomment-600632114): >> >> Without the falloff == 0 branching: >> sphere 1000 subdivisions: 120 >> mesh 5000: 9.5 >> mesh 200: 111.5 >> >> With the falloff == 0 branching: >> sphere 1000 subdivisions: 120 >> mesh 5000: 9.3 >> mesh 200: 112.5 >> >> Ubuntu 20: >> With the patch: >> Mesh 200: 60 fps >> Mesh 5000: 15 fps >> Sphere 1000: 60 fps >> >> Without the patch (master branch) >> Mesh 200: 60 fps >> Mesh 5000: 16.3 fps >> Sphere 1000: 60 fps >> >> So no major changes. I will repeat these tests to make sure there was no >> mistake. > > I suggest we start with looking at the API and the subclass question. This > will unblock the CSR process. My preference would be for `SpotLight` to subclass `PointLight`. I also somewhat prefer the 1/2 angle (radius) rather than diameter to specify the spread angles (inner and outer). ------------- PR: https://git.openjdk.java.net/jfx/pull/334
