Re: RFR: 8322964: Optimize performance of CSS selector matching [v9]

[Nir Lisker]

On Mon, 11 Mar 2024 16:54:25 GMT, John Hendrikx  wrote:

>> Improves performance of selector matching in the CSS subsystem. This is done 
>> by using custom set implementation which are highly optimized for the most 
>> common cases where the number of selectors is small (most commonly 1 or 2). 
>> It also should be more memory efficient for medium sized and large 
>> applications which have many style names defined in various CSS files.
>> 
>> Due to the optimization, the concept of `StyleClass`, which was only 
>> introduced to assign a fixed bit index for each unique style class name 
>> encountered, is no longer needed. This is because style classes are no 
>> longer stored in a `BitSet` which required a fixed index per encountered 
>> style class.
>> 
>> The performance improvements are the result of several factors:
>> - Less memory use when only very few style class names are used in selectors 
>> and styles from a large pool of potential styles (a `BitSet` for potentially 
>> 1000 different style names needed 1000 bits (worst case)  as it was not 
>> sparse).
>> - Specialized sets for small number of elements (0, 1, 2, 3-9 and 10+)
>> - Specialized sets are append only (reduces code paths) and can be made read 
>> only without requiring a wrapper
>> - Iterator creation is avoided when doing `containsAll` check thanks to the 
>> inverse function `isSuperSetOf`
>> - Avoids making a copy of the list of style class names to compare (to 
>> convert them to `StyleClass` and put them into a `Set`) -- this copy could 
>> not be cached and was always discarded immediately after...
>> 
>> The overall performance was tested using the JFXCentral application which 
>> displays about 800 nodes on its start page with about 1000 styles in various 
>> style sheets (and which allows to refresh this page easily).  
>> 
>> On JavaFX 20, the fastest refresh speed was 121 ms on my machine.  With the 
>> improvements in this PR, the fastest refresh had become 89 ms.  The speed 
>> improvement is the result of a 30% faster `Scene#doCSSPass`, which takes up 
>> the bulk of the time to refresh the JFXCentral main page (about 100 ms 
>> before vs 70 ms after the change).
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move getStyleClassNames to location it was introduced to reduce diff

The code looks good. I didn't test it, but I'm fine with integrating.

-

PR Comment: https://git.openjdk.org/jfx/pull/1316#issuecomment-2129765316

Re: RFR: 8332313: Update code review guidelines [v4]

[Nir Lisker]

On Tue, 21 May 2024 22:32:21 GMT, Kevin Rushforth  wrote:

>> Update the code review guidelines for JavaFX.
>> 
>> The JavaFX 
>> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
>>  guidelines includes guidance for creating, reviewing, and integrating 
>> changes to JavaFX, along with a pointer to a [Code Review 
>> Policies](https://wiki.openjdk.org/display/OpenJFX/Code+Reviews) Wiki page.
>> 
>> This PR updates these guidelines to improve the quality of reviews, with a 
>> goal of improving JavaFX and decreasing the chance of introducing a serious 
>> regression or other critical bug.
>> 
>> The source branch has three commits:
>> 1. Converts the Code Review Policies Wiki page to a 
>> [README-code-reviews.md](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/README-code-reviews.md)
>>  file in the repo and updates hyperlinks to the new location.
>> 2. Update `README-code-reviews.md` with new guidelines
>> 3. Update `CONTRIBUTING.md` to highlight important requirements  (and minor 
>> changes to `README-code-reviews.md`)
>> 
>> Commit 1 is content neutral, so it might be helpful for reviewers to look at 
>> the changes starting from the second commit.
>> 
>> The updates are:
>> 
>> * In the Overview section, add a list of items for Reviewers, PR authors, 
>> and sponsoring Committers to verify prior to integration
>> * Create a "Guidelines for reviewing a PR" subsection of the Code review 
>> policies section
>> * Create a "Before you integrate or sponsor a PR" subsection of the Code 
>> review policies section
>> * Update the `CONTRIBUTING.md` page to highlight important requirements
>
> Kevin Rushforth has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Clarify need for CSR when API is modified or removed as well as added

Marked as reviewed by nlisker (Reviewer).

-

PR Review: https://git.openjdk.org/jfx/pull/1455#pullrequestreview-2069892902

Re: RFR: 8332313: Update code review guidelines [v2]

[Nir Lisker]

On Sat, 18 May 2024 14:24:49 GMT, Kevin Rushforth  wrote:

> Maybe it's worth changing "adds any new" to "adds, removes, or modifies any"?

This looks like a good idea.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605798488

Re: RFR: 8332313: Update code review guidelines [v2]

[Nir Lisker]

On Fri, 17 May 2024 14:58:41 GMT, Nir Lisker  wrote:

>> Kevin Rushforth 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 20 additional 
>> commits since the last revision:
>> 
>>  - Wait for re-review if you've pushed a change addressing a substantive 
>> comment
>>  - Typo + remove sentence that invites reformatting PRs
>>  - Wording changes, added example of API addition
>>  - Formatting
>>  - Add item about checking the target branch
>>  - Remove trailing period from previous
>>  - Minor changes regarding syncing from upstream master
>>  - Clarify that GHA tests might fail for a reason unrelated to the PR
>>  - Fix typo
>>
>>"but It" --> "but it"
>>  - Remove bullet about checking the commit message (Skara already checks)
>>  - ... and 10 more: https://git.openjdk.org/jfx/compare/1b088e5b...9cf7f920
>
> README-code-reviews.md line 72:
> 
>> 70: * Focus first on substantive comments rather than stylistic comments
>> 71: * Check whether there is an automated test; if not, ask for one, if it 
>> is feasible
>> 72: * Make sure that the PR has executed the GitHub Actions (GHA) tests; if 
>> they aren't being run, ask the PR author to enable GHA workflows; if the 
>> test fails on some platforms, check whether it is a real bug (sometimes a 
>> job fails becau se of GHA infrastucture changes or we see a spurious GHA 
>> failure)
> 
> becau se -> because

Never mind, got simul-fixed.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605160705

Re: RFR: 8332313: Update code review guidelines [v2]

[Nir Lisker]

On Fri, 17 May 2024 14:10:43 GMT, Kevin Rushforth  wrote:

>> Update the code review guidelines for JavaFX.
>> 
>> The JavaFX 
>> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
>>  guidelines includes guidance for creating, reviewing, and integrating 
>> changes to JavaFX, along with a pointer to a [Code Review 
>> Policies](https://wiki.openjdk.org/display/OpenJFX/Code+Reviews) Wiki page.
>> 
>> This PR updates these guidelines to improve the quality of reviews, with a 
>> goal of improving JavaFX and decreasing the chance of introducing a serious 
>> regression or other critical bug.
>> 
>> The source branch has three commits:
>> 1. Converts the Code Review Policies Wiki page to a 
>> [README-code-reviews.md](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/README-code-reviews.md)
>>  file in the repo and updates hyperlinks to the new location.
>> 2. Update `README-code-reviews.md` with new guidelines
>> 3. Update `CONTRIBUTING.md` to highlight important requirements  (and minor 
>> changes to `README-code-reviews.md`)
>> 
>> Commit 1 is content neutral, so it might be helpful for reviewers to look at 
>> the changes starting from the second commit.
>> 
>> The updates are:
>> 
>> * In the Overview section, add a list of items for Reviewers, PR authors, 
>> and sponsoring Committers to verify prior to integration
>> * Create a "Guidelines for reviewing a PR" subsection of the Code review 
>> policies section
>> * Create a "Before you integrate or sponsor a PR" subsection of the Code 
>> review policies section
>> * Update the `CONTRIBUTING.md` page to highlight important requirements
>
> Kevin Rushforth 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 20 additional 
> commits since the last revision:
> 
>  - Wait for re-review if you've pushed a change addressing a substantive 
> comment
>  - Typo + remove sentence that invites reformatting PRs
>  - Wording changes, added example of API addition
>  - Formatting
>  - Add item about checking the target branch
>  - Remove trailing period from previous
>  - Minor changes regarding syncing from upstream master
>  - Clarify that GHA tests might fail for a reason unrelated to the PR
>  - Fix typo
>
>"but It" --> "but it"
>  - Remove bullet about checking the commit message (Skara already checks)
>  - ... and 10 more: https://git.openjdk.org/jfx/compare/1b088e5b...9cf7f920

README-code-reviews.md line 69:

> 67: * Carefully consider the risk of regression
> 68: * Carefully consider any compatibility concerns
> 69: * Check whether it adds any new public or protected API, even implicitly 
> (such as a public method that overrides a protected method, or a class that 
> is moved from a non-exported to an exported package); if it does, indicate 
> that it needs a CSR

I think that if it looks like an oversight (the author forgot about the default 
constructor), it's better to indicate that than to indicate that the PR needs a 
CSR. Maybe something like:
"if it does and it doesn't look like an oversight, indicate that it needs a CSR"

README-code-reviews.md line 72:

> 70: * Focus first on substantive comments rather than stylistic comments
> 71: * Check whether there is an automated test; if not, ask for one, if it is 
> feasible
> 72: * Make sure that the PR has executed the GitHub Actions (GHA) tests; if 
> they aren't being run, ask the PR author to enable GHA workflows; if the test 
> fails on some platforms, check whether it is a real bug (sometimes a job 
> fails becau se of GHA infrastucture changes or we see a spurious GHA failure)

becau se -> because

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605152176
PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605147892

Re: RFR: 8332328: [GHA] GitHub Actions build fails on Linux: unable to find gcc-13

[Nir Lisker]

On Thu, 16 May 2024 15:42:05 GMT, Kevin Rushforth  wrote:

> It looks like the list of packages available in the Ubuntu 22.04 GitHub 
> Actions test runner no longer includes `gcc-13` as of yesterday. I didn't 
> find any documentation to indicate that this was intentional, but now that 
> Ubuntu 24.04 is available we can fix the problem by updating to that version. 
> We will need to do this at some point anyway, so we might as well do it now.
> 
> With this fix, the GHA test build now passes on all platforms. See the test 
> results to verify this.
> 
> Given that GHA builds are currently broken on Linux, and that this fix 
> doesn't affect the product at all (just our GHA tests), I will integrate this 
> as soon as it is approved by 1 Reviewer, as an exception to the usual 
> requirement to wait for 24 hours.

Note that the Ubuntu 24 runner is in beta: 
https://github.com/actions/runner-images#available-images.

-

PR Comment: https://git.openjdk.org/jfx/pull/1457#issuecomment-2115686652

Re: RFR: 8332313: Update code review guidelines

[Nir Lisker]

On Wed, 15 May 2024 20:08:10 GMT, John Hendrikx  wrote:

>> or is it?  :-)
>
> Isn't this automatic? Seems weird you could integrate without these passing.

> or is it? :-)

