NullPointer this.layoutCache is null
I am encountering the following Null Pointer exception during runtime, however the application keeps working as intended. Exception posted at the end to declutter the mail. Google search for this exception took me to this thread https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044343.html but in my case the JVM doesn't crash, neither does the application crash. The application continues to work as intended. I am unable to identify the node that causes this as the stack trace doesn't originate from the Application code, could anyone throw some light. I am running the code on /usr/lib/jvm/bellsoft-java21-full-amd64/bin/java tion in thread "JavaFX Application Thread" java.lang.NullPointerException: Cannot read field "advances" because "this.layoutCache" is null at javafx.graphics/com.sun.javafx.text.PrismTextLayout.shape(PrismTextLayout.java:919) at javafx.graphics/com.sun.javafx.text.PrismTextLayout.layout(PrismTextLayout.java:1113) at javafx.graphics/com.sun.javafx.text.PrismTextLayout.ensureLayout(PrismTextLayout.java:230) at javafx.graphics/com.sun.javafx.text.PrismTextLayout.getBounds(PrismTextLayout.java:256) at javafx.graphics/javafx.scene.text.Text.getLogicalBounds(Text.java:432) at javafx.graphics/javafx.scene.text.Text.doComputeLayoutBounds(Text.java:1137) at javafx.graphics/javafx.scene.text.Text$1.doComputeLayoutBounds(Text.java:143) at javafx.graphics/com.sun.javafx.scene.shape.TextHelper.computeLayoutBoundsImpl(TextHelper.java:95) at javafx.graphics/com.sun.javafx.scene.NodeHelper.computeLayoutBounds(NodeHelper.java:108) at javafx.graphics/javafx.scene.Node$13.computeBounds(Node.java:3474) at javafx.graphics/javafx.scene.Node$LazyBoundsProperty.get(Node.java:9749) at javafx.graphics/javafx.scene.Node$LazyBoundsProperty.get(Node.java:9740) at javafx.graphics/javafx.scene.Node.getLayoutBounds(Node.java:3489) at javafx.graphics/javafx.scene.Node.prefWidth(Node.java:2989) at javafx.graphics/javafx.scene.Node.minWidth(Node.java:2930) at javafx.graphics/javafx.scene.layout.Region.computeChildMinAreaWidth(Region.java:1898) at javafx.graphics/javafx.scene.layout.HBox.getAreaWidths(HBox.java:462) at javafx.graphics/javafx.scene.layout.HBox.computeContentWidth(HBox.java:549) at javafx.graphics/javafx.scene.layout.HBox.computeMinWidth(HBox.java:409) at javafx.graphics/javafx.scene.Parent.minWidth(Parent.java:1049) at javafx.graphics/javafx.scene.layout.Region.minWidth(Region.java:1500) at javafx.graphics/javafx.scene.layout.Region.computeChildMinAreaWidth(Region.java:1898) at javafx.graphics/javafx.scene.layout.Region.getMaxAreaWidth(Region.java:2227) at javafx.graphics/javafx.scene.layout.Region.computeMaxMinAreaWidth(Region.java:2064) at javafx.graphics/javafx.scene.layout.VBox.computeMinWidth(VBox.java:399) at javafx.graphics/javafx.scene.Parent.minWidth(Parent.java:1049) at javafx.graphics/javafx.scene.layout.Region.minWidth(Region.java:1500) at javafx.graphics/javafx.scene.layout.Region.computeChildMinAreaWidth(Region.java:1898) at javafx.graphics/javafx.scene.layout.HBox.getAreaWidths(HBox.java:462) at javafx.graphics/javafx.scene.layout.HBox.computeContentWidth(HBox.java:549) at javafx.graphics/javafx.scene.layout.HBox.computeMinWidth(HBox.java:409) at javafx.graphics/javafx.scene.Parent.minWidth(Parent.java:1049) at javafx.graphics/javafx.scene.layout.Region.minWidth(Region.java:1500) at javafx.graphics/javafx.scene.layout.Region.computeChildMinAreaWidth(Region.java:1898) at javafx.graphics/javafx.scene.layout.Region.getMaxAreaWidth(Region.java:2227) at javafx.graphics/javafx.scene.layout.Region.computeMaxMinAreaWidth(Region.java:2064) at javafx.graphics/javafx.scene.layout.VBox.computeMinWidth(VBox.java:399) at javafx.graphics/javafx.scene.Parent.minWidth(Parent.java:1049) at javafx.graphics/javafx.scene.layout.Region.minWidth(Region.java:1500) at javafx.graphics/javafx.scene.layout.Region.computeChildMinAreaWidth(Region.java:1898) at javafx.graphics/javafx.scene.layout.HBox.getAreaWidths(HBox.java:462) at javafx.graphics/javafx.scene.layout.HBox.computeContentWidth(HBox.java:549) at javafx.graphics/javafx.scene.layout.HBox.computeMinWidth(HBox.java:409) at javafx.graphics/javafx.scene.Parent.minWidth(Parent.java:1049) at javafx.graphics/javafx.scene.layout.Region.minWidth(Region.java:1500) at javafx.graphics/javafx.scene.layout.Region.computeChildPrefAreaWidth(Region.java:1959) at javafx.graphics/javafx.scene.layout.Region.getMaxAreaWidth(Region.java:2228) at javafx.graphics/javafx.scene.layout.Region.computeMaxPrefAreaWidth(Region.java:2092) at
Re: RFR: 8314147: Updated the PhongMaterial documentation [v10]
On Wed, 28 Feb 2024 18:48:21 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Resize specular color images Looks good. Very well explained docs. - Marked as reviewed by arapte (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1378#pullrequestreview-1908063245
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v15]
On Thu, 29 Feb 2024 00:27:33 GMT, Andy Goryachev wrote: > P.S. Tested on on macOS 14.3.1. The results on Windows/Linux should be > identical, but if we can get some testing done on these two platforms that > would be nice. I tested the changes in windows and do not see any issues. I have executed the headful test using our headful test machines, no failures found in all platforms. Addressed inline comments. Please check. - PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-197033
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v16]
> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation > conditions were not considered, hence hit test values such as character index > and insertion index values were incorrect. > > Added checks for RTL orientation of nodes and fixed the issue in > `getHitInfo()` to calculate correct hit test values. > > Added system tests to validate the changes. Karthik P K has updated the pull request incrementally with one additional commit since the last revision: Review comments - Changes: - all: https://git.openjdk.org/jfx/pull/1323/files - new: https://git.openjdk.org/jfx/pull/1323/files/1376441b..130587c9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1323=15 - incr: https://webrevs.openjdk.org/?repo=jfx=1323=14-15 Stats: 19 lines in 3 files changed: 8 ins; 6 del; 5 mod Patch: https://git.openjdk.org/jfx/pull/1323.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1323/head:pull/1323 PR: https://git.openjdk.org/jfx/pull/1323
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v15]
On Wed, 28 Feb 2024 12:18:08 GMT, Karthik P K wrote:
>> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation
>> conditions were not considered, hence hit test values such as character
>> index and insertion index values were incorrect.
>>
>> Added checks for RTL orientation of nodes and fixed the issue in
>> `getHitInfo()` to calculate correct hit test values.
>>
>> Added system tests to validate the changes.
>
> Karthik P K has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Simplified approach
Tested with the MonkeyTester, using different justification (left, right,
center, justify) and node orientation, for both Text and TextFlow. Multiple
Text instances, rich text, inline nodes, bidi, various line breaking scenarios.
I could not find any issues! The new code also looks much cleaner and much,
much easier to understand. I left a few non-essential suggestions inline.
Overall, thank you @karthikpandelu and @hjohn.Good job guys!
P.S. Tested on on macOS 14.3.1. The results on Windows/Linux should be
identical, but if we can get some testing done on these two platforms that
would be nice.
modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java
line 212:
> 210: * and non-null in the case of {@link
> javafx.scene.text.Text}
> 211: * @param textRunStart Start position of first Text run where hit
> info is requested.
> 212: * @param curRunStart Start position of text run where hit info is
> requested.
please remove these three parameters (text, textRunStart, curRunStart)
modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
line 440:
> 438: TextRun run = null;
> 439: x -= bounds.getMinX();
> 440: //TODO binary search
we can remove this TODO because the new approach is incompatible with the
binary search
modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
line 443:
> 441: for (int i = 0; i < runs.length; i++) {
> 442: run = runs[i];
> 443: if (x < run.getWidth()) break;
I would strongly recommend placing `break`s on separate lines for convenience
during debugging.
modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
line 445:
> 443: if (x < run.getWidth()) break;
> 444: if (i + 1 < runs.length) {
> 445: if (runs[i + 1].isLinebreak()) break;
same here
modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
line 710:
> 708: while (index < lineCount) {
> 709: bottom += lines[index].getBounds().getHeight() + spacing;
> 710: if (index + 1 == lineCount) bottom -=
> lines[index].getLeading();
please keep one statement per line, if possible
if (index + 1 == lineCount) {
bottom -= lines[index].getLeading();
}
modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
line 711:
> 709: bottom += lines[index].getBounds().getHeight() + spacing;
> 710: if (index + 1 == lineCount) bottom -=
> lines[index].getLeading();
> 711: if (bottom > y) break;
same here
modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1036:
> 1034: double py = y;
> 1035:
> 1036: if(isSpan()) {
please insert a space between `if` and `(`
-
PR Review: https://git.openjdk.org/jfx/pull/1323#pullrequestreview-1907661966
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506809039
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506822747
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506822242
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506822301
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506825343
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506825604
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1506827456
Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window
On Mon, 26 Feb 2024 20:51:56 GMT, Marius Hanl wrote: > This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is > broken when `sizeToScene()` was called before or after. > > The approach here is to ignore the `sizeToScene()` request when the `Stage` > is maximized or set to fullscreen. > Otherwise the Window Manager of the OS will be confused and you will get > weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' > button still shows the 'Restore' Icon, while we already resized the `Stage` > to not be maximized). Btw, I get the following test failures on our headful Linux test systems: SizeToSceneFullscreenTest > testInitialSizeFullscreen FAILED java.lang.AssertionError: Stage height expected:<1080.0> but was:<1117.0> at org.junit.Assert.fail(Assert.java:89) at org.junit.Assert.failNotEquals(Assert.java:835) at org.junit.Assert.assertEquals(Assert.java:555) at test.javafx.stage.SizeToSceneFullscreenTest.testInitialSizeFullscreen(SizeToSceneFullscreenTest.java:78) This seems unrelated to your fix. I think the problem might be that in full-screen mode the stage can be bigger than the visible screen size or maybe the decorations are just larger on that system. You might either need to increase the tolerance values or instead check for >= visible width / height (possibly with some small tolerance). - PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1970158754
Re: RFR: 8325073: javadoc warnings: missing @param tags and other issues
On Wed, 28 Feb 2024 21:48:38 GMT, Andy Goryachev wrote: > Q: Does this PR need a CSR? Not if all of the changes are just clarification or documenting missing parameters. - PR Comment: https://git.openjdk.org/jfx/pull/1384#issuecomment-1970132810
Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window
On Mon, 26 Feb 2024 20:51:56 GMT, Marius Hanl wrote: > This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is > broken when `sizeToScene()` was called before or after. > > The approach here is to ignore the `sizeToScene()` request when the `Stage` > is maximized or set to fullscreen. > Otherwise the Window Manager of the OS will be confused and you will get > weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' > button still shows the 'Restore' Icon, while we already resized the `Stage` > to not be maximized). This seems like a reasonable fix and spec change. Have you tested the case of calling sizeToScene before setting full-screen or maximzed? Since the pending flag will still be set in that case, I want to make sure that case is tested as well. Also, if this fixed [JDK-8316425](https://bugs.openjdk.org/browse/JDK-8316425), then that bug should be closed as a duplicate of this one. @lukostyra @arapte can you also review this? - PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1970103785
RFR: 8325073: javadoc warnings: missing @param tags and other issues
This change brings the number of javadoc warnings back to 91 (to be fixed in
[JDK-8270996](https://bugs.openjdk.org/browse/JDK-8270996))
- adds missing information in `@param` tags
- adds `@SuppressWarnings("doclint:missing")` to `Skinnable` to silence the
warning due to [JDK-8325071](https://bugs.openjdk.org/browse/JDK-8325071)
- fixed an empty `` in `Subscription`
- cleaned up unnecessary `@throws` in Filtered/SortedList
Q: Does this PR need a CSR?
-
Commit messages:
- cleanup
- 8325073: javadoc warnings: missing @param tags and other issues
Changes: https://git.openjdk.org/jfx/pull/1384/files
Webrev: https://webrevs.openjdk.org/?repo=jfx=1384=00
Issue: https://bugs.openjdk.org/browse/JDK-8325073
Stats: 203 lines in 68 files changed: 108 ins; 9 del; 86 mod
Patch: https://git.openjdk.org/jfx/pull/1384.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/1384/head:pull/1384
PR: https://git.openjdk.org/jfx/pull/1384
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Wed, 28 Feb 2024 20:20:35 GMT, Martin Fox wrote: > I don't think this is a problem with the nested event loop bookkeeping. It > looks like a much simpler bug in the invokeLaterDispatcher. Well, yes and no. My testing was showing that `notifyLeftNestedEventLoop` was not even called in the finally block, so there is that. We may still have a problem at the `InvokeLaterDispatcher`, although not 100% sure since it worked good for me on Windows. But I will test it more as soon as I have more time, since I want to really check everything, so I do not miss anything here. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1969990780
Re: RFR: 8326618 : Replace usage of deprecated method getId() in Thread
On Tue, 27 Feb 2024 18:24:02 GMT, John Hendrikx wrote: > Looks okay. `threadId` was added in JDK 19, but since we're on 21 now that > should be fine. Exactly. And as I noted in the JBS issue, this fix must not be backported to earlier versions of JavaFX (it would fail to compile anyway, since we use `--release 17` for JavaFX 21 and 22). - PR Comment: https://git.openjdk.org/jfx/pull/1383#issuecomment-1969972185
Re: RFR: 8326618 : Replace usage of deprecated method getId() in Thread
On Tue, 27 Feb 2024 17:58:13 GMT, Anirvan Sarkar wrote: > Replace Thread.currentThread().getId() with Thread.currentThread().threadId() Marked as reviewed by kcr (Lead). - PR Review: https://git.openjdk.org/jfx/pull/1383#pullrequestreview-1907489547
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Thu, 22 Feb 2024 23:13:26 GMT, Marius Hanl wrote: >> This PR fixes the dialog freeze problem once and for all. >> >> This one is a bit tricky to understand, here is how it works: >> This bug happens on every platform, although the implementation of nested >> event loops differs on every platform. >> E.g. on Linux we use `gtk_main` and `gtk_main_quit`, on Windows and Mac we >> have an own implementation of a nested event loop (while loop), controlled >> by a boolean flag. >> >> Funny enough, the reason why this bug happens is always the same: Timing. >> >> 1. When we hide a dialog, `_leaveNestedEventLoop` is called. >> 2. This will call native code to get out of the nested event loop, e.g. on >> Windows we try to break out of the while loop with a boolean flag, on Linux >> we call `gtk_main_quit`. >> 3. Now, if we immediately open a new dialog, we enter a new nested event >> loop via `_enterNestedEventLoop`, as a consequence we do not break out of >> the while loop on Windows (the flag is set back again, the while loop is >> still running), and we do not return from `gtk_main` on Linux. >> 4. And this will result in the Java code never returning and calling >> `notifyLeftNestedEventLoop`, which we need to recover the UI. >> >> So it is actually not trivial to fix this problem, and we can not really do >> anything on the Java side. We may can try to wait until one more frame has >> run so that things will hopefully be right, but that sounds rather hacky. >> >> I therefore analyzed, if we even need to return from >> `_enterNestedEventLoop`. Turns out, we don't need to. >> There is a return value which we actually do not use (it does not have any >> meaning to us, other that it is used inside an assert statement). >> Without the need of a return value, we also do not need to care when >> `_enterNestedEventLoop` is returning - instead we cleanup and call >> `notifyLeftNestedEventLoop` in `_leaveNestedEventLoop`, after the native >> code was called. >> >> Lets see if this is the right approach (for all platforms). >> Testing appreciated. >> # >> - [x] Tested on Windows >> - [x] Tested on Linux >> - [x] Tested on Mac >> - [ ] Tested on iOS (although the nested event loop code is the same as for >> Mac) (I would appreciate if someone can do this as I have no access to an >> iOS device) >> - [ ] Adjust copyright >> - [ ] Write Systemtest > > Marius Hanl has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge remote-tracking branch 'openjfx/master' into > JDK-8285893-dialog-freezing-略 > - JDK-8285893: Decrement nestedEventLoopCounter in leaveNestedEventLoop > - JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen I don't think this is a problem with the nested event loop bookkeeping. It looks like a much simpler bug in the invokeLaterDispatcher. When exitNestedEventLoop is called on the innermost loop the invokeLaterDispatcher suspends operation until the loop finishes. But if you immediately start a new event loop the previous one won't finish and the dispatcher will be wedged. If you're already in a nested loop you can get into the wedged state with just two lines of code: `exitNestedEventLoop(innermost, retVal); enterNestedEventLoop(newLoop);` When the invokeLaterDispatcher is told that the innermost loop is exiting it currently sets `leavingNestedEventLoop` to true. When the dispatcher is told that a new event loop has started it is *not* clearing `leavingNestedEventLoop` but it should. Basically it should follow the same logic used in glass; leaving the innermost loop updates a boolean indicating that the loop should exit but if a new loop is started the boolean is set back to a running state since it now applies to the new loop, not the previous one. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1969844034
Re: RFR: 8314147: Updated the PhongMaterial documentation [v10]
On Wed, 28 Feb 2024 18:48:21 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Resize specular color images Looks good. - Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1378#pullrequestreview-1907046997
Re: RFR: 8314147: Updated the PhongMaterial documentation [v10]
> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass > `Material`). Except for the introduction, I divided the documentation into 3 > sections: qualitative explanation, mathematical model (I wouldn't think it > necessary, but the current doc explains it), and examples. > > The reason for the verbosity of the doc is that I envisioned 2 target > audiences for this class. One is a Java developer who wants to understand the > terminology and workings of computer graphics or of the artists who are > already familiar with this domain. (How many Java developers know what > diffuse, specular and normal maps are?) The other is an artist who is already > familiar with the domain, but wants to see how this class compares with other > renderers. For this reason, I looked at the terminology used by engines like > Blender, Maya, UE4 and Unity and tried to mention the comparisons (like bump > vs. height vs. normal maps, or specular vs. roughness/smoothness). > > The examples I chose and some of the schematics are not the best, looking at > it retroactively, but I want to give enough time for reviewers and get this > into 22. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Resize specular color images - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/e5466476..60b41ca7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1378=09 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=08-09 Stats: 0 lines in 5 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jfx/pull/1378.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1378/head:pull/1378 PR: https://git.openjdk.org/jfx/pull/1378
Re: RFR: 8314147: Updated the PhongMaterial documentation [v9]
On Wed, 28 Feb 2024 18:24:30 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Rename and resize images Actually, there are still 5 files that could use to be resized: modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/specular_color/copper_high.png: PNG image data, 820 x 820, 8-bit/color RGB, non-interlaced modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/specular_color/copper_low.png: PNG image data, 820 x 820, 8-bit/color RGB, non-interlaced modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/specular_color/copper_medium.png: PNG image data, 820 x 820, 8-bit/color RGB, non-interlaced modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/specular_color/gold_high.png: PNG image data, 820 x 820, 8-bit/color RGB, non-interlaced modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/specular_color/gold_low.png: PNG image data, 820 x 820, 8-bit/color RGB, non-interlaced - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969600849
Re: RFR: 8314147: Updated the PhongMaterial documentation [v9]
On Wed, 28 Feb 2024 18:24:30 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Rename and resize images The images look good. I'll do a final pass on the docs. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969590760
Re: RFR: 8314147: Updated the PhongMaterial documentation [v9]
On Wed, 28 Feb 2024 18:24:30 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Rename and resize images I think I got all the images' sizes reduced. Except for a few small points in discussion with Ambarish, I think that this is ready to go in in time for 22. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969586936
Re: RFR: 8314147: Updated the PhongMaterial documentation [v9]
> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass > `Material`). Except for the introduction, I divided the documentation into 3 > sections: qualitative explanation, mathematical model (I wouldn't think it > necessary, but the current doc explains it), and examples. > > The reason for the verbosity of the doc is that I envisioned 2 target > audiences for this class. One is a Java developer who wants to understand the > terminology and workings of computer graphics or of the artists who are > already familiar with this domain. (How many Java developers know what > diffuse, specular and normal maps are?) The other is an artist who is already > familiar with the domain, but wants to see how this class compares with other > renderers. For this reason, I looked at the terminology used by engines like > Blender, Maya, UE4 and Unity and tried to mention the comparisons (like bump > vs. height vs. normal maps, or specular vs. roughness/smoothness). > > The examples I chose and some of the schematics are not the best, looking at > it retroactively, but I want to give enough time for reviewers and get this > into 22. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Rename and resize images - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/d586d748..e5466476 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1378=08 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=07-08 Stats: 32 lines in 50 files changed: 0 ins; 0 del; 32 mod Patch: https://git.openjdk.org/jfx/pull/1378.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1378/head:pull/1378 PR: https://git.openjdk.org/jfx/pull/1378
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 16:01:15 GMT, Nir Lisker wrote: > > One other thing I noticed is that the file names have spaces in them, which > > is not a best practice and causes problems for scripts. Can you change all > > spaces to _ in the names of the newly-added files? > > Yes. What about folders? Also, do you prefer camelCase or the `_` naming? Generally we use all lower case for directory names, but as long as it isn't used as a package name, it isn't as important. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969357960
JavaFX 22.0.1 will be closed for fixes on March 11th
All, If you have backports that you want to get into jfx22u for JavaFX 22.0.1, please do so by Monday, March 11th. Note that approval from one of the project leads is needed as outlined in this message [1]. Thanks. -- Kevin [1] https://mail.openjdk.org/pipermail/openjfx-dev/2024-January/044659.html
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 14:21:55 GMT, Kevin Rushforth wrote: > One other thing I noticed is that the file names have spaces in them, which > is not a best practice and causes problems for scripts. Can you change all > spaces to _ in the names of the newly-added files? Yes. What about folders? Also, do you prefer camelCase or the `_` naming? - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969301404
Re: RFR: 8314147: Updated the PhongMaterial documentation [v8]
> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass > `Material`). Except for the introduction, I divided the documentation into 3 > sections: qualitative explanation, mathematical model (I wouldn't think it > necessary, but the current doc explains it), and examples. > > The reason for the verbosity of the doc is that I envisioned 2 target > audiences for this class. One is a Java developer who wants to understand the > terminology and workings of computer graphics or of the artists who are > already familiar with this domain. (How many Java developers know what > diffuse, specular and normal maps are?) The other is an artist who is already > familiar with the domain, but wants to see how this class compares with other > renderers. For this reason, I looked at the terminology used by engines like > Blender, Maya, UE4 and Unity and tried to mention the comparisons (like bump > vs. height vs. normal maps, or specular vs. roughness/smoothness). > > The examples I chose and some of the schematics are not the best, looking at > it retroactively, but I want to give enough time for reviewers and get this > into 22. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Fixed typos from review comments - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/2989624d..d586d748 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1378=07 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=06-07 Stats: 15 lines in 1 file changed: 1 ins; 3 del; 11 mod Patch: https://git.openjdk.org/jfx/pull/1378.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1378/head:pull/1378 PR: https://git.openjdk.org/jfx/pull/1378
Re: RFR: 8314147: Updated the PhongMaterial documentation [v6]
On Tue, 27 Feb 2024 12:13:09 GMT, Ambarish Rapte wrote: >> Nir Lisker has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed typo > > modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java > line 170: > >> 168: * V - the vector from the surface to the viewer (camera); >> 169: * R - the reflection vector of L from the surface. >> R can be calculated from L and N: >> 170: * R=2(L⋅N)N - L. > > Should these values be described as respect to point instead of surface ? > for example: the vector from the surface to the light source -> the vector > from the **point** to the light source The sentence above says "Four normalized vectors are considered for each point on the surface", so I think this is clear. The problem with looking at points is that they have no direction, unlike a surface, so you can't have a "normal of a point", you can only have "normal of a surface", and the reflection vector also depends on the directionality of the surface. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1506187511
Re: RFR: 8314147: Updated the PhongMaterial documentation [v6]
On Tue, 27 Feb 2024 11:24:08 GMT, Ambarish Rapte wrote: > PhongMaterial is not suitable for surfaces that reflect or refract the > incident light. But it does reflect the incident light as explained in the paragraphs before. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1506179403
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Update images Btw, if this ends up missing the deadline for JavaFX 22 (about 24 hours from now) it can go into 22u for JavaFX 22.0.1. The release date for 22.0.1 is only a few weeks after 22, and the deadline to get fixes into 22.0.1 is almost 2 weeks away. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969223783
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Update images I looked at the images in the javadoc-generated web page, and the rendered image size looks good. I recommend scaling most of the images down by at least a factor of 4 (at least a factor of 2 in both width and height), since they are much higher resolution than needed for the size they are rendered at in the page. I see that the animated gif for the writeable image is 6 Mbytes, and could probably be scaled down even more without losing quality. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969143245
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Update images Thank you for generating new images. Some of the new files are quite large, and the total added storage is 35 Mbytes, which is a lot for binary image files stored in the repo. I haven't generated the docs and looked at them yet (doing that now), but it might be better to use lower resolution images. One other thing I noticed is that the file names have spaces in them, which is not a best practice and causes problems for scripts. Can you change all spaces to `_` in the names of the newly-added files? - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969088841
Re: RFR: 8314147: Updated the PhongMaterial documentation [v6]
On Tue, 27 Feb 2024 11:53:35 GMT, Ambarish Rapte wrote: >> Nir Lisker has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed typo > > modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java > line 115: > >> 113: * >> 114: * Important: there is currently a bug that causes objects with >> 0 opacity to not render at all (despite having a >> 115: * specular or a self-illumination component). Setting the opacity to >> 1/255 instead will give the desirable result. > > Is there is JBS issue to track this? The JBS issue should have a pointer for > this comment to be removed when fixed. No, not yet. There a few oddities in the shader that I intend to look at for jfx23, this will be one of them. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1506020787
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 GMT, Nir Lisker wrote: >> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass >> `Material`). Except for the introduction, I divided the documentation into 3 >> sections: qualitative explanation, mathematical model (I wouldn't think it >> necessary, but the current doc explains it), and examples. >> >> The reason for the verbosity of the doc is that I envisioned 2 target >> audiences for this class. One is a Java developer who wants to understand >> the terminology and workings of computer graphics or of the artists who are >> already familiar with this domain. (How many Java developers know what >> diffuse, specular and normal maps are?) The other is an artist who is >> already familiar with the domain, but wants to see how this class compares >> with other renderers. For this reason, I looked at the terminology used by >> engines like Blender, Maya, UE4 and Unity and tried to mention the >> comparisons (like bump vs. height vs. normal maps, or specular vs. >> roughness/smoothness). >> >> The examples I chose and some of the schematics are not the best, looking at >> it retroactively, but I want to give enough time for reviewers and get this >> into 22. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Update images I generated the new background is an engine. I also enlarged the images a bit and added another one in the transparency section with not highlight for comparison. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969032540
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass > `Material`). Except for the introduction, I divided the documentation into 3 > sections: qualitative explanation, mathematical model (I wouldn't think it > necessary, but the current doc explains it), and examples. > > The reason for the verbosity of the doc is that I envisioned 2 target > audiences for this class. One is a Java developer who wants to understand the > terminology and workings of computer graphics or of the artists who are > already familiar with this domain. (How many Java developers know what > diffuse, specular and normal maps are?) The other is an artist who is already > familiar with the domain, but wants to see how this class compares with other > renderers. For this reason, I looked at the terminology used by engines like > Blender, Maya, UE4 and Unity and tried to mention the comparisons (like bump > vs. height vs. normal maps, or specular vs. roughness/smoothness). > > The examples I chose and some of the schematics are not the best, looking at > it retroactively, but I want to give enough time for reviewers and get this > into 22. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Update images - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/60d45bc8..2989624d Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1378=06 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=05-06 Stats: 28 lines in 20 files changed: 4 ins; 1 del; 23 mod Patch: https://git.openjdk.org/jfx/pull/1378.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1378/head:pull/1378 PR: https://git.openjdk.org/jfx/pull/1378
Re: RFR: 8324233: Update JPEG Image Decoding Software to 9f [v2]
On Wed, 28 Feb 2024 05:19:06 GMT, Jayathirth D V wrote: >> IJG has released latest version of libjpeg 9f and we need to update our >> version also match 9f changes. >> IJG reference : https://www.ijg.org/ >> >> With updated changes both headless and headful tests are green on all >> platforms. >> >> Also while updating to 9f noticed that many files don't have latest >> copyright year and comments from previous version updates like 9e, so >> updated that part also to match 9f version. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Update Looks good. All testing is green. - Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1372#pullrequestreview-1906212513
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v15]
> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation > conditions were not considered, hence hit test values such as character index > and insertion index values were incorrect. > > Added checks for RTL orientation of nodes and fixed the issue in > `getHitInfo()` to calculate correct hit test values. > > Added system tests to validate the changes. Karthik P K has updated the pull request incrementally with one additional commit since the last revision: Simplified approach - Changes: - all: https://git.openjdk.org/jfx/pull/1323/files - new: https://git.openjdk.org/jfx/pull/1323/files/f36199fd..1376441b Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1323=14 - incr: https://webrevs.openjdk.org/?repo=jfx=1323=13-14 Stats: 284 lines in 5 files changed: 14 ins; 227 del; 43 mod Patch: https://git.openjdk.org/jfx/pull/1323.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1323/head:pull/1323 PR: https://git.openjdk.org/jfx/pull/1323
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v14]
On Tue, 27 Feb 2024 16:18:31 GMT, John Hendrikx wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix repeating text node issue
>
> So, I looked into the mirroring problem as well. I had to remove a single
> line from the original `getHitInfo` code to get it to work correctly. Here's
> the image that shows the positions are all correct:
>
> 
>
> Here's the `getHitInfo` I'm using:
>
>
> @Override
> public Hit getHitInfo(float x, float y, String text, int textRunStart,
> int curRunStart) {
> int charIndex = -1;
> int insertionIndex = -1;
> boolean leading = false;
>
> ensureLayout();
> int lineIndex = getLineIndex(y);
> if (lineIndex >= getLineCount()) {
> charIndex = getCharCount();
> insertionIndex = charIndex + 1;
> } else {
> if (isMirrored()) {
> //x = getMirroringWidth() - x;
> }
> TextLine line = lines[lineIndex];
> TextRun[] runs = line.getRuns();
> RectBounds bounds = line.getBounds();
> TextRun run = null;
> x -= bounds.getMinX();
> //TODO binary search
> for (int i = 0; i < runs.length; i++) {
> run = runs[i];
> if (x < run.getWidth()) break;
> if (i + 1 < runs.length) {
> if (runs[i + 1].isLinebreak()) break;
> x -= run.getWidth();
> }
> }
> if (run != null) {
> int[] trailing = new int[1];
> charIndex = run.getStart() + run.getOffsetAtX(x, trailing);
> leading = (trailing[0] == 0);
>
> insertionIndex = charIndex;
> if (getText() != null && insertionIndex < getText().length) {
> if (!leading) {
> BreakIterator charIterator =
> BreakIterator.getCharacterInstance();
> charIterator.setText(new String(getText()));
> int next = charIterator.following(insertionIndex);
> if (next == BreakIterator.DONE) {
> insertionIndex += 1;
> } else {
> insertionIndex = next;
> }
> }
> } else if (!leading) {
> insertionIndex += 1;
> }
> } else {
> //empty line, set to line break leading
> charIndex = line.getStart();
> leading = true;
> insertionIndex ...
The simplified solution works for all the cases. Thanks @hjohn for suggesting
this code change.
I have simplified the function and removed extra parameters. I had to make same
correction for insertion index similar to character index in Text class after
getting hit info.
@andy-goryachev-oracle and @hjohn please re-review the changes.
-
PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1968857485
