On Tue, 2 Aug 2022 10:57:25 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> I'll have a look. What command did you run this test with?
>> 
>> Can you also test the point light attenuation under 
>> `tests\system\src\test\java\test\javafx\scene\lighting3D\PointLightAttenuationTest.java`?
>>  (I think there's a bit of a mess in the tests folders.)
>
> Command to run the system test:
> `gradle -PFULL_TEST=true -PUSE_ROBOT=true systemTests:test --tests 
> test.robot.test3d.PointLightIlluminationTest`
> 
> I ran all the tests, only above test failed:
> Command to run all the system tests: `gradle -PFULL_TEST=true 
> -PUSE_ROBOT=true systemTests:test`

First of all, I found that the mistake was in the shortcut branch of the 
shader: it was using the light direction instead of the vector to the light 
(incident ray), so in the code I need to replace `dot(n, -lightDir)` with 
`dot(n, -l)`, like is done in the full computation branch. Then the test passes 
with the `0` input for no-attenuation. Thanks.

Then I looked at the computation some more and I found something in the 
computation of the specular component. According to the theory, both in online 
sources and in the `PhongMaterial` class doc, the computation should be:
`R . V` where `V` is the vector to the eye/cam and `R` is the reflection vector 
computed by `R = 2(N . L)N - L`, or using the 
[reflect](https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-reflect)
 HLSL function, `R = -reflect(N, L)`. So the shader files should look something 
like this:

float3 refl = reflect(l, n);
s = ... dot(-refl, e); // e is the vector to the eye, like V

However, looking at the shader files, [already from 
legacy](https://github.com/openjdk/jfx/blob/c420248b9b459efcfbd3657170d9be0b96b5fb38/modules/javafx.graphics/src/main/native-prism-d3d/hlsl/psMath.h),
 the vectors are switched:

float3 refl = reflect(e, n);
s = ... dot(-refl, l);

It looks like the specular computation was wrong from the start.

I tested the visuals on the `master` branch before and after swapping the `l` 
and `e` vectors and I see no difference in the specular reflection. Rather odd. 
Will need to look into this more.

The same mistakes are coded into the glsl shaders too, so I should fix the one 
you found at the very least.

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

PR: https://git.openjdk.org/jfx/pull/789

Reply via email to