![image](https://github.com/openjdk/jfx/assets/37422899/8daab7cf-f050-4964-b8a6-731666422293)

Looks to me like it is...

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1602201785

Re: RFR: 8332313: Update code review guidelines

[Nir Lisker]

On Wed, 15 May 2024 17:45:56 GMT, Kevin Rushforth  wrote:

> Update the code review guidelines for JavaFX.
> 
> The JavaFX 
> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
>  guidelines includes guidance for creating, reviewing, and integrating 
> changes to JavaFX, along with a pointer to a [Code Review 
> Policies](https://wiki.openjdk.org/display/OpenJFX/Code+Reviews) Wiki page.
> 
> This PR updates these guidelines to improve the quality of reviews, with a 
> goal of improving JavaFX and decreasing the chance of introducing a serious 
> regression or other critical bug.
> 
> The source branch has three commits:
> 1. Converts the Code Review Policies Wiki page to a 
> [README-code-reviews.md](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/README-code-reviews.md)
>  file in the repo and updates hyperlinks to the new location.
> 2. Update `README-code-reviews.md` with new guidelines
> 3. Update `CONTRIBUTING.md` to highlight important requirements  (and minor 
> changes to `README-code-reviews.md`)
> 
> Commit 1 is content neutral, so it might be helpful for reviewers to look at 
> the changes starting from the second commit.
> 
> The updates are:
> 
> * In the Overview section, add a list of items for Reviewers, PR authors, and 
> sponsoring Committers to verify prior to integration
> * Create a "Guidelines for reviewing a PR" subsection of the Code review 
> policies section
> * Create a "Before you integrate or sponsor a PR" subsection of the Code 
> review policies section
> * Update the `CONTRIBUTING.md` page to highlight important requirements

README-code-reviews.md line 48:

> 46: All code reviews must be done via a pull request submitted against this 
> GitHub repo, [openjdk/jfx](https://github.com/openjdk/jfx). A JBS bug ID must 
> exist before the pull request will be reviewed. See 
> [CONTRIBUTING.md](CONTRIBUTING.md) for information on how to submit a pull 
> request.
> 47: 
> 48: All fixes must be reviewed by at least one reviewer with the "Reviewer" 
> role (aka a "R"eviewer). We have a different code review threshold for 
> different types of changes. If there is disagreement as to whether a fix is 
> low-impact or high-impact, then it is considered high-impact. In other words 
> we will always err on the side of quality by "rounding up" to the next higher 
> category. The contributor can say whether they think something is low-impact 
> or high-impact, but It is up to a Reviewer to confirm this. A Reviewer either 
> adds a comment indicating that they think a single review is sufficient, or 
> else issues the Skara `/reviewers 2` command requesting a second reviewer (a 
> Reviewer can request more than 2 reviewers in some cases where a fix might be 
> especially risky or cut across multiple functional areas).

"but **It** is" -> it

README-code-reviews.md line 48:

> 46: All code reviews must be done via a pull request submitted against this 
> GitHub repo, [openjdk/jfx](https://github.com/openjdk/jfx). A JBS bug ID must 
> exist before the pull request will be reviewed. See 
> [CONTRIBUTING.md](CONTRIBUTING.md) for information on how to submit a pull 
> request.
> 47: 
> 48: All fixes must be reviewed by at least one reviewer with the "Reviewer" 
> role (aka a "R"eviewer). We have a different code review threshold for 
> different types of changes. If there is disagreement as to whether a fix is 
> low-impact or high-impact, then it is considered high-impact. In other words 
> we will always err on the side of quality by "rounding up" to the next higher 
> category. The contributor can say whether they think something is low-impact 
> or high-impact, but It is up to a Reviewer to confirm this. A Reviewer either 
> adds a comment indicating that they think a single review is sufficient, or 
> else issues the Skara `/reviewers 2` command requesting a second reviewer (a 
> Reviewer can request more than 2 reviewers in some cases where a fix might be 
> especially risky or cut across multiple functional areas).

About requesting reviews. I think that only some people can request reviews 
through GitHub, I never managed to do it on this repo, probably a matter of 
permissions. Might worth clarifying how this works.

README-code-reviews.md line 58:

> 56: * Determine whether this needs 2 reviewers and whether it needs a CSR; 
> issue the `/reviewers 2` or `/csr` command as needed
> 57: * If you want to indicate your approval, but still feel additional 
> reviewers are needed, you may increase the number of reviewers (e.g., from 2 
> to 3)
> 58: * If you want an area expert to review a PR, indicate this in a comment 
> of the form: `Reviewers: @PERSON1 @PERSON2`; the requested reviewers can 
> indicate whether or not they plan to review it

Should a list of experts per area be available somewhere? The Wiki has an old 
"code ownership" table that is out of date. Usually you get to know the experts 
only after they have reviewed 

Gradle support

[Nir Lisker]

Hello,

I see that dependency scanning is supported for Maven pom files, however,
many projects use Gradle as their dependency management tool. Has Gradle
support been considered?

Best,
Nir

Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex [v3]

[Nir Lisker]

On Fri, 26 Apr 2024 07:53:59 GMT, Lukasz Kostyra  wrote:

>> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are 
>> no longer needed.
>> 
>> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I 
>> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it 
>> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it 
>> was leveraged to transparently use the Ex device in the backend) but now we 
>> don't have the non-Ex device, so that keeps it a bit more consistent and 
>> clear IMO.
>> 
>> Verified by running tests on Windows 11, did not notice any regressions. 
>> Unfortunately I have no way to test this on older systems.
>
> Lukasz Kostyra has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change pd3dEx to pd3d9

Marked as reviewed by nlisker (Reviewer).

-

PR Review: https://git.openjdk.org/jfx/pull/1445#pullrequestreview-2024451797

Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex

[Nir Lisker]

On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra  wrote:

> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are 
> no longer needed.
> 
> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I 
> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it 
> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was 
> leveraged to transparently use the Ex device in the backend) but now we don't 
> have the non-Ex device, so that keeps it a bit more consistent and clear IMO.
> 
> Verified by running tests on Windows 11, did not notice any regressions. 
> Unfortunately I have no way to test this on older systems.

Tested the functionality with the 3DLighting app, things look the same as 
before the patch. Left a minor comment.

modules/javafx.graphics/src/main/native-prism-d3d/D3DPipeline.cc line 237:

> 235: }
> 236: 
> 237: int getMaxSampleSupport(IDirect3D9Ex *d3d9, UINT adapter) {

Minor: In some cases you also change the name of the variable to add the "Ex" 
suffix., like in

D3DContext::D3DContext(IDirect3D9Ex *pd3dEx, UINT adapter)
 ^

Here and In `PiplineManager.h` it's left as `IDirect3D9Ex * pd3d9;` without 
"Ex".
I don't mind it either way (I would probably not bother changing them myself), 
but perhaps we should remain consistent.

-

Marked as reviewed by nlisker (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1445#pullrequestreview-2022879147
PR Review Comment: https://git.openjdk.org/jfx/pull/1445#discussion_r1579699815

Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex

[Nir Lisker]

On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra  wrote:

> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are 
> no longer needed.
> 
> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I 
> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it 
> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was 
> leveraged to transparently use the Ex device in the backend) but now we don't 
> have the non-Ex device, so that keeps it a bit more consistent and clear IMO.
> 
> Verified by running tests on Windows 11, did not notice any regressions. 
> Unfortunately I have no way to test this on older systems.

Found the problem. That merge bumped the min JDK to 21, which I was using 
anyway, but it required recompiling the native D3D resources with the new JDK. 
The application works now.

-

PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2077287504

Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex

[Nir Lisker]

On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra  wrote:

> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are 
> no longer needed.
> 
> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I 
> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it 
> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was 
> leveraged to transparently use the Ex device in the backend) but now we don't 
> have the non-Ex device, so that keeps it a bit more consistent and clear IMO.
> 
> Verified by running tests on Windows 11, did not notice any regressions. 
> Unfortunately I have no way to test this on older systems.

I traced the issue to commit id 3914db26f3abb573ed0e320a361477e1d3e7a9ac, which 
is a merge Kevin did ~6 weeks ago. Placing the head on this commit or after 
causes the above error, but moving 1 commit back to "Create release notes for 
JavaFX 22" lets the application start normally.

I understand that this PR has nothing to do with this, I just can't test it 
considering this problem.

-

PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2076940263

Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex

[Nir Lisker]

On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra  wrote:

> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are 
> no longer needed.
> 
> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I 
> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it 
> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was 
> leveraged to transparently use the Ex device in the backend) but now we don't 
> have the non-Ex device, so that keeps it a bit more consistent and clear IMO.
> 
> Verified by running tests on Windows 11, did not notice any regressions. 
> Unfortunately I have no way to test this on older systems.

I tried to run the 3DLighting application and I'm getting an error. However, it 
looks like it's not this patch that is causing it since going back a few 
commits also gives this error. The jfx22 branch doesn't have this issue so it 
will take some investigation to find out what's going on. I'm on Win 10.


#
# A fatal error has been detected by the Java Runtime Environment:
#
#  EXCEPTION_ACCESS_VIOLATION (0xc005) at pc=0x01b44b9ad260, pid=6332, 
tid=18096
#
# JRE version: OpenJDK Runtime Environment (21.0.1+12) (build 21.0.1+12-29)
# Java VM: OpenJDK 64-Bit Server VM (21.0.1+12-29, mixed mode, sharing, tiered, 
compressed oops, compressed class ptrs, g1 gc, windows-amd64)
# Problematic frame:
# J 170 c2 java.lang.String.length()I java.base@21.0.1 (11 bytes) @ 
0x01b44b9ad260 [0x01b44b9ad220+0x0040]
#
# No core dump will be written. Minidumps are not enabled by default on client 
versions of Windows
#
# An error report file with more information is saved as:
# C:\Users\Nir\git\jfx\tests\performance\3DLighting\hs_err_pid6332.log
[5.830s][warning][os] Loading hsdis library failed
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#

-

PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2076903265

Re: Wayland

[Nir Lisker]

Not sure it helps with warmup, but marking a foreign function as critical
can improve performance:
https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/foreign/Linker.Option.html#critical(boolean)
.

On Mon, Apr 22, 2024 at 10:02 PM Philip Race  wrote:

> No, it wasn't. I didn't even use jextracted code.
> The startup cost is around initialisation of FFM - around 70 ms (IIRC)
> overhead on my MacBook
> Then creation of VarHandles and MethodHandles - 2-5 ms each is what I
> measured, so do these lazily if you can.
> And warmup cost is that it takes about 1 iterations to get code fully
> compiled.
>
> java -XX:+PrintFlagsFinal -version 2>&1 | grep CompileThreshold
>  intx CompileThreshold =
> 1  {pd product} {default}
> double CompileThresholdScaling  =
> 1.00  {product} {default}
> uintx IncreaseFirstTierCompileThresholdAt  =
> 50{product} {default}
>  intx Tier2CompileThreshold=
> 0 {product} {default}
>  intx Tier3CompileThreshold=
> 2000  {product} {default}
>  intx Tier4CompileThreshold=
> 15000 {product} {default}
>
> -phil.
>
>
> On 4/22/24 11:45 AM, Thiago Milczarek Sayão wrote:
>
> I think the startup time might be related to all static symbol lookups.
> So I'm manually including just what is needed:
>
> jextract --output src -t com.sun.glass.wayland.extracted \
>   --header-class-name GlassWayland \
>   `pkg-config --libs glib-2.0 gio-2.0 libportal wayland-client` \
>   `pkg-config --cflags-only-I glib-2.0 gio-2.0 libportal wayland-client` \
>glass-wayland.h \
>--include-function xdp_portal_initable_new \
>--include-function xdp_session_close \
>--include-function xdp_portal_open_file \
>--include-function xdp_portal_open_file_finish \
>--include-function g_object_unref \
>--include-function g_timeout_add \
>--include-function g_add_idle \
>--include-function g_main_loop_run \
>--include-function g_main_loop_new \
>--include-function g_main_loop_ref \
>--include-function g_main_loop_unref \
>--include-function g_main_loop_quit \
>--include-function g_settings_new \
>--include-function g_settings_get_int \
>--include-function wl_display_connect \
>--include-function wl_display_disconnect \
>--include-function wl_display_roundtrip \
>--include-function wl_display_dispatch_pending \
>--include-typedef GAsyncReadyCallback \
>--include-typedef GSourceFunc \
>--include-typedef GError
>
>
> Em seg., 22 de abr. de 2024 às 13:24, Philip Race 
> escreveu:
>
>> As a reminder, using FFM will require all FX *applications* to specify
>> --enable-native-access on the command line
>> Although this is likely coming to JNI soon too.
>>
>> https://docs.oracle.com/en/java/javase/21/core/restricted-methods.html
>>
>> But one thing to watch out for with FFM is startup + warm up time.
>> I struggled a lot with that in using FFM for just one library in the
>> java.desktop module.
>>
>> -phil
>>
>> On 4/22/24 9:12 AM, Nir Lisker wrote:
>>
>> Sorry, we bumped to Java 21 in JavaFX 22 I think since we preserve the
>> N-1 rule.
>>
>> On Mon, Apr 22, 2024 at 6:03 PM Nir Lisker  wrote:
>>
>>> I think that we'll be able to bump to Java 25 in JavaFX 25, like we did
>>> with 21. I suggested initially to bump to Java 22 exactly for FFM as it's
>>> very useful for JavaFX, but was told we shouldn't since it's not an LTS
>>> version.
>>>
>>> I have no idea how long the work on Wayland will take including the code
>>> review (a rather long process), but you should be able to request code
>>> reviews with FFM and have it ready for integration by Java 25.
>>>
>>> On Mon, Apr 22, 2024 at 5:49 PM Thiago Milczarek Sayão <
>>> thiago.sa...@gmail.com> wrote:
>>>
>>>> I was just experimenting, but it seems to be less work than going with
>>>> JNI.
>>>> If I am correct, the next Java LTS will be 25, which will be required
>>>> on JavaFX 29 to be released on September/29.
>>>>
>>>> It's 7 years - that's really too much.
>>>>
>>>> Maybe it's still worthwhile to prototype using FFM and then port
>>>> everything to JNI.
>>>>
>>>> -- Thiago.
>&g

Re: Wayland

[Nir Lisker]

Sorry, we bumped to Java 21 in JavaFX 22 I think since we preserve the N-1
rule.

On Mon, Apr 22, 2024 at 6:03 PM Nir Lisker  wrote:

> I think that we'll be able to bump to Java 25 in JavaFX 25, like we did
> with 21. I suggested initially to bump to Java 22 exactly for FFM as it's
> very useful for JavaFX, but was told we shouldn't since it's not an LTS
> version.
>
> I have no idea how long the work on Wayland will take including the code
> review (a rather long process), but you should be able to request code
> reviews with FFM and have it ready for integration by Java 25.
>
> On Mon, Apr 22, 2024 at 5:49 PM Thiago Milczarek Sayão <
> thiago.sa...@gmail.com> wrote:
>
>> I was just experimenting, but it seems to be less work than going with
>> JNI.
>> If I am correct, the next Java LTS will be 25, which will be required on
>> JavaFX 29 to be released on September/29.
>>
>> It's 7 years - that's really too much.
>>
>> Maybe it's still worthwhile to prototype using FFM and then port
>> everything to JNI.
>>
>> -- Thiago.
>>
>>
>> Em seg., 22 de abr. de 2024 às 11:21, Kevin Rushforth <
>> kevin.rushfo...@oracle.com> escreveu:
>>
>>> Note also that we cannot use Panama in the JavaFX internals yet, since
>>> the minimum version of the JDK is 21.
>>>
>>> -- Kevin
>>>
>>>
>>> On 4/21/2024 10:51 AM, Thiago Milczarek Sayão wrote:
>>> > Hi,
>>> >
>>> > I did a small test app to explore Wayland client and portals (for
>>> > Robot and dialogs such as file open/save).
>>> >
>>> > https://github.com/tsayao/wayland-test/blob/main/wayland-test.c
>>> >
>>> > It seems it will work as a glass backend, but some walls will be hit
>>> > on the way :)
>>> >
>>> > I have tried to use jextract (from project Panama) to work directly
>>> > with java, but it seems it does not support wl_ types.
>>> >
>>> > -- Thiago.
>>>
>>>

Re: Wayland

[Nir Lisker]

I think that we'll be able to bump to Java 25 in JavaFX 25, like we did
with 21. I suggested initially to bump to Java 22 exactly for FFM as it's
very useful for JavaFX, but was told we shouldn't since it's not an LTS
version.

I have no idea how long the work on Wayland will take including the code
review (a rather long process), but you should be able to request code
reviews with FFM and have it ready for integration by Java 25.

On Mon, Apr 22, 2024 at 5:49 PM Thiago Milczarek Sayão <
thiago.sa...@gmail.com> wrote:

> I was just experimenting, but it seems to be less work than going with JNI.
> If I am correct, the next Java LTS will be 25, which will be required on
> JavaFX 29 to be released on September/29.
>
> It's 7 years - that's really too much.
>
> Maybe it's still worthwhile to prototype using FFM and then port
> everything to JNI.
>
> -- Thiago.
>
>
> Em seg., 22 de abr. de 2024 às 11:21, Kevin Rushforth <
> kevin.rushfo...@oracle.com> escreveu:
>
>> Note also that we cannot use Panama in the JavaFX internals yet, since
>> the minimum version of the JDK is 21.
>>
>> -- Kevin
>>
>>
>> On 4/21/2024 10:51 AM, Thiago Milczarek Sayão wrote:
>> > Hi,
>> >
>> > I did a small test app to explore Wayland client and portals (for
>> > Robot and dialogs such as file open/save).
>> >
>> > https://github.com/tsayao/wayland-test/blob/main/wayland-test.c
>> >
>> > It seems it will work as a glass backend, but some walls will be hit
>> > on the way :)
>> >
>> > I have tried to use jextract (from project Panama) to work directly
>> > with java, but it seems it does not support wl_ types.
>> >
>> > -- Thiago.
>>
>>

