Re: RFR: 8301302: Platform preferences API [v26]

2023-11-17 Thread Michael Strauß
On Sat, 18 Nov 2023 05:50:35 GMT, Michael Strauß  wrote:

>> Please read [this 
>> document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) 
>> for an introduction to the Platform Preferences API, and how it interacts 
>> with the proposed style theme and stage appearance features.
>
> Michael Strauß has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove Preferences.getPaint

I've removed `Preferences.getPaint(String)` because I can't see this particular 
typed getter being useful in the foreseeable future. No platform reports 
non-`Color` paints, and I don't expect this to change. Even if it did, we have 
the `Preferences.getValue(String, Class)` getter to retrieve exotic values.

-

PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1817404685


Re: RFR: 8301302: Platform preferences API [v23]

2023-11-17 Thread Michael Strauß
On Fri, 17 Nov 2023 19:03:16 GMT, Kevin Rushforth  wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Rename Appearance to ColorScheme
>
> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 
> 469:
> 
>> 467:  * the untyped {@link #get} method.
>> 468:  * 
>> 469:  * The preferences that are reported by the platform may be 
>> dependent on the operating system version,
> 
> They also might be dependent on some mode of operation of the platform. Do 
> you think that's worth calling out?

Yes, I've included `and its current configuration` after `operating system 
version`.

> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 
> 632:
> 
>> 630:  * @return the optional {@code Integer} to which the key is 
>> mapped
>> 631:  */
>> 632: Optional getInteger(String key);
> 
> Did you consider using `OptionalInt` instead of `Optional`? What you 
> chose seems more consistent (e.g., there is no `OptionalBoolean` class, so 
> `getBoolean` has to return an `Optional`), so I wouldn't advocate 
> changing it.

I did consider it, but decided not to do it for consistency.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1398101875
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1398101579


Re: RFR: 8301302: Platform preferences API [v26]

2023-11-17 Thread Michael Strauß
> Please read [this 
> document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) for 
> an introduction to the Platform Preferences API, and how it interacts with 
> the proposed style theme and stage appearance features.

Michael Strauß has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove Preferences.getPaint

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1014/files
  - new: https://git.openjdk.org/jfx/pull/1014/files/3cc29e94..9a817d10

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1014=25
 - incr: https://webrevs.openjdk.org/?repo=jfx=1014=24-25

  Stats: 19 lines in 3 files changed: 0 ins; 15 del; 4 mod
  Patch: https://git.openjdk.org/jfx/pull/1014.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1014/head:pull/1014

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


Re: RFR: 8301302: Platform preferences API [v23]

2023-11-17 Thread Michael Strauß
On Fri, 17 Nov 2023 19:43:58 GMT, Kevin Rushforth  wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Rename Appearance to ColorScheme
>
> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 
> 630:
> 
>> 628:  * @throws NullPointerException if {@code key} is null
>> 629:  * @throws IllegalArgumentException if a mapping exists, but 
>> the key is not mapped to an {@code Integer}
>> 630:  * @return the optional {@code Integer} to which the key is 
>> mapped
> 
> I presume that it returns `Optional.empty()` if the mapping is not present? 
> This could lead to a situation where calling this method with a particular 
> key will return an empty optional in some cases and throw an exception in 
> others. Might it be better to specify that it will throw IAE if the type of 
> the key is known to not be an Integer, whether or not there exists a mapping 
> for that key? Alternatively, might it be better to always return an empty 
> Optional unless there exists a mapping and the value is an integer? In the 
> latter case, we would get rid of "IAE" entirely.

Alternative 2 (return empty value if the key maps to a different type) is 
attractive because it doesn't require additional type validation for mappings 
that are not present, and gets rid of the sometimes unexpected IAE.

However, there's a major downside: the preferences map is, first and foremost, 
a `Map`. The typed getters are just a convenience to make it easier to work 
with this map. When a native toolkit reports a key-value mapping, it will be 
included in the map, and its value can be unconditionally retrieved with 
`Map.get(String)`.

When we use a typed getter, it should return `Optional.empty()` exactly if 
`get(String)` would return `null`. Returning an empty value when the wrong 
typed getter is called (which is a bug) is unexpected and basically hides what 
would be the equivalent of a `ClassCastException`. This rules out this approach.

Alternative 1 (always throw IAE if we call the wrong typed getter, whether or 
not we have a mapping) is not unexpected, and does not violate the 
`Optional.empty() <=> get(String)==null` invariant. This alternative is a bit 
heavier on the implementation side, as we must now keep well-known lists of 
key-type mappings around, and more importantly, keep them in sync with the 
native toolkit.

I've implemented this alternative and updated the specification of the typed 
getters similar to this:

/**
 * Returns an optional {@code Integer} to which the specified key is mapped.
 *
 * @param key the key
 * @throws NullPointerException if {@code key} is null
 * @throws IllegalArgumentException if the key is not mappable to an {@code 
Integer}
 * @return the optional {@code Integer} to which the key is mapped
 */

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1398099493


Re: RFR: 8301302: Platform preferences API [v25]

2023-11-17 Thread Michael Strauß
> Please read [this 
> document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) for 
> an introduction to the Platform Preferences API, and how it interacts with 
> the proposed style theme and stage appearance features.

Michael Strauß has updated the pull request incrementally with one additional 
commit since the last revision:

  Add eager type checking for typed getters

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1014/files
  - new: https://git.openjdk.org/jfx/pull/1014/files/6ec7e997..3cc29e94

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1014=24
 - incr: https://webrevs.openjdk.org/?repo=jfx=1014=23-24

  Stats: 217 lines in 9 files changed: 191 ins; 0 del; 26 mod
  Patch: https://git.openjdk.org/jfx/pull/1014.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1014/head:pull/1014

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


javafx.base and java.desktop

2023-11-17 Thread Nir Lisker
Hi,

A previous discussion mentioned the removal of AWT dependencies. One of the
points that Kevin brought up was

 Refactor Java Beans implementation in javafx.base such that java.desktop
> is optional


John and I looked at this some time ago when we discussed the usage of the
javafx base module outside of JavaFX, as its observables/binding
capabilities are suitable for non-GUI applications, which currently have to
pull in GUI modules as dependencies.

The dependency is used in the property.adapter packages that bridge
javafx.base properties with Java Beans. I think that these classes are
seldom used.

What could be a way to deal with that dependency? Perhaps the module can be
declared 'requires static'. Or extract the adapter packages into a
different "interop" module (javafx.javabeans) like javafx.swing?

- Nir


Re: RFR: 8301302: Platform preferences API [v23]

2023-11-17 Thread Michael Strauß
On Fri, 17 Nov 2023 19:20:32 GMT, Kevin Rushforth  wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Rename Appearance to ColorScheme
>
> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 
> 578:
> 
>> 576:  * from the perceptual brightness of {@link 
>> #backgroundColorProperty() backgroundColor} in relation
>> 577:  * to {@link #foregroundColorProperty() foregroundColor} and 
>> defaults to {@link ColorScheme#LIGHT}
>> 578:  * if the platform does not report color preferences.
> 
> Do we want to allow for the possibility of a platform reporting the color 
> scheme directly (rather than specifying that it is always derived from the 
> relative brightness of the foreground and background colors)? I can't think 
> of a need off hand.

I've removed the language that specified how the color scheme is determined, as 
it is immaterial for read-only platform preferences. This question will become 
relevant for mutable application preferences, where we will need to decide 
whether `colorScheme` is automatically derived from foreground and background 
colors, or whether it will need to be set by application developers _along 
with_ the foreground and background colors.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397995715


Re: [External] : Re: ImageView.fitWidth()/.fitHeight()/.preserveRatio() styling

2023-11-17 Thread Andy Goryachev
Created https://bugs.openjdk.org/browse/JDK-8320359

-andy

From: Dirk Lemmermann 
Date: Friday, November 17, 2023 at 14:48
To: Andy Goryachev 
Cc: openjfx-dev 
Subject: [External] : Re: ImageView.fitWidth()/.fitHeight()/.preserveRatio() 
styling
I guess the use-case is the same as for any node property. I want to be able to 
specify it via CSS instead of code.

I could easily implement this for ImageView but I am not familiar with the 
OpenJFX dev process (e.g. which tests are required), so it would take me a long 
time to do. Pretty sure that other contributors could do this in a fraction of 
the time.

Dirk


Am 16.11.2023 um 20:57 schrieb Andy Goryachev :

Dear Dirk:

I don’t see any reason why not - we’d need to add entries in getCssMetaData() 
and update CSS reference.  In fact, there is a TODO item in ImageView:812 along 
the lines

// TODO
// "preserve-ratio","smooth","viewport","fit-width","fit-height"

Perhaps it just fell through the cracks.

Can you describe your use case?  Is this something you’d like to impement?

-andy

From: openjfx-dev  on behalf of Dirk Lemmermann 

Date: Thursday, November 16, 2023 at 07:12
To: openjfx-dev 
Subject: ImageView.fitWidth()/.fitHeight()/.preserveRatio() styling
Hi everyone,

Is there a particular reason why the fitWidth, fitHeight, and preserveRatio() 
properties of an ImageView can not be styled via CSS?

Dirk



Re: ImageView.fitWidth()/.fitHeight()/.preserveRatio() styling

2023-11-17 Thread Dirk Lemmermann
I guess the use-case is the same as for any node property. I want to be able to 
specify it via CSS instead of code.

I could easily implement this for ImageView but I am not familiar with the 
OpenJFX dev process (e.g. which tests are required), so it would take me a long 
time to do. Pretty sure that other contributors could do this in a fraction of 
the time.

Dirk

> Am 16.11.2023 um 20:57 schrieb Andy Goryachev :
> 
> Dear Dirk:
>  
> I don’t see any reason why not - we’d need to add entries in getCssMetaData() 
> and update CSS reference.  In fact, there is a TODO item in ImageView:812 
> along the lines
>  
> // TODO
> // "preserve-ratio","smooth","viewport","fit-width","fit-height"
>  
> Perhaps it just fell through the cracks.
>  
> Can you describe your use case?  Is this something you’d like to impement?
>  
> -andy
>  
> From: openjfx-dev  on behalf of Dirk Lemmermann 
> 
> Date: Thursday, November 16, 2023 at 07:12
> To: openjfx-dev 
> Subject: ImageView.fitWidth()/.fitHeight()/.preserveRatio() styling
> 
> Hi everyone,
> 
> Is there a particular reason why the fitWidth, fitHeight, and preserveRatio() 
> properties of an ImageView can not be styled via CSS?
> 
> Dirk



Re: RFR: 8301302: Platform preferences API [v24]

2023-11-17 Thread Michael Strauß
> Please read [this 
> document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) for 
> an introduction to the Platform Preferences API, and how it interacts with 
> the proposed style theme and stage appearance features.

Michael Strauß has updated the pull request incrementally with one additional 
commit since the last revision:

  Doc changes

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1014/files
  - new: https://git.openjdk.org/jfx/pull/1014/files/9b607c6a..6ec7e997

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1014=23
 - incr: https://webrevs.openjdk.org/?repo=jfx=1014=22-23

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

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


Re: RFR: 8318387: Update GStreamer to 1.22.6

2023-11-17 Thread Kevin Rushforth
On Fri, 17 Nov 2023 21:27:04 GMT, Alexander Matveev  
wrote:

> - GStreamer updated to 1.22.6 and GLib updated to 2.78.1.
> - Tested on all platforms with all supported media streams.
> - GStreamer 1.22.6 requires GLib 2.62.0, but since we need to support older 
> GLib versions on Linux following APIs were changed from new to old one 
> (restored GStreamer 1.20.1 code) (Linux only, other platforms using latest 
> code):
>   - g_queue_clear_full() -> g_queue_foreach(), g_queue_clear().
>   - g_atomic_rc_box_*() -> g_weak_ref_init(), g_weak_ref_clear().

The Windows GHA failure is a transient network or infrastructure failure 
unrelated to this PR:


curl: (56) Recv failure: Connection was reset

-

PR Comment: https://git.openjdk.org/jfx/pull/1290#issuecomment-1817136518


RFR: 8318387: Update GStreamer to 1.22.6

2023-11-17 Thread Alexander Matveev
- GStreamer updated to 1.22.6 and GLib updated to 2.78.1.
- Tested on all platforms with all supported media streams.
- GStreamer 1.22.6 requires GLib 2.62.0, but since we need to support older 
GLib versions on Linux following APIs were changed from new to old one 
(restored GStreamer 1.20.1 code) (Linux only, other platforms using latest 
code):
  - g_queue_clear_full() -> g_queue_foreach(), g_queue_clear().
  - g_atomic_rc_box_*() -> g_weak_ref_init(), g_weak_ref_clear().

-

Commit messages:
 - 8318387: Update GStreamer to 1.22.6 [v2]
 - 8318387: Update GStreamer to 1.22.6

Changes: https://git.openjdk.org/jfx/pull/1290/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1290=00
  Issue: https://bugs.openjdk.org/browse/JDK-8318387
  Stats: 45668 lines in 438 files changed: 19431 ins; 18936 del; 7301 mod
  Patch: https://git.openjdk.org/jfx/pull/1290.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1290/head:pull/1290

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


Re: Behavior Proposal v2

2023-11-17 Thread Andy Goryachev
Thank you, John, for a productive discussion!  I am very happy to see that we 
both gained a better understanding of the problems we are trying to solve.

For me, some of the insights came to light after our conversation.  To 
complement John’s list:

- in cases where the behavior part is stateless (or state is fully contained 
within the control), the behavior can indeed be implemented statically, 
potentially saving memory.

- we might still have a problem with event handling priority, so addressing 
that (along the lines of https://github.com/openjdk/jfx/pull/1266 or in some 
other way) is likely a prerequisite

- perhaps the InputMap can be split into two parts (user- and skin-), providing 
a better API and enforcing the separation of concerns.

I would like to thank you both, John and Michael, for your patience and 
discussion!  I am going to re-do my proposal to incorporate your feedback, 
hopefully before the Thanksgiving break next week.

-andy


From: openjfx-dev  on behalf of John Hendrikx 

Date: Friday, November 17, 2023 at 05:35
To: openjfx-dev@openjdk.org 
Subject: Behavior Proposal v2

I'm working on a new version of the behavior proposal after some fruitful 
discussions here on this list, and a meeting with Andy.

In this meeting a few problems I was unable to address came to light, and which 
I've now addressed in this post:

1) TextAreaBehavior is relying heavily on internal functions provided by the 
corresponding Skin. These are all related to how Text is visually laid out and 
actions that you can take that would need this layout information of which the 
Control is unaware; the simplest example is pressing the "HOME" or "END" key 
which needs to move to the start or end of current line depending on its visual 
bounds.

2) Having Behaviors as a separate concept didn't seem all that useful.

3) The potential for having a HUGE number of semantic events in Skin -> 
Behavior communication

TextAreaBehavior
==

I think its fair to say that this is definitely one of the more unique 
behaviors in JavaFX.  Together with its Skin it is doing things that I don't 
think any of the other skins do.  The Skin for example provides styleable CSS 
properties, and provides a lot of call backs for the behavior.  This latter 
part I've not seen in any other skins (I did a quick search).  I didn't 
investigate if any other Skins are also providing CSS stylable properties.

Now I think this can mean that perhaps the TextArea control is lacking some 
functions that it should be providing. TextArea is aware that its content will 
be wrapped and split into lines, that it has a view port, and provides ways to 
move the caret.  What it doesn't provide is more precise caret control.  It 
doesn't seem like a stretch to also provide for functions that can navigate the 
caret to the start or end of the current line, or to the start or end of the 
current visible page, etc.

Lacking that, we could also accept that perhaps a Behavior should have some 
more awareness of a related Skin.  In MVC, the Controller (Behavior) has a 
reference to the View (Skin) and also has some knowledge of how the View 
operates. A solution here can be to have Skins with very specific needs 
implement an interface.  The Behavior can then be attuned to that interface, 
which would allow a new Skin to be constructed which reuses a lot of an 
existing Behavior without the requirement to subclass a specific Skin.  Note 
that the Behavior can fairly easily access the Skin already through Control.  A 
simple instanceof check will suffice to see if it can expand its behavior to 
support visual operations.

void moveCaretToEndOfLine(TextArea control) {  // called in response to a 
KeyEvent
  if (control.getSkinnable() instanceof VisuallyCaretAwareInterface x) {
   x.moveCaret(...);
  }
  // do nothing, wanted behavior not supported by Skin
}

Usefulness of Behavior Concept
==

The point was made that creating a new Behavior is still very limited in what 
it allows the user to change how a control "feels".  This is because the Skin 
does a lot of filtering when it is passing things to the associated behavior.  
For example, the SpinnerSkin will call the behavior when the spinner buttons 
are pressed (with any mouse button(!!!)), but the Behavior can't limit this to 
just the left mouse button (like Spinners on Windows do). Other Skins will only 
call the Behavior for specific mouse buttons, or for specific events.  In 
effect, the Skin is already dictating part of the behavior (either by omission 
or by generalization) which should be part of the Behavior impementation.

For example, if I want to create a SpinnerBehavior that would react to the 
scroll wheel when it is hovering above the Spinner's text field, I'm out of 
luck; the Skin is not calling the Behavior for SCROLL events.

So perhaps we can leverage something here that is already public knowledge: the 

Re: RFR: 8301302: Platform preferences API [v5]

2023-11-17 Thread Kevin Rushforth
On Wed, 6 Sep 2023 18:02:31 GMT, Kevin Rushforth  wrote:

>> +1
>> 
>> we could put the doc (in markdown format) in the doc-files/platform 
>> directory (or create any other subdirectory, see JDK-8309749)
>
> I think Michael meant that the javadoc-generated API docs in this file need 
> to be kept up to date. If we are going to say _all_, then yes it will. I 
> don't see a need for any other doc related to this.

On further reflection, I don't think we want to specify this list so tightly 
that adding a new key that isn't in the list would be a spec violation. We _do_ 
want to keep the implementation and the docs in sync, but I can imagine a case 
where a new platform might cause a new key to show up even though we haven't 
documented it yet.

It's unlikely that this would happen, since the proposed implementation doesn't 
blindly surface properties that it doesn't know about, but perhaps a future 
implementation on some platform might do this.

So my suggestion is to soften the wording a little bit. One suggestion is 
something like:


 * The following preferences are potentially available on the specified 
platforms:


I note that Joe Darcy also raised this question in [this 
comment](https://bugs.openjdk.org/browse/JDK-8319138?focusedId=14625402=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14625402)
 in the CSR.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397753195


Re: RFR: 8301302: Platform preferences API [v23]

2023-11-17 Thread Kevin Rushforth
On Fri, 10 Nov 2023 21:23:54 GMT, Michael Strauß  wrote:

>> Please read [this 
>> document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) 
>> for an introduction to the Platform Preferences API, and how it interacts 
>> with the proposed style theme and stage appearance features.
>
> Michael Strauß has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Rename Appearance to ColorScheme

Here is my feedback on the latest API specification. I left a few inline 
comments along with a couple general comments below.

I like the name change of Appearance to ColorScheme.

I see that Joe made this comment in the CSR:

> As a stylistic point, it would also be acceptable to have a general "Throws 
> NPE on null argument unless otherwise specified" disclaimer at the type level 
> as opposed to repeating that for every method.

I'll leave it up to you to decide whether you want to do this.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 469:

> 467:  * the untyped {@link #get} method.
> 468:  * 
> 469:  * The preferences that are reported by the platform may be 
> dependent on the operating system version,

They also might be dependent on some mode of operation of the platform. Do you 
think that's worth calling out?

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 578:

> 576:  * from the perceptual brightness of {@link 
> #backgroundColorProperty() backgroundColor} in relation
> 577:  * to {@link #foregroundColorProperty() foregroundColor} and 
> defaults to {@link ColorScheme#LIGHT}
> 578:  * if the platform does not report color preferences.

Do we want to allow for the possibility of a platform reporting the color 
scheme directly (rather than specifying that it is always derived from the 
relative brightness of the foreground and background colors)? I can't think of 
a need off hand.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 630:

> 628:  * @throws NullPointerException if {@code key} is null
> 629:  * @throws IllegalArgumentException if a mapping exists, but the 
> key is not mapped to an {@code Integer}
> 630:  * @return the optional {@code Integer} to which the key is 
> mapped

I presume that it returns `Optional.empty()` if the mapping is not present? 
This could lead to a situation where calling this method with a particular key 
will return an empty optional in some cases and throw an exception in others. 
Might it be better to specify that it will throw IAE if the type of the key is 
known to not be an Integer, whether or not there exists a mapping for that key? 
Alternatively, might it be better to always return an empty Optional unless 
there exists a mapping and the value is an integer? In the latter case, we 
would get rid of "IAE" entirely.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 632:

> 630:  * @return the optional {@code Integer} to which the key is 
> mapped
> 631:  */
> 632: Optional getInteger(String key);

Did you consider using `OptionalInt` instead of `Optional`? What you 
chose seems more consistent (e.g., there is no `OptionalBoolean` class, so 
`getBoolean` has to return an `Optional`), so I wouldn't advocate 
changing it.

-

PR Review: https://git.openjdk.org/jfx/pull/1014#pullrequestreview-1737695354
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397751899
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397771608
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397796039
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1397800252


RFR: 8089373: Translation from character to key code is not sufficient

2023-11-17 Thread Martin Fox
On Windows a common shortcut like Ctrl+'+' could only be invoked from the main 
keyboard and not the numeric keypad. Toolkit.getKeyCodeForChar did not have 
enough context to know whether it should return a result from the main keyboard 
or the keypad.

This PR alters getKeyCodeForChar to pass in the code of the key the system is 
trying to match against. Only the Windows platform actually uses this 
additional information.

On the Mac the numeric keypad has always worked due to the odd way 
getKeyCodeForChar is implemented (until PR #1209 the keypad worked more 
reliably than the main keyboard). On Linux getKeyCodeForChar is a mess; neither 
the main keyboard or the numeric keypad work reliably. I have an upcoming PR 
which should make both work correctly.

-

Commit messages:
 - Added SEPARATOR to list of keypad keys
 - CharacterCombinations now work on the numeric keypad
 - Fixed Monocle
 - Merge remote-tracking branch 'upstream/master' into keypadcombo
 - Added hint to getKeyCodeForChar to enable numeric keypad

Changes: https://git.openjdk.org/jfx/pull/1289/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1289=00
  Issue: https://bugs.openjdk.org/browse/JDK-8089373
  Stats: 77 lines in 19 files changed: 46 ins; 0 del; 31 mod
  Patch: https://git.openjdk.org/jfx/pull/1289.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1289/head:pull/1289

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


RE: My JavaFX Christmas Wishlist

2023-11-17 Thread Jan Tosovsky
On 2023-11-15, Dirk Lemmermann wrote:
> 
> All I want for Christmas is … / what I think is needed for JavaFX 
> going forward ...

I wish JavaFX better MARKETING. If I sometimes encounter articles comparing 
various frameworks for building desktop apps, JavaFX is never listed. 

I was sad about it, but nowadays, seeing the rapid progress of novel frameworks 
like Tauri or Flutter, I tend to understand JavaFX is becoming obsolete. Java 
itself has a bad reputation in the young generation.

I am losing hope JavaFX can compete, be sexy again, attract people, volunteers 
willing to implement missing features...

I would be grateful to be wrong. JavaFX still has my sympathy. It won my 
desktop app frameworks comparison [1] but it itself can hardly revert the 
trend. 

So, in sum, I wish JavaFX better MARKETING, so it was more visible than today. 
From my POV it is the key to change the status quo.

Jan

___
[1] 
https://medium.com/@jan.tosovsky.cz/comparing-modern-multiplatform-desktop-frameworks-on-the-image-browser-app-305552c7ebe2



Re: CFV: New OpenJFX Committer: Martin Fox

2023-11-17 Thread José Pereda
Vote: Yes

Jose

On Fri, Nov 17, 2023 at 3:34 PM John Hendrikx 
wrote:

> Vote: YES
>
> --John
>
> On 16/11/2023 16:54, Kevin Rushforth wrote:
> > I hereby nominate Martin Fox [1] to OpenJFX Committer.
> >
> > Martin is an OpenJFX community member, who has contributed 12 commits
> > [2] to OpenJFX.
> >
> > Votes are due by November 30, 2023 at 16:00 UTC.
> >
> > Only current OpenJFX Committers [3] are eligible to vote on this
> > nomination. Votes must be cast in the open by replying to this mailing
> > list.
> >
> > For Lazy Consensus voting instructions, see [4]. Nomination to a
> > project Committer is described in [5].
> >
> > Thanks.
> >
> > -- Kevin
> >
> >
> > [1] https://openjdk.java.net/census#mfox
> >
> > [2]
> >
> https://github.com/openjdk/jfx/search?q=author-name%3A%22Martin+Fox%22=author-date=commits
> >
> > [3] https://openjdk.java.net/census#openjfx
> >
> > [4] https://openjdk.java.net/bylaws#lazy-consensus
> >
> > [5] https://openjdk.java.net/projects#project-committer
> >
>


--


Re: CFV: New OpenJFX Committer: Florian Kirmaier

2023-11-17 Thread José Pereda
Vote: Yes

Jose

On Fri, Nov 17, 2023 at 3:34 PM John Hendrikx 
wrote:

> Vote: YES
>
> --John
>
> On 16/11/2023 16:38, Kevin Rushforth wrote:
> > I hereby nominate Florian Kirmaier [1] to OpenJFX Committer.
> >
> > Florian is an OpenJFX community member, who has contributed 24 commits
> > [2] to OpenJFX.
> >
> > Votes are due by November 30, 2023 at 16:00 UTC.
> >
> > Only current OpenJFX Committers [3] are eligible to vote on this
> > nomination. Votes must be cast in the open by replying to this mailing
> > list.
> >
> > For Lazy Consensus voting instructions, see [4]. Nomination to a
> > project Committer is described in [5].
> >
> > Thanks.
> >
> > -- Kevin
> >
> >
> > [1] https://openjdk.java.net/census#fkirmaier
> >
> > [2]
> >
> https://github.com/openjdk/jfx/search?q=author-name%3A%22Florian+Kirmaier%22=author-date=commits
> >
> > [3] https://openjdk.java.net/census#openjfx
> >
> > [4] https://openjdk.java.net/bylaws#lazy-consensus
> >
> > [5] https://openjdk.java.net/projects#project-committer
> >
>


--


Re: CFV: New OpenJFX Committer: Florian Kirmaier

2023-11-17 Thread John Hendrikx

Vote: YES

--John

On 16/11/2023 16:38, Kevin Rushforth wrote:

I hereby nominate Florian Kirmaier [1] to OpenJFX Committer.

Florian is an OpenJFX community member, who has contributed 24 commits 
[2] to OpenJFX.


Votes are due by November 30, 2023 at 16:00 UTC.

Only current OpenJFX Committers [3] are eligible to vote on this 
nomination. Votes must be cast in the open by replying to this mailing 
list.


For Lazy Consensus voting instructions, see [4]. Nomination to a 
project Committer is described in [5].


Thanks.

-- Kevin


[1] https://openjdk.java.net/census#fkirmaier

[2] 
https://github.com/openjdk/jfx/search?q=author-name%3A%22Florian+Kirmaier%22=author-date=commits


[3] https://openjdk.java.net/census#openjfx

[4] https://openjdk.java.net/bylaws#lazy-consensus

[5] https://openjdk.java.net/projects#project-committer



Re: CFV: New OpenJFX Committer: Martin Fox

2023-11-17 Thread John Hendrikx

Vote: YES

--John

On 16/11/2023 16:54, Kevin Rushforth wrote:

I hereby nominate Martin Fox [1] to OpenJFX Committer.

Martin is an OpenJFX community member, who has contributed 12 commits 
[2] to OpenJFX.


Votes are due by November 30, 2023 at 16:00 UTC.

Only current OpenJFX Committers [3] are eligible to vote on this 
nomination. Votes must be cast in the open by replying to this mailing 
list.


For Lazy Consensus voting instructions, see [4]. Nomination to a 
project Committer is described in [5].


Thanks.

-- Kevin


[1] https://openjdk.java.net/census#mfox

[2] 
https://github.com/openjdk/jfx/search?q=author-name%3A%22Martin+Fox%22=author-date=commits


[3] https://openjdk.java.net/census#openjfx

[4] https://openjdk.java.net/bylaws#lazy-consensus

[5] https://openjdk.java.net/projects#project-committer



Behavior Proposal v2

2023-11-17 Thread John Hendrikx
I'm working on a new version of the behavior proposal after some 
fruitful discussions here on this list, and a meeting with Andy.


In this meeting a few problems I was unable to address came to light, 
and which I've now addressed in this post:


1) TextAreaBehavior is relying heavily on internal functions provided by 
the corresponding Skin. These are all related to how Text is visually 
laid out and actions that you can take that would need this layout 
information of which the Control is unaware; the simplest example is 
pressing the "HOME" or "END" key which needs to move to the start or end 
of current line depending on its visual bounds.


2) Having Behaviors as a separate concept didn't seem all that useful.

3) The potential for having a HUGE number of semantic events in Skin -> 
Behavior communication


TextAreaBehavior
==

I think its fair to say that this is definitely one of the more unique 
behaviors in JavaFX.  Together with its Skin it is doing things that I 
don't think any of the other skins do.  The Skin for example provides 
styleable CSS properties, and provides a lot of call backs for the 
behavior.  This latter part I've not seen in any other skins (I did a 
quick search).  I didn't investigate if any other Skins are also 
providing CSS stylable properties.


Now I think this can mean that perhaps the TextArea control is lacking 
some functions that it should be providing. TextArea is aware that its 
content will be wrapped and split into lines, that it has a view port, 
and provides ways to move the caret.  What it doesn't provide is more 
precise caret control.  It doesn't seem like a stretch to also provide 
for functions that can navigate the caret to the start or end of the 
current line, or to the start or end of the current visible page, etc.


Lacking that, we could also accept that perhaps a Behavior should have 
some more awareness of a related Skin.  In MVC, the Controller 
(Behavior) has a reference to the View (Skin) and also has some 
knowledge of how the View operates. A solution here can be to have Skins 
with very specific needs implement an interface. The Behavior can then 
be attuned to that interface, which would allow a new Skin to be 
constructed which reuses a lot of an existing Behavior without the 
requirement to subclass a specific Skin.  Note that the Behavior can 
fairly easily access the Skin already through Control.  A simple 
instanceof check will suffice to see if it can expand its behavior to 
support visual operations.


    void moveCaretToEndOfLine(TextArea control) {  // called in 
response to a KeyEvent
  if (control.getSkinnable() instanceof 
VisuallyCaretAwareInterface x) {

   x.moveCaret(...);
  }
  // do nothing, wanted behavior not supported by Skin
    }

Usefulness of Behavior Concept
==

The point was made that creating a new Behavior is still very limited in 
what it allows the user to change how a control "feels".  This is 
because the Skin does a lot of filtering when it is passing things to 
the associated behavior.  For example, the SpinnerSkin will call the 
behavior when the spinner buttons are pressed (with any mouse 
button(!!!)), but the Behavior can't limit this to just the left mouse 
button (like Spinners on Windows do). Other Skins will only call the 
Behavior for specific mouse buttons, or for specific events.  In effect, 
the Skin is already dictating part of the behavior (either by omission 
or by generalization) which should be part of the Behavior impementation.


For example, if I want to create a SpinnerBehavior that would react to 
the scroll wheel when it is hovering above the Spinner's text field, I'm 
out of luck; the Skin is not calling the Behavior for SCROLL events.


So perhaps we can leverage something here that is already public 
knowledge: the Substructure of Controls. Spinner for example has the 
substructure:


 * text-field — TextField
 * increment-arrow-button — StackPane
 o increment-arrow — Region
 * decrement-arrow-button — StackPane
 o decrement-arrow — Region

The names here are part of the CSS public API and indicate interesting 
parts of the spinner control.


What if we completely forbid the Skin from installing event handlers, 
even on its children (or at least the ones that are part of the 
substructure), and let those events bubble up for Behavior to catch?  An 
immediate problem would be how the Behavior would know whether I pressed 
the mouse button on the UP or DOWN button, or if I pressed the button on 
the Text Field portion.  We could look at the event's target, but even 
though you can see which specific Node was targetted, the Behavior 
doesn't know what that node means.  So, instead we could ask the Skin 
this (or alternatively, put this in a fixed Key on the Node in its 
mappings, or use the CSS id field for this purpose):


 interface Skin {
  default String 

Re: Public Behavior API proposal

2023-11-17 Thread John Hendrikx



On 13/11/2023 07:14, Michael Strauß wrote:

Hi John,

I think this is an excellent summary of all the discussions so far,
and the best proposal I've seen for a comprehensive control
architecture capable of addressing many shortcomings of the existing
controls.

Here are some thoughts:

1. Semantic events == Interaction API
Previously, skins were a black box with close to no extensibility.
Re-implementing skins is often a no-starter because you'll instantly
lose all of the intricate behaviors that are invisibly built into the
skins. Semantic events are a game changer, as they clearly define the
API of a skin, i.e. the kind of interactions that a well-behaved skin
will support. A custom skin doesn't need to recreate the entirety of
its control behavior from scratch, it should be enough to implement
the interaction API defined by its control's semantic events. If you
consider ActionEvent (which *is* a semantic event), it seems obvious
to me that more semantic events are a natural evolution of JavaFX.

2. Clear separation of concerns
All parts in the control architecture now have a clearly defined
purpose. Some of the restrictions that come with each of the parts
could be enforced by API, others might need to be explained in
documentation. The roles and responsibilities of each part of the
control architecture are easy to understand with a small set of rules
(what's allowed and what's not allowed).

3. Behaviors
I'm not sold on the Behavior design as proposed in the GitHub
document, specifically the idea of BehaviorContext and using
composition to extend existing behaviors. The API feels rather clunky,
and maybe unnecessary. Perhaps mirroring the Skin design (i.e. an
interface that gets a Control reference) can narrow down the API
surface.


The Skin design is not very well done in my opinion, and I think many 
will agree, as it was recently refactored. Mirroring it would be 
repeating the same mistake.


A Skin is used in a few ways currently:

- As a thing to set on an unspecified/any Control (not good to construct 
this with a Control)
- As a thing that contains state for a *specific* Control (perfectly 
fine to construct this with a Control)
- As a thing that tracks modifications it did to Control for later 
uninstalling (should you rely on Skin for this?)


Skins are currently like a Factory class that contains the state of its 
Product; for each new Product you need to construct a new Factory.  The 
Factory can be used only once, and can't be reused. The solution is of 
course to put the state in the Product.  The factory is now reusable.


Same with Skins. Remove the Control reference from its constructor, pass 
the Control reference during installation and create a "Product" which 
contains the state.  Skins are now reusable with minimal effort.  No 
need to check if the Skin that is installed is referring to the correct 
Control, or if that Skin was "used" before.  Perhaps what we call Skin 
should be SkinFactory, but as the stateful part (which would be called 
Skin) will be hidden anyway, the name Skin remains perfectly available. 
Naming them "Skin" and "InstalledSkinState" or even "Skin.State" 
(they're non public anyway) would be accurate as well.


So let's not repeat this with Behaviors.

The BehaviorContext is there for multiple reasons; it could just a 
Control reference, but I find that its better to enforce what an 
implementation is allowed to do then to only put some rules in the 
documentation. Rules can be broken (just see the implementation of 
com.sun.javafx.css.BitSet which broke almost every rule for Sets 
imaginable before it was fixed).  By enforcing the rules we give 
guidance to how a Behavior should be constructed.  BehaviorContext is 
nothing more than a Control reference which has some methods crossed out 
as "not allowed" -- avoid giving functions more than they need (ie. 
don't give a "sendMail" function an Employee object when only its 
Address would do), it makes things far easier to reason about (a 
Behavior couldn't have done X, as we didn't expose X).


BehaviorContext also serves another purpose, which I think is incredibly 
valuable: it allows the Control to track what changes were made by the 
Behavior when it was installed, and also gives it control of HOW they 
are installed (priority wise for example). If you give it a direct 
Control reference, you have to rely on the Behavior later to do this 
correctly, and also (with a `dispose` method) to "undo" its actions, 
possibly leaving behind things that later lead to weird bugs in a newly 
installed unrelated behavior -- lots and lots of effort was put into 
Skins to clean up after themselves properly, leading to at least two or 
three different "tracking" systems, many many bugs and many discussions 
-- we should not leave this up to the implementation, instead enforce 
it, and debug it once. When you are able to track all the changes, you 
can uninstall any Behavior, no matter how unruly, without its 

Re: Public Behavior API proposal

2023-11-17 Thread John Hendrikx

Hi Andy,

I don't think these really qualify as a good enough reason to expose an 
InputMap API by themselves.


The first comment sounds like an X/Y problem, where the user wants to 
achieve X (remove unwanted behavior) and thinks Y (exposing InputMap) is 
the solution -- certainly that is currently the only solution (hack your 
way with reflection to get at the input map), but there are other 
solutions as well to solve X aside from doing Y.  As we've already 
discussed, part of the reason why it is so hard in FX to remove unwanted 
behavior (X) is that Behavior event handling can't be overridden 
reliably.  Event filters may be an (unsatisfactory) solution that the 
user wasn't even aware of. If the user wanted to also respond to the 
events for a different purpose, then prioritized event handlers or 
having behaviors only act on unconsumed events would be a viable 
alternative.


The second comment does not give a problem statement at all, so it is 
hard to count that as an endorsement to opening up InputMap.


The third comment seems to have an entirely different problem, how to 
turn a selected text (which can no doubt consist of styled parts, 
perhaps inline images, etc) into something suitable to put on the 
clipboard (and perhaps vice versa as well). That seems like a problem 
that the Control should solve, and at most has a very thin connection 
with InputMap that such functions should be called when cut/copy/paste 
shortcuts are pressed...


Perhaps I am not seeing something here?

--John

On 16/11/2023 00:44, Andy Goryachev wrote:


Dear colleagues:

I wanted to share some of the feedback we received which might be 
relevant to the input map / behavior discussion (and sorry for not 
sending it earlier).  Email addresses in DM are redacted (though some 
of the responses might be from this mailing list anyway).


Robert REDACTED REDACTED @gmail.com

Very much appreciated from my side.

I can still remember hacking my way into the the InputMap of some 
controls to change unwanted behaviour.


Generally speaking: Making JavaFX more open is (for me) more important 
than keeping it 100% backwards compatible / super stable.


...

Pedro REDACTED REDACTED @gmail.com

Yes Input Map would be a welcome addition.

...

Buried in

https://github.com/FXMisc/RichTextFX/issues/1167#issuecomment-1435450535

  * There is no way in TextFlow to provide key/action mappings. This
becomes a complex area for rich text, for example turning a number
of selected Nodes into something suitable for cut/copy/paste is
not straight forward.

-andy