[Rev 03] RFR: 8217472: Add attenuation for PointLight

Nir Lisker nlisker at openjdk.java.net
Thu Jan 9 18:00:29 UTC 2020

On Fri, 3 Jan 2020 23:02:56 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>>> I still need to test your sample app on Mac.
>> I get the error with your sample app. It fails on Mac or Linux (Ubuntu 16.04) with the same error as I reported above.
> The error was for the cases of 2 and 3 lights (I was testing 1) and should be fixed now. My fault with copy-paste... that's why we use loops, but I guess this is some optimization for the es2 pipeline. I wonder if it's really worth it over a single shader looping over the number of lights like d3d does.

> This will need a fair bit of testing to ensure that there are no regressions either in functionality or (especially) performance, in addition to tests for the new functionality.

Which tests for the new functionality should I write? Up to the shader itself it's mostly just passing on variables, and the API is standard `DoubleProperty`s. I can test the dirty bits / redraw logic.

> On the performance aspect, the inner loop of the lighting calculation now has an additional if test for the max range and additional arithmetic calculations for the attenuation. What we will need is a test program that we can run on Mac and Windows to measure the performance of rendering in a fill-rate-limited case. Ideally, we would not pay much of a performance hit in the default case where `ca == 1, la == 0, qa == 0`, but we first need to be able to measure the drop in performance before we can say whether it is acceptable.

Can you point me to a similar performance test? I didn't see a way to easily measure FPS.
I don't know how to avoid the `if` test for the `maxRange`, I'm open to suggestions. The only thing I can think of is if `maxRange` is infinite (the default), we will swap the shader for one that doesn't make that check. However, swapping shaders also costs performance.


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

More information about the openjfx-dev mailing list