Re: Wayland

[Nir Lisker]

Ah, yes, opaque types are indeed unsupported:
https://github.com/openjdk/jextract/blob/master/doc/GUIDE.md#unsupported-features.
As Jorn said there, if the API exposes methods that use opaque types, then
you wouldn't have a problem. Also, if you have the .c file where they are
defined, jextract can handle them. It could be a bit of a hack though.

I wrote a jextract GUI wrapper with JavaFX, which I tested only on Windows
for now. I will try to get the Linux and Mac versions up soon as well. I
find it very helpful compared to the command line and I think it could help
you with the complex headers there.

Note that jextract generates Java 22 compatible code, which is unusable in
JavaFX for now (we comply with Java 21).

On Mon, Apr 22, 2024 at 1:36 AM Thiago Milczarek Sayão <
thiago.sa...@gmail.com> wrote:

> I mailed the jextract list and Jorn Vernee explained that wayland use
> opaque types, which are just defined as such:
>
> struct wl_registry;
>
> I guess you just pass it around and it's defined in the internal .c file.
> Those are not supported by jextract.
>
> I'll find a way to get around it.
>
> But I've been playing with it all day, it seems very good. I was able to
> generate bindings for:
>
> GMain - for the main loop;
> GSettings - for reading settings;
> XDG Portal - for screen capture, screenshot, file dialogs
>
> It looks like this:
>
> 1) To get a setting
>
> try(var a = Arena.ofConfined()) {
> var mouseSettings = 
> g_settings_new(a.allocateUtf8String("org.gnome.desktop.interface"));
> int size = g_settings_get_int(mouseSettings, 
> a.allocateUtf8String("cursor-size"));
> g_object_unref(mouseSettings);
> return new Size(size, size);
> }
>
>
> 2) Callbacks
>
> @Override
> protected void _invokeLater(Runnable runnable) {
> MemorySegment call = GSourceFunc.allocate(p -> {
> runnable.run();
> return 0;
> }, Arena.ofAuto());
>
> g_idle_add(call, MemorySegment.NULL);
> }
>
>
> It looks correct to me, but untested.
>
> -- Thiago.
>
> Em dom., 21 de abr. de 2024 às 18:15, Nir Lisker 
> escreveu:
>
>> Can you link to where all the headers are? I found some in
>> https://gitlab.freedesktop.org/wayland/wayland/-/tree/main/src, but I
>> couldn't see where wl_registry is defined. From what I see, wl_XYZ types
>> are structs, which are supported.
>>
>> By the way, there's a new guide for jextract at
>> https://github.com/openjdk/jextract/blob/master/doc/GUIDE.md. When
>> working with complex headers (includes upon includes), it's important to
>> specify the correct target header.
>>
>> On Sun, Apr 21, 2024 at 11:35 PM Thiago Milczarek Sayão <
>> thiago.sa...@gmail.com> wrote:
>>
>>> jextract --output src -t org.freedesktop.wayland.client
>>> --header-class-name WlClient `pkg-config --cflags-only-I wayland-client`
>>> `pkg-config --libs wayland-client`  /usr/include/wayland-client.h
>>>
>>> WARNING: Skipping wl_registry (type Declared(wl_registry) is not
>>> supported)
>>>
>>> I would need this to hook the events with wl_registry_add_listener, but
>>> currently the code generation for this is not working.
>>>
>>>
>>>
>>>
>>>
>>> Em dom., 21 de abr. de 2024 às 16:39, Nir Lisker 
>>> escreveu:
>>>
>>>> What are wl_ types? jextract only supports c headers.
>>>>
>>>> On Sun, Apr 21, 2024 at 10:31 PM Thiago Milczarek Sayão <
>>>> thiago.sa...@gmail.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I did a small test app to explore Wayland client and portals (for
>>>>> Robot and dialogs such as file open/save).
>>>>>
>>>>> https://github.com/tsayao/wayland-test/blob/main/wayland-test.c
>>>>>
>>>>> It seems it will work as a glass backend, but some walls will be hit
>>>>> on the way :)
>>>>>
>>>>> I have tried to use jextract (from project Panama) to work directly
>>>>> with java, but it seems it does not support wl_ types.
>>>>>
>>>>> -- Thiago.
>>>>>
>>>>

Re: Wayland

[Nir Lisker]

Can you link to where all the headers are? I found some in
https://gitlab.freedesktop.org/wayland/wayland/-/tree/main/src, but I
couldn't see where wl_registry is defined. From what I see, wl_XYZ types
are structs, which are supported.

By the way, there's a new guide for jextract at
https://github.com/openjdk/jextract/blob/master/doc/GUIDE.md. When working
with complex headers (includes upon includes), it's important to specify
the correct target header.

On Sun, Apr 21, 2024 at 11:35 PM Thiago Milczarek Sayão <
thiago.sa...@gmail.com> wrote:

> jextract --output src -t org.freedesktop.wayland.client
> --header-class-name WlClient `pkg-config --cflags-only-I wayland-client`
> `pkg-config --libs wayland-client`  /usr/include/wayland-client.h
>
> WARNING: Skipping wl_registry (type Declared(wl_registry) is not supported)
>
> I would need this to hook the events with wl_registry_add_listener, but
> currently the code generation for this is not working.
>
>
>
>
>
> Em dom., 21 de abr. de 2024 às 16:39, Nir Lisker 
> escreveu:
>
>> What are wl_ types? jextract only supports c headers.
>>
>> On Sun, Apr 21, 2024 at 10:31 PM Thiago Milczarek Sayão <
>> thiago.sa...@gmail.com> wrote:
>>
>>> Hi,
>>>
>>> I did a small test app to explore Wayland client and portals (for Robot
>>> and dialogs such as file open/save).
>>>
>>> https://github.com/tsayao/wayland-test/blob/main/wayland-test.c
>>>
>>> It seems it will work as a glass backend, but some walls will be hit on
>>> the way :)
>>>
>>> I have tried to use jextract (from project Panama) to work directly with
>>> java, but it seems it does not support wl_ types.
>>>
>>> -- Thiago.
>>>
>>

Re: Wayland

[Nir Lisker]

What are wl_ types? jextract only supports c headers.

On Sun, Apr 21, 2024 at 10:31 PM Thiago Milczarek Sayão <
thiago.sa...@gmail.com> wrote:

> Hi,
>
> I did a small test app to explore Wayland client and portals (for Robot
> and dialogs such as file open/save).
>
> https://github.com/tsayao/wayland-test/blob/main/wayland-test.c
>
> It seems it will work as a glass backend, but some walls will be hit on
> the way :)
>
> I have tried to use jextract (from project Panama) to work directly with
> java, but it seems it does not support wl_ types.
>
> -- Thiago.
>

Re: Possible leak on setOnAction

[Nir Lisker]

I didn't find such memory leaks in my application, though I don't do stage
handling. What I would look at is where the `stage` reference in the lambda
is coming from. You say you have a list of open stages. When you close a
stage, do you remove the references to that stage from all places? What
about the object that is holding the reference `stage` in the lambda?

On Thu, Apr 18, 2024 at 1:58 PM Thiago Milczarek Sayão <
thiago.sa...@gmail.com> wrote:

> Hi,
>
> I'm pretty sure setOnAction is holding references.
>
> I have a "Open Windows" menu on my application where it lists the Stages
> opened and if you click, it calls stage.toFront():
>
> menuItem.seOnAction(e -> stage.toFront())
>
> I had many crash reports, all OOM. I got the hprof files and analyzed them
> - turns out this was holding references to all closed stages.
>
> To fix it, I call setOnAction(null) when the stage is closed.
>
> I will investigate further and provide an example.
>
> -- Thiago.
>

Re: How to configure the default connection for virsh

[Nir Soffer]

On Thu, Apr 18, 2024 at 5:20 PM Daniel P. Berrangé 
wrote:

> On Thu, Apr 18, 2024 at 05:07:24PM +0300, Nir Soffer wrote:
> > We are using minikube vms, which require adding the user to to libvirt
> > group[1].
> > We use `virsh -c qemu:///system` for debugging these vms or related
> libvirt
> > networks.
> >
> > Using virsh without specifying the '-c' argument is a common mistake that
> > leads to
> > trouble, for example creating the libvirt default network incorrectly.
> >
> > I wonder if it is possible to configure virsh so it uses -c
> qemu:///system
> > by default?
>
> The $HOME/.config/libvirt/libvirt.conf client config file can contain
>
>uri_default = "qemu:///system"
>
> Or you can set per shell with
>
>export LIBVIRT_DEFAULT_URI=qemu:///system
>
> Or you can set it just for virsh with
>
>export VIRSH_DEFAULT_CONNECT_URI=qemu:///system
>
> See:
>
>   https://libvirt.org/uri.html#default-uri-choice
>   https://libvirt.org/manpages/virsh.html#environment
>
>
Awesome, thanks!
___
Users mailing list -- users@lists.libvirt.org
To unsubscribe send an email to users-le...@lists.libvirt.org

How to configure the default connection for virsh

[Nir Soffer]

We are using minikube vms, which require adding the user to to libvirt
group[1].
We use `virsh -c qemu:///system` for debugging these vms or related libvirt
networks.

Using virsh without specifying the '-c' argument is a common mistake that
leads to
trouble, for example creating the libvirt default network incorrectly.

I wonder if it is possible to configure virsh so it uses -c qemu:///system
by default?

We know that this is not great, but we don't know a better way to use
minikube
with the kvm2 driver. The minikube vms are started for creating a
development
environment and during CI, and they cannot require a password or any
complicated
setup.

[1] https://github.com/kubernetes/minikube/issues/3467

Nir
___
Users mailing list -- users@lists.libvirt.org
To unsubscribe send an email to users-le...@lists.libvirt.org

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v3]

[Nir Lisker]

On Thu, 28 Mar 2024 22:21:32 GMT, drmarmac  wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java 
>> line 166:
>> 
>>> 164: sm.startAtomic();
>>> 165: 
>>> 166: final List removed = new 
>>> ArrayList<>(c.getRemovedSize());
>> 
>> I wonder if we should add a check for 0 size here to bypass all this code 
>> and unnecessary object allocations if nothing is removed (same for added)
>
> We certainly could, or maybe use wasRemoved(), but I doubt there will be much 
> impact. I guess it's preferred to keep changes unrelated to the issue 
> minimal, so I'd leave it as it is if everyone's ok with that.

These short circuiting operations tend to be worth it only if it's a critical 
path. The GC will collect the allocations very efficiently these days. I didn't 
check what this code segment is used for, but unless it's looped somewhere, I 
doubt there will by any change.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1543793491

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

[Nir Lisker]

On Wed, 27 Mar 2024 23:18:43 GMT, Marius Hanl  wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
>>  line 773:
>> 
>>> 771: .collect(Collectors.toList());
>>> 772: 
>>> 773: sortedNewIndices.forEach(this::set);
>> 
>> Why do the double-iteration pattern here and not do the `peek` operation in 
>> a `forEach` like in the other 2 places?
>
> `forEach` is void, so we can not return a list afterwards.

You don't need to return a list, you create it ahead of time like was done in 
line 167

List indices = new ArrayList<>();

and the add the elements in `forEach`.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1542145635

Re: RFR: 8327179: Update the 3D lighting application [v5]

[Nir Lisker]

On Wed, 27 Mar 2024 20:38:58 GMT, Nir Lisker  wrote:

>> Update for the 3D lighting test tool as described in the JBS issue.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added gradle script

Looks like the line specified by jcheck was off.

-

PR Comment: https://git.openjdk.org/jfx/pull/1387#issuecomment-2024008625

Re: RFR: 8327179: Update the 3D lighting application [v6]

[Nir Lisker]

> Update for the 3D lighting test tool as described in the JBS issue.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Whitespace?

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1387/files
  - new: https://git.openjdk.org/jfx/pull/1387/files/f063d088..ae471f2e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1387=05
 - incr: https://webrevs.openjdk.org/?repo=jfx=1387=04-05

  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: RFR: 8327179: Update the 3D lighting application [v5]

[Nir Lisker]

On Wed, 27 Mar 2024 20:38:58 GMT, Nir Lisker  wrote:

>> Update for the 3D lighting test tool as described in the JBS issue.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added gradle script

I added the application to the gradle build file so it can be hooked up with 
its module dependencies. You can now produce a jar that bundles the resource 
with the classes. You still need to point to the modules at runtime.
Let me know if this is a good enough solution.

The way the project is configured in the main build file is not the best, but 
it will require large changes to do it better.

Not sure what jcheck wants. It's complaining about empty spaces that have a `}` 
appear after them. This is the error: 
https://github.com/openjdk/jfx/pull/1387/files#annotation_19798318258.

-

PR Comment: https://git.openjdk.org/jfx/pull/1387#issuecomment-2023946848
PR Comment: https://git.openjdk.org/jfx/pull/1387#issuecomment-2023948839

Re: RFR: 8327179: Update the 3D lighting application [v5]

[Nir Lisker]

> Update for the 3D lighting test tool as described in the JBS issue.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Added gradle script

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1387/files
  - new: https://git.openjdk.org/jfx/pull/1387/files/9b3f0c5e..f063d088

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1387=04
 - incr: https://webrevs.openjdk.org/?repo=jfx=1387=03-04

  Stats: 26 lines in 1 file changed: 26 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: RFR: 8328746: Remove unused imports in demo apps

[Nir Lisker]

On Thu, 21 Mar 2024 21:50:37 GMT, Andy Goryachev  wrote:

> Using Eclipse IDE to remove unused imports in **demo apps** (3D, Ensemble, 
> etc.) and update the copyright year to 2024. Using wildcard for more than 10 
> static imports.
> 
> 
> --
> 
> This is a trivial change (though fairly large), 1 reviewer is probably enough.

I find it helpful to separate the imports by their high-level domain name, 
"java.", "javafx.", "com." etc. It makes it easier to see the "span" or 
requirements of the class.

-

PR Comment: https://git.openjdk.org/jfx/pull/1420#issuecomment-2022856424

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

[Nir Lisker]

