On Mon, 2 May 2022 16:05:08 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
> Refactoring and renaming changes to some of the D3D pipeline files and a few > changes on the Java side. These are various "leftovers" from previous issues > that we didn't want to touch at the time in order to confine the scope of the > changes. They will make future work easier. > > Since there are many small changes, I'm giving a full list here: > > **Java** > > * `NGShape3D.java` > * Extracted methods to help with the cumbersome lighting loop: one method > per light type + empty light (reset light) + default point light. This > section of the code would benefit from the upcoming pattern matching on > `switch`. > * Normalized the direction here instead of in the native code. > * Ambient light is now only set when it exists (and is not black). > * `NGPointLight,java` - removed unneeded methods that were used by > `NGShape3D` before the per-lighting methods were extracted (point light > doesn't need spotlight-specific methods since they each have their own "add" > method). > * `NGSpotLight.java` - removed `@Override` annotations as a result of the > above change. > > **Native C++** > > * Initialized the class members of `D3DLight`, `D3DMeshView` and > `D3DPhongMaterial` in the header file instead of a more cumbersome > initialization in the constructor (this is allowed since C++ 11). > * `D3DLight` > * Commented out unused methods. Were they supposed to be used at some point? > * Renamed the `w` component to `lightOn` since it controls the number of > lights for the special pixel shader variant and having it in the 4th position > of the color array was confusing. > * `D3DPhongShader.h` > * Renamed some of the register constants for more clarity. > * Moved the ambient light color constant from the vertex shader to the > pixel shader (see the shader section below). I don't understand the > calculation of the number of registers in the comment there: "8 ambient > points + 2 coords = 10". There is 1 ambient light, what are the ambient > points and coordinates? In `vsConstants` there is `gAmbinetData[10]`, but it > is unused. > * Reduced the number of assigned vertex register for the `VSR_LIGHTS` > constant since it included both position and color, but color was unused > there (it was used directly in the pixel shader), so now it's only the > position. > * `D3DMeshView.cc` > * Unified the lighting loop that prepares the lights' 4-vetors that are > passed to the shaders. > * Removed the direction normalization as stated in the change for > `NGShape3D.java`. > * Reordered the shader constant assignment to be the same order as in > `D3DPhongShader.h`. > > **Native shaders** > * Renamed many of the variables to what I think are more descriptive names. > This includes noting in which space they exist as some calculations are done > in model space, some in world space, and we might need to do some in view > space. For vectors, also noted if the vector is to or from (`eye` doesn't > tell me if it's from or to the camera). > * Commented out many unused functions. I don't know what they are for, so I > didn't remove them. > * `vsConstants` > * Removed the light color from here since it's unused, only the position is. > * Removed the ambient light color constant from here since it's unused, and > added it to `psConstants` instead. > * `vs2ps` > * Removed the ambient color interpolation, which frees a register (no > change in performance). > * Simplified the structure (what is `LocalBumpOut` and why is it called > `light` and contains `LocalBump`?). > * `Mtl1PS` and `psMath` > * Moved the shader variant constants (`#ifndef`) to `Mtl1PS` where they are > used for better clarity. > * Moved the lights loop to `Mtl1PS`. The calculation itself remains in > `psMath`. > > No changes in performance were measured and the behavior stayed the same. This pull request has now been integrated. Changeset: 4a278013 Author: Nir Lisker <nlis...@openjdk.org> URL: https://git.openjdk.org/jfx/commit/4a2780134b88e86e437ae2bdffe06b145788e61d Stats: 685 lines in 22 files changed: 232 ins; 227 del; 226 mod 8217853: Cleanup in the D3D native pipeline Reviewed-by: arapte, kcr ------------- PR: https://git.openjdk.org/jfx/pull/789
