> 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 d3d pipeline

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

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/334/files
  - new: https://git.openjdk.java.net/jfx/pull/334/files/dc37e2e6..b3ca78ad

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=334&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=334&range=01-02

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 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

Reply via email to