On Wed, 27 Mar 2024 09:11:56 GMT, drmarmac  wrote:

>> This PR removes potentially incorrect usages of Stream.peek().
>> The changed code should be covered by the tests that are already present.
>
> drmarmac has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove outdated comment

modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
 line 773:

> 771: .collect(Collectors.toList());
> 772: 
> 773: sortedNewIndices.forEach(this::set);

Why do the double-iteration pattern here and not do the `peek` operation in a 
`forEach` like in the other 2 places?

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1541157874

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

[Nir Lisker]

On Wed, 27 Mar 2024 12:23:47 GMT, Marius Hanl  wrote:

>> I'd say .forEach() is used correctly here, according to docs, it guarantees 
>> execution of the side-effects (add to removed list & clear index), just not 
>> in any particular order. This way we avoid multiple iteration.
>
> Another idea is to use `toList()`, which is a very efficient operation and 
> then iterate over it.

> In the java.util.stream package 
> [docs](https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/stream/package-summary.html#SideEffects)
>  it is mentioned that `forEach()` method operates only via side-effects. So 
> do you think we should avoid using `forEach()` here and iterate the generated 
> list separately to clear selected index?

`forEach` is used correctly here. From the docs:
> With the exception of terminal operations 
> [forEach](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#forEach(java.util.function.Consumer))
>  and 
> [forEachOrdered](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#forEachOrdered(java.util.function.Consumer)),
>  side-effects of behavioral parameters may not always be executed when the 
> stream implementation can optimize away the execution of behavioral 
> parameters without affecting the result of the computation.

> Another idea is to use `toList()`, which is a very efficient operation and 
> then iterate over it.

That's still 2 iterations. If the code is not performance-critical it doesn't 
matter.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1541140317

Re: RFR: 8327179: Update the 3D lighting application [v4]

[Nir Lisker]

On Wed, 13 Mar 2024 22:32:59 GMT, Nir Lisker  wrote:

>> Update for the 3D lighting test tool as described in the JBS issue.
>
> Nir Lisker has updated the pull request incrementally with five additional 
> commits since the last revision:
> 
>  - Added spacing
>  - Renamed constant
>  - Updated benchmark reset message
>  - Simplified models creation
>  - Revert resource package

I can't reproduce the exception. The path to the image is correct. Both the 
class and the image are under the `attenuation` package. Here is my launch 
command:


C:\Program Files\Java\jdk-21\bin\javaw.exe 
"-Djava.library.path=C:\Users\Nir\git\jfx\modules\javafx.graphics\build\module-lib"
--add-modules=javafx.controls,javafx.swing
-Dfile.encoding=UTF-8
-Dstdout.encoding=UTF-8
-Dstderr.encoding=UTF-8
-p 
"C:\Users\Nir\git\jfx\modules\javafx.controls\bin;C:\Users\Nir\git\jfx\modules\javafx.graphics\bin;C:\Users\Nir\git\jfx\modules\javafx.base\bin;C:\Users\Nir\git\jfx\modules\javafx.swing\bin"
-classpath "C:\Users\Nir\git\jfx\tests\performance\3DLighting\bin"
-XX:+ShowCodeDetailsInExceptionMessages
--add-exports javafx.graphics/test.com.sun.javafx.pgstub=javafx.controls
--add-exports javafx.base/test.com.sun.javafx.binding=javafx.controls
--add-exports javafx.base/test.util.memory=javafx.controls
--add-exports javafx.base/test.javafx.collections=javafx.controls
--add-exports javafx.base/com.sun.javafx.property=javafx.graphics
--add-exports javafx.base/test.javafx.collections=javafx.graphics
--add-exports javafx.base/test.util.memory=javafx.graphics
--add-exports java.base/sun.security.util=javafx.graphics
--add-reads javafx.base=java.management
--add-reads javafx.base=jdk.management attenuation.LightingApplication


Perhaps it's a platform-dependent path issue?

-

PR Comment: https://git.openjdk.org/jfx/pull/1387#issuecomment-2018183992

Re: RFR: 8328718: Remove unused imports in javafx.controls

[Nir Lisker]

On Thu, 21 Mar 2024 18:47:24 GMT, Andy Goryachev  wrote:

> Using Eclipse IDE to remove unused imports in javafx.controls and update the 
> copyright year to 2024.  Using wildcard for more than 10 static imports.
> 
> This is a trivial change, 1 reviewer is probably enough.

I can, but it will take me some time to get to it.

-

PR Comment: https://git.openjdk.org/jfx/pull/1416#issuecomment-2015727802

[ovirt-users] Re: virt-v2v paused by system after one hour or a bit more

[Nir Soffer]

On Thu, Mar 21, 2024 at 7:03 PM Cyril VINH-TUNG  wrote:

> Hello
>
> Here's the technique we use :
> - create manually the vm on ovirt with same disks (same size that original
> but you can choose target type, thin provision or preallocated)
> - on any node, force activating the disks to make them writable at the os
> level (lvm, vgchange...)
> - if the disk type is the same on target and destination, you can use dd
> over netcat to copy the disks
> - if the type is not the same, you might use qemu-img convert over netcat
>

This is very fragile and dangerous, you must really know what you are doing
:-)

If you already imported the disks from the other system, uploading them to
any storage domain
in any wanted (and supported) image format and allocation policy is much
easier and faster
using ovirt-img.

See https://www.ovirt.org/media/ovirt-img-v8.pdf


>
> If you have physical access to the node, you might use a flat backup
>
> Another workaround is to backup/restore the vm with a backup tool that
> works both with vmware and ovirt... I would say vprotect or vinchin
>

This should also work, but when importing vms from vmware, it is not enough
to copy the disk data a is,
you want to modify it to remove vmware bits and add the ovirt bits -
virt-v2v does all this for you.

Nir
___
Users mailing list -- users@ovirt.org
To unsubscribe send an email to users-le...@ovirt.org
Privacy Statement: https://www.ovirt.org/privacy-policy.html
oVirt Code of Conduct: 
https://www.ovirt.org/community/about/community-guidelines/
List Archives: 
https://lists.ovirt.org/archives/list/users@ovirt.org/message/6MASYRHUZPJCRX45G3TO73OURT5LFWCB/

[ovirt-users] Re: virt-v2v paused by system after one hour or a bit more

[Nir Soffer]

On Thu, Mar 21, 2024 at 12:44 PM Claus Serbe via Users 
wrote:

> Hi,
>
> I am migrating some vmware VM's from an NFS Storage via rhv-upload in
> virt-v2v, what is working good.
>
> But now I try to move some bigger VM's with several disks and sadly after
> a while (I would guess around an hour) the Ovirt-engine shows me "Paused by
> system" instead of transfering, so when the next disk should be imported,
> it will fail
>
> In the ovirt-engine.log I see the following lines for the remaining 4
> disks.
>
> 2024-03-21 06:14:06,815-04 ERROR
> [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector]
> (EE-ManagedScheduledExecutorService-engineScheduledThreadPool-Thread-35)
> [f61b3906-804d-470f-8524-6507081fbdec] EVENT_ID:
> UPLOAD_IMAGE_PAUSED_BY_SYSTEM_TIMEOUT(1,071), Upload was paused by system.
> Reason: timeout due to transfer inactivity.
> 2024-03-21 06:14:17,915-04 ERROR
> [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector]
> (EE-ManagedScheduledExecutorService-engineScheduledThreadPool-Thread-14)
> [aef8e312-d811-4a39-b5fb-342157209bce] EVENT_ID:
> UPLOAD_IMAGE_PAUSED_BY_SYSTEM_TIMEOUT(1,071), Upload was paused by system.
> Reason: timeout due to transfer inactivity.
> 2024-03-21 06:14:24,959-04 ERROR
> [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector]
> (EE-ManagedScheduledExecutorService-engineScheduledThreadPool-Thread-85)
> [860b012d-78a4-49f8-a875-52f4299c8298] EVENT_ID:
> UPLOAD_IMAGE_PAUSED_BY_SYSTEM_TIMEOUT(1,071), Upload was paused by system.
> Reason: timeout due to transfer inactivity.
> 2024-03-21 06:14:46,099-04 ERROR
> [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector]
> (EE-ManagedScheduledExecutorService-engineScheduledThreadPool-Thread-65)
> [f93869ee-2ecb-4f54-b3e9-b12259637b0b] EVENT_ID:
> UPLOAD_IMAGE_PAUSED_BY_SYSTEM_TIMEOUT(1,071), Upload was paused by system.
> Reason: timeout due to transfer inactivity.
>
>
> There are 2 strange things.
>
> 1. When I start virt-v2v it will create all 6 disks and set them to
> transferring, but virt-v2v will import one after the other, what leads to
> kind of unused/timing out transferring tickets.
>

This is a incorrect usage of ovirt-imageio API in virt-v2v, please report
it here:
https://github.com/libguestfs/virt-v2v/issues

The right way to use the API is:
1. Start transfer
2. Upload the data
3. End the transfer

It does not matter if you create all the disk at the start of the
operation, but starting a transfer must be done
right before you upload the data.


> 2. When I copy the disk images to a local disk before, it works. Maybe
> just because of faster transfer speeds.
>
> Is there a possibility to transfer parallel or maybe extend the timeout?
>

Sure you can upload in parallel, but I'm not sure virt-v2v will probably
have issues importing in parallel from other
systems (e.g. vmware would not allow this).

We tested uploading 10 100g images in parallel using the ovirt-img tool,
each upload using 4 connections
(total of 40 upload connections).

You may be able to extend the timeout, but this is not recommended since
your system will not clean up quickly
after a bad client disconnects uncleanly without ending the transfer.
Unfortunately I don't remember the which
timeout should be modified on the engine side, maybe Arik or Albert can
help with this.

Nir
___
Users mailing list -- users@ovirt.org
To unsubscribe send an email to users-le...@ovirt.org
Privacy Statement: https://www.ovirt.org/privacy-policy.html
oVirt Code of Conduct: 
https://www.ovirt.org/community/about/community-guidelines/
List Archives: 
https://lists.ovirt.org/archives/list/users@ovirt.org/message/2TUOTBUYA6GEZYJK3W4EC3PSDJLYXHK3/

[ovirt-users] Re: Create Vm without Storage Domain

[Nir Soffer]

On Wed, Mar 20, 2024 at 12:06 PM Shafi Mohammed 
wrote:

> Hi Guys ,
>
> Thanks for the Info .
>
> I am trying to migrate my data from local to Storage domain . But it
> requires twice the effort in terms of space and time to copy the data to
> the local file system and then upload the disk to it .
>
> I created a storage domain and tried to expose it to the NBD server to
> write the data from the source Vm disk . But I'm facing an nbd
> permission issue  . Even a copy or write is restricted .
>
> Actual Command
> qemu-nbd -f qcow2 /rhev/data-center/mnt/192.168.108.27:
> _storage/d62b04f8-973f-4168-9e69-1f334a4968b6/images/cd6b9fc4-ef8a-4f40-ac8d-f18d355223d0/d2a57e6f-029e-46cc-85f8-ce151d027dcb
>
>
>
> *qemu-nbd: Failed to blk_new_open
> '/rhev/data-center/mnt/192.168.108.27:_storage/d62b04f8-973f-4168-9e69-1f334a4968b6/images/cd6b9fc4-ef8a-4f40-ac8d-f18d355223d0/d2a57e6f-029e-46cc-85f8-ce151d027dcb':
> Could not open
> '/rhev/data-center/mnt/192.168.108.27:_storage/d62b04f8-973f-4168-9e69-1f334a4968b6/images/cd6b9fc4-ef8a-4f40-ac8d-f18d355223d0/d2a57e6f-029e-46cc-85f8-ce151d027dcb':
> Permission denied*
> Please suggest me if Ovirt has any API  to open a disk from a
> storage domain and write data to it
>

You are trying to reinvent ovirt-img. Try it:
https://www.ovirt.org/media/ovirt-img-v8.pdf
___
Users mailing list -- users@ovirt.org
To unsubscribe send an email to users-le...@ovirt.org
Privacy Statement: https://www.ovirt.org/privacy-policy.html
oVirt Code of Conduct: 
https://www.ovirt.org/community/about/community-guidelines/
List Archives: 
https://lists.ovirt.org/archives/list/users@ovirt.org/message/BK2RAMNASXIYOBKXSXG7JLN7ZPL2SI4R/

Re: RFR: 8325566: [TestBug] Util.shutdown() to hide ALL Windows [v2]

[Nir Lisker]

On Tue, 19 Mar 2024 22:28:54 GMT, Andy Goryachev  wrote:

>> tests/system/src/test/java/test/util/Util.java line 383:
>> 
>>> 381: runAndWait(() -> {
>>> 382: for (Window w : new ArrayList<>(Window.getWindows())) {
>>> 383: w.hide();
>> 
>> I think you can use `List.copyOf` instead of an `ArrayList` since it doesn't 
>> get modified (it's a one-time copy).
>> The iteration can also be
>> 
>> List.copyOf(Window.getWindows()).forEach(Window::hide);
>> 
>> if you want.
>
> The only issue with this style is - it's hard to set breakpoints.
> In this case it's not that important.
> Thank you!

It's a matter of style mostly, so I don't really mind it. The debug issue can 
be remedied by adding breakpoints inside Lambdas, which all IDEs support I 
think (Eclipse and IntelliJ for sure).

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1407#discussion_r1531207452

Re: RFR: 8325566: [TestBug] Util.shutdown() to hide ALL Windows

[Nir Lisker]

On Mon, 18 Mar 2024 22:53:03 GMT, Andy Goryachev  wrote:

> Changing `Util.shutdown()` to hide **all** showing Windows and then call 
> `Platform.exit()`.
> This simplifies the tests and ensures a clean shutdown.

tests/system/src/test/java/test/util/Util.java line 383:

> 381: runAndWait(() -> {
> 382: for (Window w : new ArrayList<>(Window.getWindows())) {
> 383: w.hide();

I think you can use `List.copyOf` instead of an `ArrayList` since it doesn't 
get modified (it's a one-time copy).
The iteration can also be

List.copyOf(Window.getWindows()).forEach(Window::hide);

if you want.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1407#discussion_r1531195323

Re: RFR: 8327179: Update the 3D lighting application

[Nir Lisker]

On Tue, 5 Mar 2024 16:44:00 GMT, Ambarish Rapte  wrote:

> Executing the LightingSample manually fails with an exception

I think the package names of the main and resources weren't aligned.

> The copyright year new files should be 2024 only. Some new files have more 
> than one copyright year. If you are moving any existing files then please use 
> git mv so that the files are shown as moved.

For now, for the ease of reviewing, I reverted the package name change.

> The effect of lighting on mesh vs a box is quite different.

I think this is the same issue you submitted in the past: 
https://bugs.openjdk.org/browse/JDK-8269133

-

PR Comment: https://git.openjdk.org/jfx/pull/1387#issuecomment-1996005413

Re: RFR: 8327179: Update the 3D lighting application [v4]

[Nir Lisker]

> Update for the 3D lighting test tool as described in the JBS issue.

Nir Lisker has updated the pull request incrementally with five additional 
commits since the last revision:

 - Added spacing
 - Renamed constant
 - Updated benchmark reset message
 - Simplified models creation
 - Revert resource package

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1387/files
  - new: https://git.openjdk.org/jfx/pull/1387/files/3f2994ff..9b3f0c5e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1387=03
 - incr: https://webrevs.openjdk.org/?repo=jfx=1387=02-03

  Stats: 48 lines in 6 files changed: 9 ins; 5 del; 34 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: RFR: 8327179: Update the 3D lighting application [v3]

[Nir Lisker]

On Tue, 5 Mar 2024 16:49:28 GMT, Ambarish Rapte  wrote:

>> Nir Lisker has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   copyright headers
>
> tests/performance/3DLighting/src/main/java/lighting3D/Benchmark.java line 49:
> 
>> 47: stopGraphic.setBoundsType(TextBoundsType.VISUAL);
>> 48: stopGraphic.setFill(Color.RED);
>> 49: stopGraphic.setFont(Font.font(20));
> 
> Play and Stop button could be same size. It would be good to look at. 
> Currently the Stop button seems lesser than half size of play button.

As you will see, the font sizes and visual sizes don't render as you would 
expect. Not sure why, but this way they look better. Perhaps small adjustments 
are in order, but if you match the sizes it comes out off.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1523953194

Re: RFR: 8327179: Update the 3D lighting application [v3]

[Nir Lisker]

> Update for the 3D lighting test tool as described in the JBS issue.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  copyright headers

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1387/files
  - new: https://git.openjdk.org/jfx/pull/1387/files/2646489d..3f2994ff

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1387=02
 - incr: https://webrevs.openjdk.org/?repo=jfx=1387=01-02

  Stats: 80 lines in 8 files changed: 75 ins; 0 del; 5 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: RFR: 8327179: Update the 3D lighting application [v2]

[Nir Lisker]

> Update for the 3D lighting test tool as described in the JBS issue.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Undo files move

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1387/files
  - new: https://git.openjdk.org/jfx/pull/1387/files/7f0e562f..2646489d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1387=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1387=00-01

  Stats: 12 lines in 8 files changed: 2 ins; 2 del; 8 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: [Acme] Happy Birthday ACME!

[Yoav Nir]

Hi, Rob

The first question whenever someone proposes a bis document is, of course, “are 
you volunteering to edit?”

Jokes aside, it’s always a question of whether or not it is worth the effort. 
Not just for whoever is editing, but the usual effort associated with any 
document, such as WG participants, shepherd, AD, IESG, various directorates, 
RFC editor.

So before embarking on something like this, it’s not enough to just count the 
number of errata. You need to put yourself in the shoes of a naive implementer 
who doesn’t know about errata. Are the errors big enough that they might lead 
them to make a mistake in the implementation?  Text that uses “example.com 
” instead of “example.org ” probably 
isn’t. Text that says that a challenge object with an error cannot have the 
“processing” status when it can likely is.

As for adding other RFCs, that’s again a judgement call. We did merge some 
add-ons to IKEv2 in one of its revisions. It make sense to merge them if the 
add-on is so obvious and so necessary, that pretty much every implementation of 
8555 would also implement the other document. Is RFC8738 like that? Or are IP 
identifiers so rare and curious that many implementations exclude them?

As always, it’s up to the group whether making a significantly bigger document 
with some of the add-ons makes sense. In general, groups and ADs tend to prefer 
smaller documents, but that is decided on a case-by-case basis.

Yoav

> On 11 Mar 2024, at 23:08, Rob Stradling  
> wrote:
> 
> RFC8555 was published [1] 5 years ago today!
> 
> Just thinking aloud, 'cos I'm curious what folks here think...
> At what point might it make sense to start work on an 8555-bis?
> 
> There's a fairly long list of Errata [2]: 10 Verified, 5 Reported, and 4 Held 
> for Document Update.
> 
> Would it make sense for an 8555-bis document to incorporate and obsolete any 
> of the "add-on" RFCs / I-Ds, such as RFC8738, that have been published since 
> RFC8555?  Or, conversely, would it be preferable to not do that?
> 
> With 5 years of deployment experience behind us, have any "missing" features 
> in RFC8555 been identified that would be best addressed by updating the core 
> specification (i.e., in an 8555-bis document) rather than by writing new 
> "add-on" I-Ds?  Or, conversely, are "add-on" I-Ds always the preferred 
> approach?  (The "missing" feature that immediately springs to my mind is 
> "profiles" [3]).
> 
> 
> [1] 
> https://mailarchive.ietf.org/arch/msg/rfc-dist/25pD6Za_dVkXMbJwyPhBJR6nIlo/
> [2] https://www.rfc-editor.org/errata_search.php?rfc=8555
> [3] https://mailarchive.ietf.org/arch/msg/acme/BLVAayrTrUCegT4s2twci3Q2BY8/
> 
> --
> Rob Stradling
> Senior Research & Development Scientist
> Sectigo Limited
> 
> ___
> Acme mailing list
> Acme@ietf.org 
> https://www.ietf.org/mailman/listinfo/acme

___
Acme mailing list
Acme@ietf.org
https://www.ietf.org/mailman/listinfo/acme

Re: RFR: JDK-8322964 Optimize performance of CSS selector matching [v6]

[Nir Lisker]

On Sat, 9 Mar 2024 06:48:25 GMT, John Hendrikx  wrote:

>> Improves performance of selector matching in the CSS subsystem. This is done 
>> by using custom set implementation which are highly optimized for the most 
>> common cases where the number of selectors is small (most commonly 1 or 2). 
>> It also should be more memory efficient for medium sized and large 
>> applications which have many style names defined in various CSS files.
>> 
>> Due to the optimization, the concept of `StyleClass`, which was only 
>> introduced to assign a fixed bit index for each unique style class name 
>> encountered, is no longer needed. This is because style classes are no 
>> longer stored in a `BitSet` which required a fixed index per encountered 
>> style class.
>> 
>> The performance improvements are the result of several factors:
>> - Less memory use when only very few style class names are used in selectors 
>> and styles from a large pool of potential styles (a `BitSet` for potentially 
>> 1000 different style names needed 1000 bits (worst case)  as it was not 
>> sparse).
>> - Specialized sets for small number of elements (0, 1, 2, 3-9 and 10+)
>> - Specialized sets are append only (reduces code paths) and can be made read 
>> only without requiring a wrapper
>> - Iterator creation is avoided when doing `containsAll` check thanks to the 
>> inverse function `isSuperSetOf`
>> - Avoids making a copy of the list of style class names to compare (to 
>> convert them to `StyleClass` and put them into a `Set`) -- this copy could 
>> not be cached and was always discarded immediately after...
>> 
>> The overall performance was tested using the JFXCentral application which 
>> displays about 800 nodes on its start page with about 1000 styles in various 
>> style sheets (and which allows to refresh this page easily).  
>> 
>> On JavaFX 20, the fastest refresh speed was 121 ms on my machine.  With the 
>> improvements in this PR, the fastest refresh had become 89 ms.  The speed 
>> improvement is the result of a 30% faster `Scene#doCSSPass`, which takes up 
>> the bulk of the time to refresh the JFXCentral main page (about 100 ms 
>> before vs 70 ms after the change).
>
> John Hendrikx has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Remove commented code in BitSetTest
>  - Remove unnecessary addAll overrides

modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 32:

> 30:  * @deprecated for removal
> 31:  */
> 32: public final class StyleClass {

Needs the `@Deprecated(forRemoval = true)` annotation on the class, and the doc 
tag needs an explanation of why it's being removed ("erroneously exposed", "no 
longer needed", "flawed from the start"...) and what should be used instead, if 
at all.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1518566872

Re: [Acme] ACME leadership changes

[Yoav Nir]

Hi, Tomofumi.

Welcome to the working group.

Looking forward to working with you.

Yoav

> On 7 Mar 2024, at 23:48, Roman Danyliw  wrote:
> 
> Hi WG!
> 
> As Deb (Cooley) will be the new, incoming SEC AD at the Brisbane meeting, 
> there is a vacancy left in the co-chair team of ACME.  It is my pleasure to 
> announced that Tomofumi Okubo will be stepping in as the new-chair.  Tomofumi 
> brings a wealth of experience in PKI and as a contributor in PKI-related IETF 
> WGs.
> 
> Tomofumi: Thank you for your willingness to serve.
> 
> Deb: Congratulations on your new AD role!  Thank you for your leadership in 
> the WG.
> 
> Yoav (Nir): Despite these transitions, thank you for your continued service 
> as co-chair in the WG!
> 
> Deb isn't going too far from ACME.  After the AD transition in Brisbane, the 
> responsible AD for ACME will transition from me to her.
> 
> Regards,
> Roman
> 
> 
> ___
> Acme mailing list
> Acme@ietf.org
> https://www.ietf.org/mailman/listinfo/acme

___
Acme mailing list
Acme@ietf.org
https://www.ietf.org/mailman/listinfo/acme

Re: RFR: 8092102: Labeled: truncated property

[Nir Lisker]

On Mon, 4 Mar 2024 21:04:28 GMT, Andy Goryachev  wrote:

> Adds Labeled.truncated property which indicates when the text is visually 
> truncated (and the ellipsis string is inserted) in order to fit the available 
> width.
> 
> The new property reacts to changes in the following properties:
> - ellipsisString
> - font
> - text
> - width
> - wrapText
> 
> For some reason, line 859 generates a javadoc "co comment" warning, despite 
> the javadoc comment present at the property declaration in line 832.
> 
> I don't think it's worth creating a headful test (headless won't work) due to 
> relative simplicity of the code.
> 
> **Alternative**
> 
> The desired functionality can be just as easily achieved on an application 
> level, by adding a similar property to a subclass.  What is the benefit of 
> adding this functionality to the core?

This enhancement should be discussed in the mailing list. I've never needed 
this myself, but it already had an old JBS ticket, so I can see that others did 
and for a legitimate reason (adding tooltips). I don't mind if it's added or 
not.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 
823:

> 821: /**
> 822:  * Truncated read-only property indicates whether the text has been 
> truncated
> 823:  * when it cannot fit into the available width.

No need to repeat the name and type in the first sentence as it's used in the 
summary table. Most phrasings opt (inconsistently) for: "Indicates whether..." 
or just "Whether..." (I prefer the former).

I'm also not sure about the sole role of the width. Can the text not be 
truncated for a reason other than not fitting in the width, like not fitting in 
the height?

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 
830:

> 828:  *
> 829:  * @since 23
> 830:  * @defaultValue false

I don't think that a default value is applicable here since it's completely 
dependent on other properties and can't be set. It's possible to call the `get` 
of this property for the first time and get `true`. The value here is the value 
used in the get method:

return property == null ? DEFAULT_VALUE : property.get();

at least in all the cases I've seen.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 
832:

> 830:  * @defaultValue false
> 831:  */
> 832: private ObservableBooleanValue truncated;

The property name should probably be `textTruncated` to be in line with 
`textOverrun`, `textFill`, `wrapText`...

Otherwise it could look like the labeled is truncated.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 
834:

> 832: private ObservableBooleanValue truncated;
> 833: 
> 834: public final ObservableBooleanValue truncatedProperty() {

`ObservableBooleanValue` is not a property, so it's odd to use it as the return 
type of one. Usually `ReadOnlyBooleanProperty` is used. This implementation 
through binding is also unique for read-only properties. A look in `Node` and 
`Labeled` show a different way of implementing read-only properties.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 
850:

> 848: protected boolean computeValue() {
> 849: if (isWrapText()) {
> 850: return false;

Are you sure that allowing text to wrap necessarily means it won't be 
truncated? What if the max height doesn't allow another line?

-

PR Review: https://git.openjdk.org/jfx/pull/1389#pullrequestreview-1917119989
PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1512963900
PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1512912954
PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1512968169
PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1512956759
PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1512898960

RFR: 8327179: Update the 3D lighting application

[Nir Lisker]

Update for the 3D lighting test tool as described in the JBS issue.

-

Commit messages:
 - Whitespaces
 - Initial commit

Changes: https://git.openjdk.org/jfx/pull/1387/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1387=00
  Issue: https://bugs.openjdk.org/browse/JDK-8327179
  Stats: 1733 lines in 15 files changed: 1021 ins; 708 del; 4 mod
  Patch: https://git.openjdk.org/jfx/pull/1387.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1387/head:pull/1387

PR: https://git.openjdk.org/jfx/pull/1387

Re: Validation Support

[Nir Lisker]

Hi Dirk,

JavaFX has some input validation support in terms of focus control and
rejecting invalid characters. Can you  explain what your proposal adds? Are
there JBS issues asking for this?

Thanks,
Nir

On Fri, Mar 1, 2024, 13:50 Dirk Lemmermann  wrote:

> Hi everyone,
>
> I updated the validation framework ValidatorFX today in our project to the
> latest release and I really like it a lot. It is a small compact API and
> works with any observable as opposed to the validation support provided by
> ControlsFX.
>
> Using it made me wonder whether it would make sense to bundle it or
> something like it directly with JavaFX. Developers often mention missing
> validation support as a drawback of using JavaFX. Adding this would take
> one item off from the list of arguments against using JavaFX.
>
> Many UI frameworks do have built-in validation support, e.g. Vaadin [0],
> Angular, [1], or QT [2]
>
> What do you think?
>
> —Dirk
>
> [0]
> https://vaadin.com/docs/latest/binding-data/components-binder-validation
> [1] https://angular.io/guide/form-validation
> [2] https://doc.qt.io/qt-6/qtquick-input-textinput.html
>
>

Re: RFR: 8314147: Updated the PhongMaterial documentation [v10]

[Nir Lisker]

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

Thanks everyone for the quick and detailed review!

-

PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1971049164

[jfx22] Integrated: 8314147: Updated the PhongMaterial documentation

[Nir Lisker]

On Thu, 29 Feb 2024 11:46:11 GMT, Nir Lisker  wrote:

> This pull request contains a backport of commit 
> [d9645730](https://github.com/openjdk/jfx/commit/d9645730f1e76e95e0bb93ceaeb5550390bf95c1)
>  from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

This pull request has now been integrated.

Changeset: 3b925780
Author:Nir Lisker 
URL:   
https://git.openjdk.org/jfx/commit/3b925780ab87c4987bd35a0d5dbcb66e8a2a5b31
Stats: 521 lines in 39 files changed: 461 ins; 13 del; 47 mod

8314147: Updated the PhongMaterial documentation

Reviewed-by: kcr
Backport-of: d9645730f1e76e95e0bb93ceaeb5550390bf95c1

-

PR: https://git.openjdk.org/jfx/pull/1385

[jfx22] RFR: 8314147: Updated the PhongMaterial documentation

[Nir Lisker]

This pull request contains a backport of commit 
[d9645730](https://github.com/openjdk/jfx/commit/d9645730f1e76e95e0bb93ceaeb5550390bf95c1)
 from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

-

Commit messages:
 - Backport d9645730f1e76e95e0bb93ceaeb5550390bf95c1

Changes: https://git.openjdk.org/jfx/pull/1385/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1385=00
  Issue: https://bugs.openjdk.org/browse/JDK-8314147
  Stats: 521 lines in 39 files changed: 461 ins; 13 del; 47 mod
  Patch: https://git.openjdk.org/jfx/pull/1385.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1385/head:pull/1385

PR: https://git.openjdk.org/jfx/pull/1385

Integrated: 8314147: Updated the PhongMaterial documentation

[Nir Lisker]

On Thu, 22 Feb 2024 20:38:00 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.

This pull request has now been integrated.

Changeset: d9645730
Author:Nir Lisker 
URL:   
https://git.openjdk.org/jfx/commit/d9645730f1e76e95e0bb93ceaeb5550390bf95c1
Stats: 521 lines in 39 files changed: 461 ins; 13 del; 47 mod

8314147: Updated the PhongMaterial documentation

Reviewed-by: arapte, kcr

-

PR: https://git.openjdk.org/jfx/pull/1378

Re: RFR: 8314147: Updated the PhongMaterial documentation [v10]

[Nir Lisker]

> 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]

[Nir Lisker]

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]

[Nir Lisker]

> 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]

[Nir Lisker]

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]

[Nir Lisker]

> 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]

[Nir Lisker]

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]

[Nir Lisker]

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 [v6]

[Nir Lisker]

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]

[Nir Lisker]

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]

[Nir Lisker]

> 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: 8314147: Updated the PhongMaterial documentation [v6]

[Nir Lisker]

> 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 typo

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/83bf2eb1..60d45bc8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1378=05
 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=04-05

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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

[jfx22] Integrated: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

On Sat, 24 Feb 2024 09:53:01 GMT, Nir Lisker  wrote:

> Backport of commit 
> [b43c4edf](https://github.com/openjdk/jfx/commit/b43c4edf7590429fd051d1b0e2ccb6dd49a10b8b)
>  from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

This pull request has now been integrated.

Changeset: 6e1246e8
Author:Nir Lisker 
URL:   
https://git.openjdk.org/jfx/commit/6e1246e870aa6dafe05a3f544bb283aa90a0c078
Stats: 66 lines in 3 files changed: 19 ins; 25 del; 22 mod

8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) 
javadoc
8318624: API docs specify incorrect default value for nodeOrientation property

Reviewed-by: kcr
Backport-of: b43c4edf7590429fd051d1b0e2ccb6dd49a10b8b

-

PR: https://git.openjdk.org/jfx/pull/1380

Re: RFR: 8314147: Updated the PhongMaterial documentation [v5]

[Nir Lisker]

> 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:

  Remove redundant image copies

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/e367c882..83bf2eb1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1378=04
 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=03-04

  Stats: 65 lines in 3 files changed: 0 ins; 65 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 [v4]

[Nir Lisker]

> 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:

  Clarifications for the diffuse component

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/0130d0f5..e367c882

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1378=03
 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=02-03

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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

[jfx22] RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

Backport of commit 
[b43c4edf](https://github.com/openjdk/jfx/commit/b43c4edf7590429fd051d1b0e2ccb6dd49a10b8b)
 from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

-

Commit messages:
 - Backport b43c4edf7590429fd051d1b0e2ccb6dd49a10b8b

Changes: https://git.openjdk.org/jfx/pull/1380/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1380=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325550
  Stats: 66 lines in 3 files changed: 19 ins; 25 del; 22 mod
  Patch: https://git.openjdk.org/jfx/pull/1380.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1380/head:pull/1380

PR: https://git.openjdk.org/jfx/pull/1380

Integrated: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

On Fri, 23 Feb 2024 17:46:20 GMT, Nir Lisker  wrote:

> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs in 
> `Node` and `Scene`.
> 
> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
> system property, while for `Node` it isn't (which means `SubScene` will be 
> different from `Scene`). Not sure if this is intended.

This pull request has now been integrated.

Changeset: b43c4edf
Author:Nir Lisker 
URL:   
https://git.openjdk.org/jfx/commit/b43c4edf7590429fd051d1b0e2ccb6dd49a10b8b
Stats: 66 lines in 3 files changed: 19 ins; 25 del; 22 mod

8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) 
javadoc
8318624: API docs specify incorrect default value for nodeOrientation property

Reviewed-by: angorya, kcr

-

PR: https://git.openjdk.org/jfx/pull/1379

Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

On Fri, 23 Feb 2024 17:46:20 GMT, Nir Lisker  wrote:

> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs in 
> `Node` and `Scene`.
> 
> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
> system property, while for `Node` it isn't (which means `SubScene` will be 
> different from `Scene`). Not sure if this is intended.

By the way, is the difference in the default values between `Node` and `Scene` 
intentional? Only `Scene` has the RTL system property checked.

-

PR Comment: https://git.openjdk.org/jfx/pull/1379#issuecomment-1962085277

Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

[Nir Lisker]

> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs in 
> `Node` and `Scene`.
> 
> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
> system property, while for `Node` it isn't (which means `SubScene` will be 
> different from `Scene`). Not sure if this is intended.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressed review comments

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1379/files
  - new: https://git.openjdk.org/jfx/pull/1379/files/dac77a49..a3c375df

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1379=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1379=00-01

  Stats: 53 lines in 2 files changed: 21 ins; 25 del; 7 mod
  Patch: https://git.openjdk.org/jfx/pull/1379.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1379/head:pull/1379

PR: https://git.openjdk.org/jfx/pull/1379

Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

On Fri, 23 Feb 2024 19:38:20 GMT, Kevin Rushforth  wrote:

>> modules/javafx.graphics/src/main/java/javafx/scene/layout/AnchorPane.java 
>> line 187:
>> 
>>> 185:  * Returns the child's bottom anchor constraint, if set.
>>> 186:  * @param child the child node of an anchor pane
>>> 187:  * @return the offset from the bottom of the anchor pane, or 
>>> {@code null} if no bottom anchor was set
>> 
>> minor suggestion: move explanation of when it returns null to the 
>> description,  i.e.:
>> 
>> 
>> Returns the child's bottom anchor constraint, if set, otherwise returns 
>> {@code null}.
>> 
>> @return the offset from the bottom of the anchor pane, or {@code null}
>
> I don't mind either way.

`@return` statements usually state this condition explicitly, like `Map#get`: 
"the value to which the specified key is mapped, or `null` if this map contains 
no mapping for the key".

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1379#discussion_r1501214852

Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

On Fri, 23 Feb 2024 19:44:06 GMT, Andy Goryachev  wrote:

>> modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 6272:
>> 
>>> 6270:  * of text in both worlds.
>>> 6271:  *
>>> 6272:  * @defaultValue if the system property {@code 
>>> javafx.scene.nodeOrientation.RTL} is {@code true},
>> 
>> Restore the `@return` to avoid a warning.
>
> it seems that placing javadoc at the property declaration instead of the 
> method which returns said property eliminates the warning. 
> See for example Stage:1329

Yes, this is why I was surprised that new warnings showed up. The `@return` 
text is auto-generated for properties, so why would a return tag be needed? I 
guess the doc tool doesn't understand that when the doc is placed on the 
property getter instead of on the field, which I think is a bug.

I suggest to move the doc to the field.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1379#discussion_r1501126942

Re: Proposal: RichTextArea Control (Incubator)

[Nir Lisker]

Hi all,

I don't use RT controls, so I can't comment about the specifics of this or
other implementations. However, people raised some good points that I would
like to be more specific about.

1. Can the "components" of the RTA be separated out so that other controls
can use them? For example, colored text segments and highlights can be
useful in any text control, like Label and TextField.
2. About creating new controls and the difficulty involved, aren't the long
discussions about splitting the behavior, skin, input etc. and John's
proposal in that regard addressing exactly that, giving the ability to
compose "facets" into controls?
3. It's obvious that the bar for what goes into a library (JavaFX, JDK and
all other GUI toolkits) is fluid. We don't *have* to provide a Button
because any 3rd party library can make one, but we all agree that it's a
good idea. More complex controls, like color picker and date picker could
be more controversial as they are not entirely standardized, but I think
it's OK to have them. The question is, how far does this go? Perhaps what
can help here is to look at various GUI toolkits and see what they provide
so we know what users expect. It also matters if they are intended for use
on mobile, in which case the set of controls is somewhat different.

- Nir

On Wed, Feb 21, 2024 at 8:07 PM Andy Goryachev 
wrote:

> Dear JavaFX developers:
>
>
>
> We would like to propose a new feature - rich text control, RichTextArea,
> intended to bridge the functional gap with Swing and its
> StyledEditorKit/JEditorPane.  The main design goal is to provide a control
> that is complete enough to be useful out-of-the box, as well as open to
> extension by the application developers.
>
>
>
> This is a complex feature with a large API surface that would be nearly
> impossible to get right the first time, even after an extensive review.  We
> are, therefore, introducing this in an incubating module,
> *javafx.incubator.richtext*.   This will allow us to evolve the API in
> future releases without the strict compatibility constraints that other
> JavaFX modules have.
>
>
>
> Please take a look at the proposal [0], a list of discussion points [1],
> and the API Specification (javadoc) [2]. While the proposed API is ready
> for review, it isn't complete nor set in stone. We are looking for
> feedback, and will update the proposal based on the suggestions we receive
> from the community.  We encourage you to comment either in the mailing
> list, or by leaving comments inline in a draft pull request [3].  For
> context, the links to the original RFE [4] and a list of missing APIs
> related to rich text [5] are provided below.
>
>
>
> Sincerely,
>
> Your friendly JavaFX development team.
>
>
>
>
>
> References
>
>
>
>
>
> [0] Proposal:
> https://github.com/andy-goryachev-oracle/Test/blob/rich.jep.review/doc/RichTextArea/RichTextArea.md
>
> [1] Discussion points:
> https://github.com/andy-goryachev-oracle/Test/blob/rich.jep.review/doc/RichTextArea/RichTextAreaDiscussion.md
>
> [2] API specification (javadoc):
> https://cr.openjdk.org/~angorya/RichTextArea/javadoc
>
> [3] Draft Pull Request for API comments and feedback:
> https://github.com/openjdk/jfx/pull/1374
>
> [4] RichTextArea RFE: https://bugs.openjdk.org/browse/JDK-8301121
>
> [5] Missing APIs related to rich text control:
> https://bugs.openjdk.org/browse/JDK-8300569
>
>
>

RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc

[Nir Lisker]

Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs in 
`Node` and `Scene`.

Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
system property, while for `Node` it isn't (which means `SubScene` will be 
different from `Scene`). Not sure if this is intended.

-

Commit messages:
 - Initial commit

Changes: https://git.openjdk.org/jfx/pull/1379/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1379=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325550
  Stats: 41 lines in 3 files changed: 4 ins; 6 del; 31 mod
  Patch: https://git.openjdk.org/jfx/pull/1379.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1379/head:pull/1379

PR: https://git.openjdk.org/jfx/pull/1379

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 23:07:21 GMT, Kevin Rushforth  wrote:

>> Perhaps the contribution guidelines should mention what resources can be 
>> included. In all other open source projects I worked on it was enough that 
>> the resource wasn't licensed (or had something like 
>> https://en.wikipedia.org/wiki/WTFPL).
>
> The only thing I can point at is the OCA (which is linked in the CONTRIBUTOR 
> guidelines), which states that contributions need to be an original work.
> 
> So to double-check: Aside from that image (and the ones derived from it), are 
> all the others your original work?

Yes, I created the textures in a generator, the schematics I drew myself, and 
the screenshots are from the 3D Lighting app.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500052213

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 22:50:06 GMT, Andy Goryachev  wrote:

>>> also, the first `+` is unnecessary
>> 
>> I think it was done for uniformity with the other properties. I would use a 
>> `StringJoiner` these days for such a task anyway.
>
> StringJoiner seems to provide little benefit for maps or key=value pairs.  Do 
> you know of an alternative?

A `StringJoiner` seems suitable to me in this case:

new StringJoiner(",", "PhongMaterial[", "]")
.add("diffuseColor=" + getDiffuseColor())
.add("specularPower=" + getSpecularPower())
...

I guess you can make a helper method that takes a property and creates the 
string for you, like `diffuseColor.getName() + "=" + diffuseColor.getValue()` 
and then it won't look like a key-value pair in the joiner.

You might want to search for the default implementation of records and see how 
they create the `name = value` representation.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500046959

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 22:42:18 GMT, Andy Goryachev  wrote:

>> OK, it will be after the weekend when I can come back to this. Reviewers can 
>> assume that only the background will change.
>
> sorry, Nir.  ;-)

Perhaps the contribution guidelines should mention what resources can be 
included. In all other open source projects I worked on it was enough that the 
resource wasn't licensed (or had something like 
https://en.wikipedia.org/wiki/WTFPL).

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500034313

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 22:20:56 GMT, Nir Lisker  wrote:

>> modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java 
>> line 848:
>> 
>>> 846: + ", specularPower=" + getSpecularPower() + ", 
>>> diffuseMap=" + getDiffuseMap() + ", specularMap="
>>> 847: + getSpecularMap() + ", bumpMap=" + getBumpMap() + ", 
>>> selfIlluminationMap=" + getSelfIlluminationMap()
>>> 848: + "]";
>> 
>> could we restore the formatting please?
>> also, the first `+` is unnecessary
>
> Weird, I specifically reverted the auto-formatting, not sure how it got in. 
> Will revert.

> also, the first `+` is unnecessary

I think it was done for uniformity with the other properties. I would use a 
`StringJoiner` these days for such a task anyway.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500028809

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 22:36:06 GMT, Kevin Rushforth  wrote:

>> Do I only need to switch the ones in the introduction that show the 
>> multiplication of the color and the map, or all of the new images that use 
>> it as a background? The latter will be a lot of work.
>
> All of them, since they were derived from the original. I can still tell what 
> the source is.

OK, it will be after the weekend when I can come back to this. Reviewers can 
assume that only the background will change.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500026555

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

On Thu, 22 Feb 2024 22:21:49 GMT, Kevin Rushforth  wrote:

>> What would an approval require?
>
> To add to this, such approval would be unlikely, and I am not in favor of 
> making the request. Perhaps you can find suitable images already in the repo? 
> Here is one that might work:
> 
> https://github.com/openjdk/jfx/tree/e0b88bc7cfede46afe28cbb4a2e333df933b5100/apps/toys/FX8-3DFeatures/src/resources

Do I only need to switch the ones in the introduction that show the 
multiplication of the color and the map, or all of the new images that use it 
as a background? The latter will be a lot of work.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1500019932

Re: RFR: 8314147: Updated the PhongMaterial documentation [v3]

[Nir Lisker]

> 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:

  Revert unintended formatting

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/34bbc561..0130d0f5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1378=02
 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=01-02

  Stats: 53 lines in 1 file changed: 4 ins; 0 del; 49 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 [v2]

[Nir Lisker]

On Thu, 22 Feb 2024 22:18:38 GMT, Andy Goryachev  wrote:

>> Nir Lisker has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year
>
> modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java 
> line 848:
> 
>> 846: + ", specularPower=" + getSpecularPower() + ", 
>> diffuseMap=" + getDiffuseMap() + ", specularMap="
>> 847: + getSpecularMap() + ", bumpMap=" + getBumpMap() + ", 
>> selfIlluminationMap=" + getSelfIlluminationMap()
>> 848: + "]";
> 
> could we restore the formatting please?
> also, the first `+` is unnecessary

Weird, I specifically reverted the auto-formatting, not sure how it got in. 
Will revert.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r157027

Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]

[Nir Lisker]

On Thu, 22 Feb 2024 22:13:33 GMT, Kevin Rushforth  wrote:

>> No, it's a standard in image processing: https://en.wikipedia.org/wiki/Lenna.
>
> But it's still third-party content and cannot go in without approval.

What would an approval require?

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r152472

Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]

[Nir Lisker]

On Thu, 22 Feb 2024 20:49:20 GMT, Andy Goryachev  wrote:

>> Nir Lisker has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year
>
> modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/color and 
> map/map.png line 1:
> 
>> (failed to retrieve contents of file, check the PR for context)
> Is this a licensed image?

No, it's a standard in image processing: https://en.wikipedia.org/wiki/Lenna.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r144625

Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]

