> 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.
Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Update to the gl pipeline ------------- Changes: - all: https://git.openjdk.java.net/jfx/pull/334/files - new: https://git.openjdk.java.net/jfx/pull/334/files/1bdfc1d6..e699127d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=334&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=334&range=05-06 Stats: 205 lines in 6 files changed: 139 ins; 22 del; 44 mod Patch: https://git.openjdk.java.net/jfx/pull/334.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/334/head:pull/334 PR: https://git.openjdk.java.net/jfx/pull/334
