On Wed, 3 Feb 2021 21:56:08 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> I still get a runtime shader compilation error on Mac (see above) I pushed a fix now. Since it doesn't happen on Win or Ubuntu I can't verify it, but I changed `0` to `0.0` etc. in the shader code. > Unless the transform itself is non-invertible, I don't think this can happen. > What I would expect is that if we go this route -- which seems like a fine > API choice to me -- the direction vector would be `(0, 0, 1)` in the local > coordinate system of the light, which would then be suitably rotated based on > the transform of the light (and any transforms above it). I think that this is also what I did when I tested this approach. When computing the direction in `NGSpotLight`, I multiplied the `Affine3D worldTransform` by a `Vec3D` of (0, 0, 1). Then, using rotations on the main axes, I was able to rotate the light in any direction I want. The problem here is ease of use. Because the order in which you add the rotation transforms matter, it becomes unintuitive for the user compared to using a direction. If I rotate 90 degrees in the X direction, the Y and Z axes are "flipped". If we can make it intuitive, then I think this approach is better. > Hmm. I wonder if the ideal choice? One of the reasons for wanting a > DirectionalLight, as distinct from a very far away PointLight with no > attenuation, is performance. On the other hand, since it will complicates the > design of the shaders to have a special case for directional lights, it might > not be worth implementing. I don't think it matters all that much, so if you > want to rename it to setLight that seems OK. It's an implementation detail > and could be revisited in the future. The branching vs unneeded computation costs is a main testing point in this patch. We will do the same tests for DirectionalLight in its own patch. A DirectionalLight can be implemented in the vertex shader, but it gives less accurate results I believe. I updated the Ubuntu tests results. ------------- PR: https://git.openjdk.java.net/jfx/pull/334
