RFR: 8217472: Add attenuation for PointLight [v5]
Ambarish Rapte
arapte at openjdk.java.net
Fri Jul 24 10:08:21 UTC 2020
On Fri, 8 May 2020 12:36:41 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8218264
>
> Nir Lisker has updated the pull request with a new target base due to a merge or a rebase. The pull request now
> contains 11 commits:
> - Merge branch 'master' into 8217472_Add_attenuation_for_PointLight
> - Attenuation and range changed internally to floats from doubles
> - Fixed shader compilation errors for 2 and 3 lights in es2
> - Addressing review comments
> - Fixed whitespaces
> - Correction for indexes
> - Docs and year update
> - Merge remote-tracking branch 'nlisker/8217472_Add_attenuation_for_PointLight' into
> 8217472_Add_attenuation_for_PointLight
> - GL pipeline
> - Separate range from attenuation
> - ... and 1 more: https://git.openjdk.java.net/jfx/compare/4ec163df...2e1223ed
Added some minor comments. The changes suggested in the shader cause no difference in FPS, but only reduces number of
instructions in compiled fragment shader. I verified it in the compiled D3D shader, that the change reduces one
multiplication instruction. The glCompileShader() API performs some optimizations on the shader. But the optimizations
would be implementation specific. The difference of fps in different machines may be caused by difference of
glCompileShader() implementation. On the shader side there are few ways of doing [glsl
optimizations](https://www.khronos.org/opengl/wiki/GLSL_Optimizations). This may be worth for a quick try now or later
as follow on.
modules/javafx.graphics/src/main/java/javafx/scene/PointLight.java line 174:
> 173:
> 174: private static final double DEFAULT_LINEAR_CONSTANT = NGPointLight.getDefaultLa();
> 175:
Small correction needed, `DEFAULT_LINEAR_CONSTANT` -> `DEFAULT_LINEAR_ATTENUATION`
similarly, `DEFAULT_QUADRATIC_CONSTANT` -> `DEFAULT_QUADRATIC_ATTENUATION`
modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl/main3Lights.frag line 92:
> 91: d += clamp(dot(n,l), 0.0, 1.0) * (lights[0].color).rgb * att;
> 92: s += pow(clamp(dot(-refl, l), 0.0, 1.0), power) * lights[0].color.rgb * att;
> 93: }
These three lines can be changed as:
float attenuatedColor = (lights[0].color).rgb / (lights[0].attn.x + lights[0].attn.y * dist + lights[0].attn.z * dist *
dist); d += clamp(dot(n,l), 0.0, 1.0) * attenuatedColor;
s += pow(clamp(dot(-refl, l), 0.0, 1.0), power) * attenuatedColor;
Similar change in the 1 light and 2 lights shader.
modules/javafx.graphics/src/main/native-prism-d3d/hlsl/psMath.h line 76:
> 75: d += saturate(dot(n, l)) * gLightColor[i].xyz * attn;
> 76: s += pow(saturate(dot(-refl, l)), power) * gLightColor[i].xyz * attn;
> 77: }
The temporary variables ca, la, qa will be removed by shader compiler optmization, so has no extra cost involved here.
But we can reduce one multiplication instruction by changing the rest of the code as,
float attenuatedColor = gLightColor[i].xyz / (ca + la * dist + qa * dist * dist);
float3 l = normalize(L[i].xyz);
d += saturate(dot(n, l)) * attenuatedColor;
s += pow(saturate(dot(-refl, l)), power) * attenuatedColor;
-------------
Changes requested by arapte (Reviewer).
PR: https://git.openjdk.java.net/jfx/pull/43
More information about the openjfx-dev
mailing list