[Nir Lisker]

> 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 copyright year

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/b744540c..34bbc561

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1378=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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

[Nir Lisker]

On Thu, 22 Feb 2024 20:38:00 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.

@kevinrushforth Please review this. Perhaps the second reviewer can be 
@jayathirthrao or @arapte.

-

PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1960282365

RFR: 8314147: Updated the PhongMaterial documentation

[Nir Lisker]

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.

-

Commit messages:
 - Whitespace
 - Initial commit

Changes: https://git.openjdk.org/jfx/pull/1378/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1378=00
  Issue: https://bugs.openjdk.org/browse/JDK-8314147
  Stats: 637 lines in 41 files changed: 525 ins; 17 del; 95 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: Proposal: Bump minimum JDK version for JavaFX 23 to JDK 21

[Nir Lisker]

+1

On Sat, Feb 17, 2024, 15:37 John Hendrikx  wrote:

> +1
>
> On 16/02/2024 23:11, Kevin Rushforth wrote:
> > All,
> >
> > Even though we build JavaFX binaries with JDK 21 as the boot JDK, the
> > latest version of JavaFX still runs with JDK 17, although it isn't
> > tested with older JDK versions. In order for JavaFX to be able to use
> > newer JDK features, such as code snippets (in API docs), record
> > patterns, pattern matching for switch statements, and so forth, we
> > need to increase the minimum version of the JDK that can run the
> > latest JavaFX. Additionally, there is an ongoing cost to keeping
> > JavaFX buildable and runnable on older versions of Java, and very
> > little reason to continue to do so.
> >
> > A question was raised [1] as to whether we should go even further and,
> > once JDK 22 is released, jump straight to JDK 22 as a minimum. While
> > we could do that, I feel that there isn't sufficient justification for
> > this at this time, although we could reconsider for next release.
> >
> > To this end, I propose to bump the minimum version of the JDK needed
> > to run JavaFX 23 to JDK 21. I filed JDK-8321603 [2] to track this and
> > prepared PR  #1370 [3] (I've moved the PR back to Draft, pending this
> > discussion). This will not affect update releases of earlier versions
> > of JavaFX (e.g., JavaFX 21.0.NN or JavaFX 17.0.NN), which will
> > continue to run with the same minimum JDK that they run on today.
> >
> > As a reminder, we only assure that JavaFX NN will run with JDK NN-1 or
> > later, although in practice, we don't bump the minimum required JDK
> > version without a good reason. For example, while JavaFX 22 is built
> > using JDK 21 as the boot JDK, it produces class files that will run
> > with JDK 17, using "--release 17". The proposed change discussed here
> > would update that in JavaFX 23 to "--release 21".
> >
> > NOTE: this will not be an invitation to do wholesale refactoring of
> > existing classes or methods to use newer language features (e.g., a PR
> > that refactors existing switch statements and switch expressions into
> > pattern-matching switch expressions would not be welcome). Rather,
> > this can be seen as enabling judicious use of new features in new
> > code, much as we did when we started allowing the use of "var",
> > records, and pattern-matching instanceof.
> >
> > Comments are welcome.
> >
> > -- Kevin
> >
> > [1]
> > https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044081.html
> > [2] https://bugs.openjdk.org/browse/JDK-8321603
> > [3] https://github.com/openjdk/jfx/pull/1370
> >
>

