On Fri, 29 Jul 2022 21:10:23 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> Nir Lisker has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unused comments, clean constructor
>
> modules/javafx.graphics/src/main/native-prism-d3d/hlsl/psMath.h line 98:
>
>> 96: * specular component. The computation is done in world space.
>> 97: */
>> 98: void computeLight(float i, float3 n, float3 refl, float specPower,
>> float3 toLight, float3 lightDir, in out float3 d, in out float3 s) {
>
> `toLight` -> `vertexToLightVec` ?
The computation at this point is for pixels, and the argument type tells you if
it's a vector (`float3`). When working with fields in the shaders' input/output
it was convenient to be more descriptive with the names. Here everything is
confined, so I don't think the descriptive names help much. Can still do a
naming pass on these if you want.
> modules/javafx.graphics/src/main/native-prism-d3d/hlsl/vsConstants.h line 36:
>
>> 34: static const int isSkinned = Skin;
>> 35:
>> 36: static const int maxLights = 3;
>
> May be name as `MAX_LIGHTS` similar to `static const int MAX_BONES = 70;` on
> line 28 in same file. ?
> of preferably use `MAX_NUM_LIGHTS` which we already use in .h, .cpp files.
The max lights limitation is going to be removed in the following change set
(at least I intend to do it), so I don't think it's worth touching all those
constants.
-------------
PR: https://git.openjdk.org/jfx/pull/789
