Re: RFR: 8217853: Cleanup in the D3D native pipeline [v2]
> 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. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Remove unused comments, clean constructor - Changes: - all: https://git.openjdk.java.net/jfx/pull/789/files - new: https://git.openjdk.java.net/jfx/pull/789/files/8db9c8ba..05281ba3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=789=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=789=00-01 Stats: 21 lines in 2 files changed: 0 ins; 18 del; 3 mod Patch: https://git.openjdk.java.net/jfx/pull/789.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/789/head:pull/789 PR: https://git.openjdk.java.net/jfx/pull/789
RFR: 8217853: Cleanup in the D3D native pipeline
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. - Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jfx/pull/789/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=789=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8217853 Stats: 624 lines in 18 files changed: 232 ins; 202 del; 190 mod Patch: https://git.openjdk.java.net/jfx/pull/789.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/789/head:pull/789 PR: https://git.openjdk.java.net/jfx/pull/789
[jfx11u] Integrated: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
On Sat, 30 Apr 2022 13:48:01 GMT, Kevin Rushforth wrote: > This PR is dependent on #97 so it is in Draft for now. Once #97 is > integrated, I will rebase this and submit it, at which time it will be a > clean backport, and the diffs will only show the updated test. This pull request has now been integrated. Changeset: f01fc732 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/f01fc732ac0972a79c1c5b397b23df38ba3a2004 Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Backport-of: 94807b6edfb9af55be353cab237e8e64007c61dc - PR: https://git.openjdk.java.net/jfx11u/pull/98
[jfx11u] RFR: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
This PR is dependent on #97 so it is in Draft for now. Once #97 is integrated, I will rebase this and submit it, at which time it will be a clean backport, and the diffs will only show the updated test. - Commit messages: - 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Changes: https://git.openjdk.java.net/jfx11u/pull/98/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=98=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280275 Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/98.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/98/head:pull/98 PR: https://git.openjdk.java.net/jfx11u/pull/98
[jfx11u] Integrated: 8276174: JavaFX build fails on macOS aarch64
On Sat, 30 Apr 2022 13:51:23 GMT, Kevin Rushforth wrote: > Backport to `jfx11u` so we can build on macOS / aarch64 without needing to > specify `-PCOMPILE_TARGET=arm64`. It isn't a clean backport, since I also had > to include the definition of `IS_AARCH64` which is present in mainline, but > not in `jfx11u` (it was added as part of another unrelated fix that isn't > backported to `jfx11u`). This pull request has now been integrated. Changeset: ac52af6e Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/ac52af6ed3535f687e2026d2afd398aac4111e84 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod 8276174: JavaFX build fails on macOS aarch64 Reviewed-by: jvos Backport-of: d289db94d15e49ed28f797b516ffccf023fce9c9 - PR: https://git.openjdk.java.net/jfx11u/pull/100
[jfx11u] Integrated: 8280840: Update libFFI to 3.4.2
On Sat, 30 Apr 2022 13:30:49 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. Tested in connection with gstreamer / glib update > in the `test-kcr-11.0.16` branch. This pull request has now been integrated. Changeset: 1e4862f4 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/1e4862f436d17c409f0a33645a413acc771b731e Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod 8280840: Update libFFI to 3.4.2 Backport-of: 1beb3235223452929ec951ee18dd30a5345307cf - PR: https://git.openjdk.java.net/jfx11u/pull/93
[jfx11u] Integrated: 8283218: Update GStreamer to 1.20.1
On Sat, 30 Apr 2022 13:37:08 GMT, Kevin Rushforth wrote: > Backport to `jfx11u`. Tested in connection with libffi update in the > `test-kcr-11.0.16` branch. > > This was not a clean backport, but the merge conflicts were trivial to > resolve. Here is a summary of the changes. The jfx mainline patch applied > cleanly to all other files. > > 1. The following file is not present in jfx11u, so that part of the patch was > skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c > > > 2. The following files had minor merge conflicts, the first was a diff in > surrounding context and the rest were in copyright header blocks: > > > modules/javafx.media/src/main/native/gstreamer/projects/linux/fxplugins/Makefile > modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile > modules/javafx.media/src/tools/native/def-glib.pl > modules/javafx.media/src/tools/native/def-gstlite.pl This pull request has now been integrated. Changeset: d0df8471 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/d0df8471c4e38e0b0dfae0e83848316694033002 Stats: 38984 lines in 412 files changed: 22060 ins; 5959 del; 10965 mod 8283218: Update GStreamer to 1.20.1 8283403: Update Glib to 2.72.0 Reviewed-by: jvos Backport-of: c4b1a72cc4d9253d1320d83281d50fb1f3bb6145 - PR: https://git.openjdk.java.net/jfx11u/pull/94
[jfx11u] Integrated: 8274274: Update JUnit to version 5.8.1
On Sat, 30 Apr 2022 13:45:39 GMT, Kevin Rushforth wrote: > Nearly clean backport to `jfx11u`. It isn't a clean backport since the > mainline patch updated `gradle/verification-metadata.xml`, which doesn't > exist in 11. This pull request has now been integrated. Changeset: 44e691e4 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/44e691e4811bac2202e2b8ee992f35937ec06f54 Stats: 63 lines in 4 files changed: 58 ins; 3 del; 2 mod 8274274: Update JUnit to version 5.8.1 Reviewed-by: arapte Backport-of: ff6e8d50badd57549811391b1380707bb94ac55b - PR: https://git.openjdk.java.net/jfx11u/pull/97
[jfx11u] Integrated: 8281564: Update cmake to 3.22.3
On Sat, 30 Apr 2022 13:39:32 GMT, Kevin Rushforth wrote: > Simple backport to `jfx11u`. It isn't a clean backport since the mainline > patch had an update to `gradle/verification-metadata.xml`, which doesn't > exist in 11. This pull request has now been integrated. Changeset: e877f305 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/e877f305edbb9c00cee091470049f7873c4f208e Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8281564: Update cmake to 3.22.3 Reviewed-by: arapte Backport-of: d960d639dc6e37de0cdb69075a31a17090b83a3d - PR: https://git.openjdk.java.net/jfx11u/pull/95
[jfx17u] Integrated: 8276167: VirtualFlow.scrollToTop doesn't scroll to the top of the last element
On Mon, 2 May 2022 13:14:05 GMT, Johan Vos wrote: > Reviewed-by: aghaisas, jvos This pull request has now been integrated. Changeset: 62ab00bd Author:Johan Vos URL: https://git.openjdk.java.net/jfx17u/commit/62ab00bd5c8913fb8089b3d02404366db1d52c37 Stats: 42 lines in 4 files changed: 37 ins; 0 del; 5 mod 8276167: VirtualFlow.scrollToTop doesn't scroll to the top of the last element Backport-of: 115833923a644be8328b8f7a7a6845e5b42a57b3 - PR: https://git.openjdk.java.net/jfx17u/pull/52
[jfx17u] Integrated: 8276167: VirtualFlow.scrollToTop doesn't scroll to the top of the last element
Reviewed-by: aghaisas, jvos - Commit messages: - 8276167: VirtualFlow.scrollToTop doesn't scroll to the top of the last element Changes: https://git.openjdk.java.net/jfx17u/pull/52/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=52=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276167 Stats: 42 lines in 4 files changed: 37 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jfx17u/pull/52.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/52/head:pull/52 PR: https://git.openjdk.java.net/jfx17u/pull/52
[jfx17u] Integrated: 8276553: ListView scrollTo() is broken after fix for JDK-8089589
On Sat, 30 Apr 2022 18:48:06 GMT, Johan Vos wrote: > Reviewed-by: kcr, mstrauss This pull request has now been integrated. Changeset: 83534e2d Author:Johan Vos URL: https://git.openjdk.java.net/jfx17u/commit/83534e2df8f4d20a661ae68c1c6f9925302953e3 Stats: 29 lines in 5 files changed: 19 ins; 2 del; 8 mod 8276553: ListView scrollTo() is broken after fix for JDK-8089589 Backport-of: d3fbb516c428876bd2389bbfd40f95a811ea6af8 - PR: https://git.openjdk.java.net/jfx17u/pull/50
[jfx17u] Integrated: 8274137: TableView scrollbar/header misaligned when reloading data
On Sat, 30 Apr 2022 18:44:09 GMT, Johan Vos wrote: > Reviewed-by: kcr, aghaisas This pull request has now been integrated. Changeset: ca0e5a47 Author:Johan Vos URL: https://git.openjdk.java.net/jfx17u/commit/ca0e5a4752e3a0ab2c9b6950c61678b4b07c4749 Stats: 34 lines in 3 files changed: 34 ins; 0 del; 0 mod 8274137: TableView scrollbar/header misaligned when reloading data Backport-of: b591912c749edef1e6c1b8509a8ea10e9fe9000f - PR: https://git.openjdk.java.net/jfx17u/pull/49
Re: [jfx11u] RFR: 8274274: Update JUnit to version 5.8.1
On Sat, 30 Apr 2022 13:45:39 GMT, Kevin Rushforth wrote: > Nearly clean backport to `jfx11u`. It isn't a clean backport since the > mainline patch updated `gradle/verification-metadata.xml`, which doesn't > exist in 11. Marked as reviewed by arapte (Reviewer). - PR: https://git.openjdk.java.net/jfx11u/pull/97
Re: [jfx11u] RFR: 8281564: Update cmake to 3.22.3
On Sat, 30 Apr 2022 13:39:32 GMT, Kevin Rushforth wrote: > Simple backport to `jfx11u`. It isn't a clean backport since the mainline > patch had an update to `gradle/verification-metadata.xml`, which doesn't > exist in 11. Marked as reviewed by arapte (Reviewer). - PR: https://git.openjdk.java.net/jfx11u/pull/95
[jfx17u] Integrated: 8284184: Crash in GraphicsContextJava::drawLinesForText on https://us.yahoo.com/
Reviewed-by: kcr, arapte - Commit messages: - 8284184: Crash in GraphicsContextJava::drawLinesForText on https://us.yahoo.com/ Changes: https://git.openjdk.java.net/jfx17u/pull/51/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=51=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284184 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx17u/pull/51.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/51/head:pull/51 PR: https://git.openjdk.java.net/jfx17u/pull/51
[jfx17u] Integrated: 8284184: Crash in GraphicsContextJava::drawLinesForText on https://us.yahoo.com/
On Mon, 2 May 2022 11:48:02 GMT, Ambarish Rapte wrote: > Reviewed-by: kcr, arapte This pull request has now been integrated. Changeset: 13ca8ee8 Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/13ca8ee891486b17551f55ef93532a703de37c3e Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod 8284184: Crash in GraphicsContextJava::drawLinesForText on https://us.yahoo.com/ Backport-of: 64da125f05f2a25038ce3370c8fe7c0baf0a354b - PR: https://git.openjdk.java.net/jfx17u/pull/51
[jfx17u] Integrated: 8270867: Debug build of WebKit 613.1 fails on Linux
On Fri, 29 Apr 2022 17:14:21 GMT, Ambarish Rapte wrote: > Reviewed-by: kcr, jbhaskar This pull request has now been integrated. Changeset: 367f3d5a Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/367f3d5a8227aeb83af5fbc646329c860ca0fcdc Stats: 142 lines in 2 files changed: 1 ins; 141 del; 0 mod 8270867: Debug build of WebKit 613.1 fails on Linux Backport-of: eb7fa5dd1c0911bca15576060691d884d29895a1 - PR: https://git.openjdk.java.net/jfx17u/pull/48
[jfx17u] Integrated: 8280020: Underline and line-through not straight in WebView
On Fri, 29 Apr 2022 17:10:09 GMT, Ambarish Rapte wrote: > Reviewed-by: kcr, arapte This pull request has now been integrated. Changeset: 8e52c7be Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/8e52c7bee8cd4e5aa83a6ea33572cd41026f9776 Stats: 192 lines in 2 files changed: 182 ins; 2 del; 8 mod 8280020: Underline and line-through not straight in WebView Backport-of: 263db3df5fdf9e0f6955be6ae58173aa589e55fe - PR: https://git.openjdk.java.net/jfx17u/pull/47
[jfx17u] Integrated: 8269115: WebView paste event contains old data
On Fri, 29 Apr 2022 17:08:56 GMT, Ambarish Rapte wrote: > Reviewed-by: arapte, kcr This pull request has now been integrated. Changeset: cc835ba1 Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/cc835ba1fe0a135e82c6e6461f67e3ff56cf7b37 Stats: 120 lines in 3 files changed: 108 ins; 3 del; 9 mod 8269115: WebView paste event contains old data Backport-of: 2338821bdbb7db929a89aa89903271dcff333a5c - PR: https://git.openjdk.java.net/jfx17u/pull/46
[jfx17u] Integrated: 8255940: localStorage is null after window.close()
On Fri, 29 Apr 2022 17:07:27 GMT, Ambarish Rapte wrote: > Reviewed-by: kcr, arapte This pull request has now been integrated. Changeset: ca7303ce Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/ca7303ce40e38848a7e606b940f376e979f6e187 Stats: 168 lines in 4 files changed: 167 ins; 0 del; 1 mod 8255940: localStorage is null after window.close() Backport-of: 5112be957be70dd6521e6fb6ee64e669c148729c - PR: https://git.openjdk.java.net/jfx17u/pull/45
[jfx17u] Integrated: 8278759: PointerEvent: buttons property set to 0 when mouse down
On Fri, 29 Apr 2022 17:06:02 GMT, Ambarish Rapte wrote: > Reviewed-by: kcr, arapte This pull request has now been integrated. Changeset: dd592f13 Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx17u/commit/dd592f13ca63abc1691b8eeff01bb03abf6ca6dc Stats: 302 lines in 8 files changed: 293 ins; 1 del; 8 mod 8278759: PointerEvent: buttons property set to 0 when mouse down Backport-of: 2e8a4a5e97bb88a5807ae5fe075b98e1d54a4ca0 - PR: https://git.openjdk.java.net/jfx17u/pull/44