Linking to anchors in the same class

[Nir Lisker]

Hi,

I have a question after reading https://bugs.openjdk.org/browse/JDK-8294195.
If I have a class Chronus.java with a heading in its class javadoc:

Time

then an anchor Chronus.html#time-heading is auto-generated. How do I link
to this anchor from within the class using @see and @link? I have tried
@see ##time-heading
@see Chronus.html##time-heading
@see Chronus##Chronus.html#time-heading

None of which work. What is the correct syntax?

Thanks,
Nir

Re: Layout and property bindings question, what is allowed?

[Nir Lisker]

You're thinking about the docs of the note in the bounds properties I think:

https://openjfx.io/javadoc/21/javafx.graphics/javafx/scene/Node.html#boundsInLocalProperty()
https://openjfx.io/javadoc/21/javafx.graphics/javafx/scene/Node.html#boundsInParentProperty()

On Fri, Feb 9, 2024, 03:28 John Hendrikx  wrote:

> Hi,
>
> I'm pretty sure I read somewhere in JavaFX docs, code or website that
> binding certain properties which are changed during layout is a really
> bad idea, but I can't find where I've seen this.  Anyone know what I mean?
>
> It was something like: binding the values of a property like
> width/height which are changed during layout to another property (which
> may in turn affect layout) should not be done, and one should be using
> properties like min/pref/max width/height.
>
> The reason I ask is because I'm running into a weird problem where I'm
> binding the value of ScrollPane.viewportBoundsProperty.width to a
> Label's text property.  What I noticed is that the Label simply is not
> updating correctly, and will display the correct value **BUT** use the
> previous value for its width calculation which causes the text to not
> fit -- the fact that it is displayed wrong is especially obvious when
> the Label's value switches between values like "312.0" and
> "313.3" where the 2nd value often gets an ellipsis (like
> "31...") because its width was computed based on "312.0".  What I see
> happening is this:
>
> - A splitpane is resized (with a scrollpane in it)
> - The ScrollPane's viewportBoundsProperty changes (in layoutChildren
> code of ScrollPaneSkin), so layout is already running...
> - A binding on viewportBoundsProperty.width updates the label text from
> "312.0" to "313."
>
> However, before that binding is executed, I see a call to
> `computePrefWidth` on the Label, which uses the old text (ie. "312.0")
> -- there is NO other call to computePrefWidth, so the layout process
> continues with an incorrect width value...
>
> When later the layout code of the Label is called, it concludes that the
> current width (based on the text "312.0") is insufficient to display
> "313.", and so adds an ellipsis...
>
> So, I'm thinking this may be "expected" behavior -- I'm binding on a
> property that is updated during layout, and then making another change
> (to Label.text) which in turn should trigger a new layout.  However,
> that doesn't happen, and it just runs in the current layout, where it
> then uses part new, part old values (new text, but old width).
>
> Any insights are appreciated.
>
> --John
>
>
>

