On Sun, 21 Mar 2021 17:48:10 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Finally getting back to this. >> >> There is an outstanding API question regarding the direction vector: Should >> the transforms of the SpotLight node to scene coordinates affect the >> direction vector (e.g., such that a rotation would rotate the direction the >> spotlight is pointing)? If so, do we even need the direction vector or could >> we define the direction as `(0,0,1)` in the local coordinates of the >> SpotLight, and tell applications to apply the appropriate rotation to define >> the direction. >> >> I think for consistency, the answer is "Yes" to the first question (that's >> how the position works and it would be odd for the transform to affect the >> position and not the direction). I'm less certain about the answer to the >> second question. Without utility functions like `lookAt` to help compute the >> appropriate transform, it seems important to be able to specify the >> direction as an explicit parameter. And even if we had utility functions, an >> app might want the control (although you might argue it would be unneeded). >> My instinct is to keep it as defined. If we go with this, the only change >> that is needed is a note that the transforms applied to the SpotLight node >> affect it's direction as well as its position. >> >> Implementation: >> >> Can you add a system test similar to what you did for attenuation to verify >> the basic functionality? >> >> I tested this a bit on Windows using D3D and it behaves as I would expect, >> although I didn't do exhaustive testing. >> >> On Mac I no longer get a GLSL shader error at runtime, but spotlights aren't >> working correctly, either. The falloff happens too fast (at 1 you can't see >> the light at all). For the inner/outer angles, something looks backwards. >> See the attached image >> <img width="1212" alt="SpotLight-macOS" >> src="https://user-images.githubusercontent.com/34689748/111841271-f293d000-88ba-11eb-915c-38e2c69f5e70.png"> >> >> I left a few inline comments, although I haven't reviewed the shader files >> in detail yet. > >> There is an outstanding API question regarding the direction vector: Should >> the transforms of the SpotLight node to scene coordinates affect the >> direction vector (e.g., such that a rotation would rotate the direction the >> spotlight is pointing)? If so, do we even need the direction vector or could >> we define the direction as `(0,0,1)` in the local coordinates of the >> SpotLight, and tell applications to apply the appropriate rotation to define >> the direction. >> >> I think for consistency, the answer is "Yes" to the first question (that's >> how the position works and it would be odd for the transform to affect the >> position and not the direction). I'm less certain about the answer to the >> second question. Without utility functions like `lookAt` to help compute the >> appropriate transform, it seems important to be able to specify the >> direction as an explicit parameter. And even if we had utility functions, an >> app might want the control (although you might argue it would be unneeded). >> My instinct is to keep it as defined. If we go with this, the only change >> that is needed is a note that the transforms applied to the SpotLight node >> affect it's direction as well as its position. > > I would like to allow a developer to achieve a functionality like is shown in > https://www.youtube.com/watch?v=CFgwZX5dkcM at 9:50. The rotations are > intuitive there. If we allow both rotation transforms and a direction, > wouldn't that cause the direction to be unintuitive? Or do you mean that the > direction is always the look-at regardless of rotation transforms and if it's > `null` then the rotations take over? > >> On Mac I no longer get a GLSL shader error at runtime, but spotlights aren't >> working correctly, either. > > I don't see this on Linux and I don't have a Mac. Can you try on Linux and > see what you get? If Linux works, I'm afraid I would not be able to debug the > Mac. I modified the existing attenuation test to use a `SpotLight`, which is more general, and separated the tests for the various contributions of the light. I renamed the class, so the previous version is removed, but its tests are the same in the new class. ------------- PR: https://git.openjdk.java.net/jfx/pull/334