RFR: 8234920: Add SpotLight to the selection of 3D light types

Nir Lisker nlisker at openjdk.java.net
Tue Dec 22 17:43:04 UTC 2020

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.


Commit messages:
 - Fixed merged conflict
 - Fixed whitespaces
 - OpenGL implementation
 - Address review comments
 - Update to the d3d pipeline
 - Docs images
 - Small docs clarifications
 - Subclassed PointLight
 - Fix whitespaces
 - Updated test app for SpotLights
 - ... and 4 more: https://git.openjdk.java.net/jfx/compare/16b697c9...40de36fe

Changes: https://git.openjdk.java.net/jfx/pull/334/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=334&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8234920
  Stats: 1142 lines in 34 files changed: 791 ins; 75 del; 276 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

More information about the openjfx-dev mailing list