Re: RFR: JDK-8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v11]

[Nir Lisker]

On Fri, 9 Jun 2023 12:00:06 GMT, John Hendrikx  wrote:

>> This provides and uses a new implementation of `ExpressionHelper`, called 
>> `ListenerManager` with improved semantics.
>> 
>> # Behavior
>> 
>> |Listener...|ExpressionHelper|ListenerManager|
>> |---|---|---|
>> |Invocation Order|In order they were registered, invalidation listeners 
>> always before change listeners|(unchanged)|
>> |Removal during Notification|All listeners present when notification started 
>> are notified, but excluded for any nested changes|Listeners are removed 
>> immediately regardless of nesting|
>> |Addition during Notification|Only listeners present when notification 
>> started are notified, but included for any nested changes|New listeners are 
>> never called during the current notification regardless of nesting|
>> 
>> ## Nested notifications:
>> 
>> | |ExpressionHelper|ListenerManager|
>> |---|---|---|
>> |Type|Depth first (call stack increases for each nested level)|(same)|
>> |# of Calls|Listeners * Depth (using incorrect old values)|Collapses nested 
>> changes, skipping non-changes|
>> |Vetoing Possible?|No|Yes|
>> |Old Value correctness|Only for listeners called before listeners making 
>> nested changes|Always|
>> 
>> # Performance
>> 
>> |Listener|ExpressionHelper|ListenerManager|
>> |---|---|---|
>> |Addition|Array based, append in empty slot, resize as needed|(same)|
>> |Removal|Array based, shift array, resize as needed|(same)|
>> |Addition during notification|Array is copied, removing collected 
>> WeakListeneres in the process|Tracked (append at end)|
>> |Removal during notification|As above|Entry is `null`ed|
>> |Notification completion with changes|-|Null entries (and collected 
>> WeakListeners) are removed|
>> |Notifying Invalidation Listeners|1 ns each|(same)|
>> |Notifying Change Listeners|1 ns each (*)|2-3 ns each|
>> 
>> (*) a simple for loop is close to optimal, but unfortunately does not 
>> provide correct old values
>> 
>> # Memory Use 
>> 
>> Does not include alignment, and assumes a 32-bit VM or one that is using 
>> compressed oops.
>> 
>> |Listener|ExpressionHelper|ListenerManager|OldValueCaching ListenerManager|
>> |---|---|---|---|
>> |No Listeners|none|none|none|
>> |Single InvalidationListener|16 bytes overhead|none|none|
>> |Single ChangeListener|20 bytes overhead|none|16 bytes overhead|
>> |Multiple listeners|57 + 4 per listener (excluding unused slots)|57 + 4 per 
>> listener (excluding unused slots)|61 + 4 per listener (excluding unused 
>> slots)|
>> 
>> # About nested changes
>> 
>> Nested changes are simply changes that are made to a property that is 
>> currently in the process of notif...
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix generic warnings

It's the first item on my review list for FX 23. Hope to get the time for the 
review queue in 1-2 weeks.

-

PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-1934971092

Re: Binding properties to constant values

[Nir Lisker]

Hi John,

If the issues with StyleOrigin are not as problematic as I presented, it
comes out to the behavioral flexibility we have. Can we change the
precedence order at this point? Do users rely on it or are surprised by it
(I see the latter, but those who think it works correctly are not going to
say that)?
Perhaps Kevin can weigh in.

The question of how to do it is simpler, and can be done with or without
breaking changes to StyleOrigin.

On Thu, Feb 1, 2024 at 2:27 AM John Hendrikx 
wrote:

