RFR: 8234921: Add DirectionalLight to the selection of 3D light types [v2]

Kevin Rushforth kcr at openjdk.java.net
Sat Dec 18 16:15:27 UTC 2021


On Sat, 18 Dec 2021 16:00:24 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/native-prism-d3d/hlsl/psMath.h line 106:
>> 
>>> 104: 
>>> 105: void computeLight(float i, float3 n, float3 refl, float specPower, float3 L, float3 lightDir, in out float3 d, in out float3 s) {
>>> 106:     if (gLightAttenuation[i].w < 0.5) {
>> 
>> Is there any performance difference between `< 0.5` and `!= 0` ? I suspect not, but in any case, you might consider the latter (as I also mentioned in the java files). The latter is more clear, so if you choose to stick with what you have, I'd like to see a comment added.
>
> Initially I went with the equality test, but it did not work well, probably since we are comparing floating points. This just seemed safer. If you want to try the equality test on you machine(s) it would be interesting, but I don't know if I would trust it. Same with the ES shaders. On the Java side I would trust it, but at this point it's a matter of consistency.
> 
> I suspect that this also messed up with the computation approaches of spot light that check `falloff != 0` where we should be checking with a small delta: `abs(falloff) < EPSILON` instead.
> Performance should be revisited anyway I think at a later stage after the emissive color is added (where we need to figure out splitting shaders, which also has to do with the removal of the 3 lights restriction).

It seems safer then to either leave it the way you have it (in which case, please a comment), or change it to `abs(gLightAttenuation[i].w) < EPSILON` both here and in the Java code (for consistency). I'll leave it up to you.

For spot light, you might want to file a new bug to address the `falloff != 0` problem.

I agree about deferring the performance question.

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

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


More information about the openjfx-dev mailing list