> Hi Nir,
>
> On 31/01/2024 22:36, Nir Lisker wrote:
> >
> > I would wait with the ramifications of setting competing values from
> > different origins until the question of precedence is answered.
> > Perhaps emitting warnings is better, though I can see some scenarios
> > in which they will be annoying.
> >
> > The way I see the order:
> > 1. Setting from code should always take precedence (including the
> > current bindings over setter order of course).
> > 2. INLINE origin (via setStyle).
> > 3. Stylesheets according to their StyleOrigin as specified by the
> > non-javafx css specifications [3]: AUTHOR > USER > USER_AGENT (see
> > below for INLINE).
> >
> > 2 and 3 are already (more or less) specified in JavaFX's css as far as
> > I can see. However, 1 is not css, hence I don't think StyleOrigin
> > should be applicable here even. Even more, 2 isn't really css either,
> > it mimics html tags and shouldn't count as a css StyleOrigin in my
> > opinion.
> StyleOrigin is more of an internal thing that got exposed, so I wouldn't
> worry too much about what is CSS and what isn't.  I agree that the above
> order would make more sense. I suspect the reasoning why it wasn't done
> is that setters are getting called to set up defaults, which if they by
> default override all, would conflict with style sheets set by the user.
> > Note also that a Stylesheet can set its origin [4], even to INLINE, so
> > it takes precedence over java property setters and conflicts with
> > 'setStyle' "real" INLINE. I'm not sure if this is a bug because the
> > javafx css specs say that "Inline styles are specified via the Node
> > setStyle AP".
>
> The CSS API is weird. It has many seemingly useful and public bits, but
> they are isolated from the rest of FX.
>
> Take StyleSheet for example.  You can convert them (using File to File)
> from regular to binary, then load a binary file and get a StyleSheet
> reference with a public API.  You can then change its origin.  However,
> it stops there.  You can't feed such a modified StyleSheet to FX via any
> public API.
>
> It almost looks like a lot of the CSS public API is there for a possibly
> previously integrated Scene Builder type application, which was later
> split off.
>
> >
> > So, if I were to able to do anything I wanted, I would have restricted
> > Stylesheets to the options in 3, remove INLINE from a public
> > perspective and apply it only behind the scenes to 'setStyle' calls,
> > and stop treating java settings in the css hierarchy (which means
> > removing the USER StyleOrigin from them). Obviously that breaks a lot
> > of code, but this behavior would be my general goal. As for how to
> > represent it, maybe a constant can be added to StyleOrigin to
> > represent java code settings, but that's not a real css origin. I
> > guess we could call INLINE and the hypothetical JAVA constants
> > "pseudo-origins", because they don't apply to stylesheets, and are
> > only used internally. Or just don't check StyleOrigin when the value
> > is set from java. There are probably more ways.
>
> StyleOrigin really is only there for the CSS engine to distinguish if it
> can replace a value or not with something else depending on precedence.
> It probably shouldn't have been public at all, aside from the fact that
> StyleableProperty being an interface forced it to be public.
>
> Oddly enough, the StyleableProperty interface is public, but it is again
> never returned anywhere (didn't check extensively). So you can't
> actually do "label.textAlignment().applyStyle( ... )" even though text
> alignment is a styleable property.
>
> If `textAlignment()` did return a StyleableProperty, then you could do
> `label.textAlignment().applyStyle(StyleOrigin.CODE, value)` for a
> permanent override of the value which the CSS engine will respect.
>
> >
> > I also wonder if StyleOrigin should implement Comparable for
> > the precedence calculations of stylesheets.
>
> It sort of already does, they are defined in the order of least to
> highest precedence, so an integer comparator on its ordinal can be used.
>
> --John
>
>

Re: Binding properties to constant values

[Nir Lisker]

>
> Specialized methods will be added to `BooleanProperty`,
> `DoubleProperty`, `FloatProperty`, `IntegerProperty`, and
> `LongProperty`, each with one of the preexisting constant wrappers
> that are already in the framework.
> Some wrappers can be deduplicated (we only ever need two wrapper
> instances for boolean values).


You mean the ones in com.sun.javafx.binding [1]?

I use both constant bindings and inline css to override stylesheets and I
like neither, but I'm not sure that exposing constant binding is the
solution. Like John said, it's specific for css and doesn't make much sense
semantically.

About the order of css origins, there was a relevant discussion on JBS [2].

1) Does it make sense to treat values set in code (which IMHO should
> always be the last word on anything) as less important than values from
> AUTHOR and INLINE styles?  This is specified in the CSS document, but
> not fully enforced.


I think that the specifications chose the wrong order. They say "The
implementation allows designers to style an application by using style
sheets to override property values set from code." and I think this was a
poor and mostly surprising choice. I see that John and Scott agree with
that too. I've seen many StackOverflow questions about this confusion, many
can be found by searching "javafx override css from code". I don't know if
it can be changed, as it's a breaking change. I wonder what the original
writer's thoughts were, and what use case they had in mind. Perhaps they
thought that setting values from the java side is akin to specifying a user
stylesheet to the browser, which takes precedence over the browser's style
sheet, but not over the author's stylesheet.

2) Should bound values (which IMHO fall under "values set from code") be
> able to override AUTHOR and INLINE styles?  Why are they being treated
> differently at all?  Which StyleOrigin level do they fall under?
> "USER"?  A 5th level with higher priority than "INLINE"?
>

I don't see how anything should override bound values since they update on
every change and throw an exception if something tries to set the value of
a bound value.

3) Should properties which hold an AUTHOR or INLINE value reject calls
> to `setValue` (to make it clear they are temporary and that your code is
> probably wrong)?  This currently is not in line with the CSS document.
> Note that this will be slightly annoying for the CSS engine as it may
> not be able to "reset" such values as easily as before (which is
> probably the reason it currently works the way it does, but that's no
> excuse IMHO).


I would wait with the ramifications of setting competing values from
different origins until the question of precedence is answered. Perhaps
emitting warnings is better, though I can see some scenarios in which they
will be annoying.

The way I see the order:
1. Setting from code should always take precedence (including the current
bindings over setter order of course).
2. INLINE origin (via setStyle).
3. Stylesheets according to their StyleOrigin as specified by the
non-javafx css specifications [3]: AUTHOR > USER > USER_AGENT (see below
for INLINE).

2 and 3 are already (more or less) specified in JavaFX's css as far as I
can see. However, 1 is not css, hence I don't think StyleOrigin should be
applicable here even. Even more, 2 isn't really css either, it mimics html
tags and shouldn't count as a css StyleOrigin in my opinion.
Note also that a Stylesheet can set its origin [4], even to INLINE, so it
takes precedence over java property setters and conflicts with 'setStyle'
"real" INLINE. I'm not sure if this is a bug because the javafx css specs
say that "Inline styles are specified via the Node setStyle AP".

So, if I were to able to do anything I wanted, I would have restricted
Stylesheets to the options in 3, remove INLINE from a public perspective
and apply it only behind the scenes to 'setStyle' calls, and stop treating
java settings in the css hierarchy (which means removing the USER
StyleOrigin from them). Obviously that breaks a lot of code, but this
behavior would be my general goal. As for how to represent it, maybe a
constant can be added to StyleOrigin to represent java code settings, but
that's not a real css origin. I guess we could call INLINE and the
hypothetical JAVA constants "pseudo-origins", because they don't apply to
stylesheets, and are only used internally. Or just don't check StyleOrigin
when the value is set from java. There are probably more ways.

I also wonder if StyleOrigin should implement Comparable for the precedence
calculations of stylesheets.

However, it seems like an arbitrary fact that attributes in an FXML
> document are equivalent to calling setters from code. Attributes in
> FXML documents could just as well be their own thing, couldn't they?
>

Another good point. I don't really use FXML, so I can't say what I would
expect. There are several options for this.

[1]

Re: Build artifacts for Linux aarch64

[Nir Lisker]

Hi Armin,

If I go to https://gluonhq.com/products/javafx/ and filter to linux
aarch64, I see both SDK and JMODS for JavaFX 22-ea+16 (latest version,
which is early access). If you're talking about commercial support, I don't
know. Different companies choose what they support.

- Nir

On Tue, Jan 30, 2024 at 4:07 PM Armin Schrenk 
wrote:

> Hey,
>
> in the past there were releases for the platform linux aarch64. But I
> can't find those anymore, neither in maven central, nor on the Gluon
> javafx page, which provides prebuild jmods/SDKs. Is this platform not
> supported anymore?
>
> Best regards,
>
> Armin Schrenk
>
> --
> Armin Schrenk
>
> Skymatic GmbH, Am Hauptbahnhof 6, 53111 Bonn
> Sitz der Gesellschaft: Bonn; Amtsgericht Bonn, HRB 22635
>
>

Re: Platform preferences theme detection

[Nir Lisker]

>
>  I'm under the impression that the last available 22+ea maven release,
> which is now almost 3 months old, does not contain the platform preferences
> API
>

You are correct, the new API was added a month after the latest ea build.
Versions 11-21 all had at least 1 ea release per month on average. Not sure
why 22 doesn't.

Johan, do you do these releases or are they part of OpenJFX?

On Tue, Jan 30, 2024 at 2:39 PM Christopher Schnick 
wrote:

> Hello Nir,
>
> I'm not entirely familiar with every ea build, but I'm under the
> impression that the last available 22+ea maven release, which is now almost
> 3 months old, does not contain the platform preferences API and also does
> not contain the kinda important css performance regression fixes.
> On 1/30/2024 1:33 PM, Nir Lisker wrote:
>
> Hi Christopher,
>
> Looking at Maven Central,
> https://mvnrepository.com/artifact/org.openjfx/javafx, JavaFX releases ea
> builds there, which I sometimes use myself from Maven/Gradle. Version 21
> had 6 ea versions, and 22 has 3. The release cycle is 6 months per final
> version (aligned with OpenJDK).
>
> - Nir
>
> On Tue, Jan 30, 2024 at 2:18 PM Christopher Schnick 
> wrote:
>
>> Alright I will try out the new ea release once the fix is integrated.
>> Other than that, everything works fine for me so far with observing colors
>> using the platform-specific strings.
>>
>> As a side note, I think the community would have caught this issue
>> earlier if there were more frequent maven releases of ea builds. As of
>> right now, the only way to properly use recent ea features is downloading
>> the jars and jmods manually from the jdk.java.net site, which is
>> cumbersome. I don't know how much of the maven release pipeline is
>> automated and how much work more frequent ea releases would be, but it
>> would definitely help with early testing and adoption.
>> On 1/29/2024 11:09 PM, Michael Strauß wrote:
>>
>> I see that the names of the platform mappings defined in
>> WinApplication::getPlatformKeyMappings() are simply wrong
>> ("Windows.UIColor.ForegroundColor" instead of
>> "Windows.UIColor.Foreground"), so the platform mappings are not applied to
>> the properties.
>>
>> That's quite surprising, and it's a change that must have slipped into
>> the feature at a very late stage during development, so that it went
>> unnoticed by all reviewers.
>> I'll file a bug and prepare a fix for this issue.
>>
>>
>> On Mon, Jan 29, 2024 at 10:45 PM Christopher Schnick 
>> wrote:
>>
>>> Hello Michael,
>>>
>>> I took a look at the implementation and tried to find the issue. From
>>> what I can see, the mappings returned are correct:
>>>
>>>
>>>
>>> but it seems like they are somehow not applied the PreferencesProperties:
>>>
>>> Let me know whether I can help with anything to debug this issue.
>>>
>>>

[jfx22] Integrated: 8324658: Allow animation play/start/stop/pause methods to be called on any thread

[Nir Lisker]

On Tue, 30 Jan 2024 12:08:41 GMT, Nir Lisker  wrote:

> Hi all,
> 
> This pull request contains a backport of commit 
> [c5ab220b](https://github.com/openjdk/jfx/commit/c5ab220bbc885f2aa99d8c1d5ed8f1753e39251f)
>  from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.
> 
> The commit being backported was authored by Nir Lisker on 30 Jan 2024 and was 
> reviewed by Kevin Rushforth and Jose Pereda.
> 
> Thanks!

This pull request has now been integrated.

Changeset: 648dfd54
Author:Nir Lisker 
URL:   
https://git.openjdk.org/jfx/commit/648dfd540cde6bc8127df333fe30b2cac5b022e6
Stats: 422 lines in 7 files changed: 225 ins; 148 del; 49 mod

8324658: Allow animation play/start/stop/pause methods to be called on any 
thread

Reviewed-by: kcr
Backport-of: c5ab220bbc885f2aa99d8c1d5ed8f1753e39251f

-

PR: https://git.openjdk.org/jfx/pull/1354

Re: Platform preferences theme detection

[Nir Lisker]

Hi Christopher,

Looking at Maven Central,
https://mvnrepository.com/artifact/org.openjfx/javafx, JavaFX releases ea
builds there, which I sometimes use myself from Maven/Gradle. Version 21
had 6 ea versions, and 22 has 3. The release cycle is 6 months per final
version (aligned with OpenJDK).

- Nir

On Tue, Jan 30, 2024 at 2:18 PM Christopher Schnick 
wrote:

> Alright I will try out the new ea release once the fix is integrated.
> Other than that, everything works fine for me so far with observing colors
> using the platform-specific strings.
>
> As a side note, I think the community would have caught this issue earlier
> if there were more frequent maven releases of ea builds. As of right now,
> the only way to properly use recent ea features is downloading the jars and
> jmods manually from the jdk.java.net site, which is cumbersome. I don't
> know how much of the maven release pipeline is automated and how much work
> more frequent ea releases would be, but it would definitely help with early
> testing and adoption.
> On 1/29/2024 11:09 PM, Michael Strauß wrote:
>
> I see that the names of the platform mappings defined in
> WinApplication::getPlatformKeyMappings() are simply wrong
> ("Windows.UIColor.ForegroundColor" instead of
> "Windows.UIColor.Foreground"), so the platform mappings are not applied to
> the properties.
>
> That's quite surprising, and it's a change that must have slipped into the
> feature at a very late stage during development, so that it went unnoticed
> by all reviewers.
> I'll file a bug and prepare a fix for this issue.
>
>
> On Mon, Jan 29, 2024 at 10:45 PM Christopher Schnick 
> wrote:
>
>> Hello Michael,
>>
>> I took a look at the implementation and tried to find the issue. From
>> what I can see, the mappings returned are correct:
>>
>>
>>
>> but it seems like they are somehow not applied the PreferencesProperties:
>>
>> Let me know whether I can help with anything to debug this issue.
>>
>>

[jfx22] RFR: 8324658: Allow animation play/start/stop/pause methods to be called on any thread

[Nir Lisker]

Hi all,

This pull request contains a backport of commit 
[c5ab220b](https://github.com/openjdk/jfx/commit/c5ab220bbc885f2aa99d8c1d5ed8f1753e39251f)
 from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

The commit being backported was authored by Nir Lisker on 30 Jan 2024 and was 
reviewed by Kevin Rushforth and Jose Pereda.

Thanks!

-

Commit messages:
 - Backport c5ab220bbc885f2aa99d8c1d5ed8f1753e39251f

Changes: https://git.openjdk.org/jfx/pull/1354/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1354=00
  Issue: https://bugs.openjdk.org/browse/JDK-8324658
  Stats: 422 lines in 7 files changed: 225 ins; 148 del; 49 mod
  Patch: https://git.openjdk.org/jfx/pull/1354.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1354/head:pull/1354

PR: https://git.openjdk.org/jfx/pull/1354