Re: RFR: 8319996: Update to GCC 13.2.0 on Linux
On Tue, 14 Nov 2023 00:24:30 GMT, Kevin Rushforth wrote: > This PR updates the compiler on Linux to GCC 13.2.0 (from 12.2.0) to match > JDK 22. > > I've run headless and headful tests on Ubuntu 16.04, 20.04, and 22.04. Marked as reviewed by arapte (Reviewer). - PR Review: https://git.openjdk.org/jfx/pull/1286#pullrequestreview-1731266174
Re: RFR: 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows [v2]
On Tue, 14 Nov 2023 12:47:51 GMT, Kevin Rushforth wrote: >> This PR updates the compiler on Windows to Visual Studio 2022 17.6.5 (from >> 17.5.0) to match JDK 22. >> >> I've run headless and headful tests. > > Kevin Rushforth has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Merge branch 'master' into 8319762-vs2022-17.6.5 > - 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows Marked as reviewed by arapte (Reviewer). - PR Review: https://git.openjdk.org/jfx/pull/1285#pullrequestreview-1731265385
Re: RFR: 8087700: [KeyCombination, Mac] KeyCharacterCombinations behave erratically
On Tue, 14 Nov 2023 21:03:59 GMT, Andy Goryachev wrote:
>> @andy-goryachev-oracle You're right, this PR also deals with the problem of
>> the map retaining stale entries when the user switches layout. I added
>> details about this to the JBS ticket. We could treat it as a separate bug if
>> you want, to me it's just the sort of standard housekeeping one does when
>> caching information.
>>
>> I also added a note about the KeyboardTest app. This PR scales back that
>> test a bit but this is the first time we've been able to run the
>> KeyCharacterCombinations on the Mac so that portion is basically a new test.
>>
>> Let me know if you want further information in the JBS ticket.
>
> @beldenfox thank you. I still not entirely sure what issue is being tested
> and how to fix it, between this PR and JBS.
> For example, when I run the application listed in JBS description with your
> fix, I get one "HI" - is it expected? (I also get 1 "HI" on Windows)
>
> On macOS, new KeyCodeCombination(KeyCode.MINUS, KeyCombination.CONTROL_DOWN)
> does not equal to new KeyCharacterCombination("-",
> KeyCombination.CONTROL_DOWN) (same on Windows)
>
> Interestingly, Ctrl+- on macOS with the US keyboard produces
>
>
> KeyEvent{type=KEY_PRESSED, character=<\u00>, text=, code=CONTROL, control}
> KeyEvent{type=KEY_PRESSED, character=<\u00>, text=, code=MINUS, control}
> KeyEvent{type=KEY_TYPED, character=<\u1F>, text=, code=UNDEFINED, control}
> KeyEvent{type=KEY_RELEASED, character=<\u00>, text=, code=MINUS, control}
> KeyEvent{type=KEY_RELEASED, character=<\u00>, text=, code=CONTROL}
>
>
> but on Windows, we don't get KEY_TYPED event:
>
>
> KeyEvent{type=KEY_PRESSED, character=<\u00>, text=, code=CONTROL, control,
> shortcut}
> KeyEvent{type=KEY_PRESSED, character=<\u00>, text=-, code=MINUS, control,
> shortcut}
> KeyEvent{type=KEY_RELEASED, character=<\u00>, text=-, code=MINUS, control,
> shortcut}
> KeyEvent{type=KEY_RELEASED, character=<\u00>, text=, code=CONTROL}
>
>
>
> is this expected?
>
> And yes, it would help if we can extract any other issues into their own
> tickets (unless they all being fixed by this change).
>
> If I may make one suggestion - for the sake of reviewers - to re-phrase the
> JBS ticket in the format of
> - steps to reproduce
> - expected outcome
> - observed outcome
>
> it would help a lot!
>
> Thanks!
@andy-goryachev-oracle What keyboard layout do you normally use on Windows? And
Mac? When talking about the behavior of punctuation this is a crucial bit of
information.
> but on Windows, we don't get KEY_TYPED event:
> is this expected?
When you hold down Ctrl and press a key the platform may or may not generate a
low-ASCII control code. JavaFX has never tried to make the platforms consistent
here, Glass just passes on whatever the OS generates. In any case the
TextInputControls treat these TYPED events as nuisances, if you look in
TextInputControlBehavior.java and read `defaultKeyTyped` you'll see how it
tries to filter out control codes and anything else that looks like it was
associated with a shortcut.
-
PR Comment: https://git.openjdk.org/jfx/pull/1209#issuecomment-1811629222
Integrated: 8274967: KeyCharacterCombinations for punctuation and symbols fail on non-US keyboards
On Tue, 17 Oct 2023 20:21:30 GMT, Martin Fox wrote: > After finding the Window virtual key code for a character getKeyCodeForChar > was using a mapping table that only works correctly for U.S. English to > retrieve the Java key code. This caused getKeyCodeForChar to encode keys > differently than the original key event handling machinery. > > With this fix the Robot, getKeyCodeForChar, and the code that handles > platform key events all agree on how Windows VK codes should map to Java > codes. > > The manual KeyboardTest app can be used to test this (tests/manual/events). > Run the tests by selecting "without keypad combinations" in the second > dropdown. This will use a Robot to test KeyCharacterCombinations excluding > the numeric keypad (which is a separate issue). This pull request has now been integrated. Changeset: c8b44bec Author:Martin Fox Committer: Andy Goryachev URL: https://git.openjdk.org/jfx/commit/c8b44bec14396d2fb4c8a51a56db957b8fdd361f Stats: 18 lines in 1 file changed: 16 ins; 0 del; 2 mod 8274967: KeyCharacterCombinations for punctuation and symbols fail on non-US keyboards Reviewed-by: angorya, jpereda - PR: https://git.openjdk.org/jfx/pull/1264
Re: Public Behavior API proposal
Dear colleagues: I would like to thank John and Michael for a lively discussion, during which we clarified a number of concepts, specifically the roles of Control (C), Skin (S), and Behavior (B). There is still a bit of difference in how we see things, but I am pleased to say we do have a lot in common. Especially valuable is definition of Behavior as a layer that translates the user input into state changes of the control. How these changes are effected is where we have different ideas, but I really like this definition. We now have a clear separation of concerns when it comes to C, S, B. We also acknowledge that effort required to migrate from the current implementation to a new one should be minimized within reason, and could be done gradually and without breaking compatibility. Another positive development is realization that FX has an issue with event handling priority, which cannot be solved using existing APIs. This problem can be decoupled from the behavior/input map discussion and solved in a separate PR ( https://github.com/openjdk/jfx/pull/1266 ), so maybe we should look at it first (or in parallel)? On the other hand, we do have some disagreement. The good thing is, we share the same goal - to give application developers a platform and the APIs that are both useful and convenient. This makes me think we can get to a mutually acceptable design. What should be the process to arrive at the common solution? -andy From: openjfx-dev on behalf of John Hendrikx Date: Sunday, November 12, 2023 at 16:13 To: openjfx-dev@openjdk.org , Michael Strauß Subject: Re: Public Behavior API proposal Hi everyone, and specifically Andy and Michael, I'm working on updating the Behavior API proposal, and I've been thinking about the semantic events a lot. I would really like to hear what you think, and how it matches with your ideas. Quick recap, Semantic Events are high level events (like the ActionEvent from Button) that can be triggered by a combination of low level events. They represent an action to be achieved, but are not tied to any specific means of triggering it. For example, the ActionEvent can be triggered with the mouse, or with the keyboard; it is irrelevant which one it was. Semantic events can be generated by Skins (as a result of interactions with the Skin's managed children), by Controls (see below) and users directly. You can compare these with Andy's FunctionTags or Actions from various systems. Let me describe exactly each part's role as I see it currently: # Controls Controls define semantic events, provides infrastructure for handling events that is separated from internal needs (user comes first). User installed event handlers always have priority to make the user feel in control. The Control also provides for another new infrastructure, the managing of key mappings. The mapping system can respond directly to Key events (after the user had their chance) to generate a semantic event. This means that both Control and Skin are allowed to generate semantic events, although for Control this is strictly limited to the mapping system. The key mappings are only overrideable, and their base configuration is provided by whatever Behavior is installed. Exchanging the Behavior does not undo user key mapping overrides, but instead provides a new base line upon which the overrides are applied. So if a Behavior provides a mapping for SPACE, and the user removed it, installing a different behavior that also provides a mapping for SPACE will still see that mapping as removed. If a behavior doesn't define SPACE, and the user removed it, then nothing special happens (but it is remembered). - Controls refer to a Skin and Behavior - Controls define semantic events - Controls can generate semantic events (via mappings) - Controls never modify their own (user writable) state on their own accord (the user is in full control) - Controls provide an override based key mapping system # Skins Skins provide the visuals, and although they get a Control reference, they are restricted to only adding property listeners (not event handlers) and modifying the children list (which is read only for users as Control extends from Region). This keeps the user fully in control when it comes to any writable properties and events on Control. Most Skins already do this as I think it was an unwritten rule from the beginning. Skins then install event handlers on their children (but never the top level Control) where translation takes place to semantic events. Skins have no reference to the Behavior to ensure that all communication has to go through (interceptable) semantic events. Not all events a Skin receives must be translated; if some events only result in the Skin's internal state changing, and does not need to be reflected in the Control's state then Skins can handle these directly without going through a Behavior. Examples might be the position of the caret, or the exact scro
Re: RFR: 8301302: Platform preferences API [v23]
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 +1 for ColorScheme - PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1811380055
Re: RFR: 8319669: [macos14] Running any JavaFX app prints Secure coding warning [v2]
On Wed, 8 Nov 2023 19:43:24 GMT, Kevin Rushforth wrote: >> Fix [JDK-8319669](https://bugs.openjdk.org/browse/JDK-8319669) as follows: >> >> 1. Override the `NSApplicationDelegate` method >> `applicationSupportsSecureRestorableState` in `GlassApplication` and return >> `YES`. This silences the warning that FX applications now get on macOS 14. >> 2. Create and initialize an `NSApplicationFX` subclass of `NSApplication` >> with no additional functionality. This stops AWT from overwriting the >> NSApplicationDelegate (`GlassApplication`) that JavaFX sets during toolkit >> initialization in the case where the AWT toolkit is used from a JavaFX >> Application (e.g., when using SwingNode or other Swing functionality), and >> is necessary in order to safely do the above. It also fixes other problems >> that can result from the delegate being overwritten. >> >> As noted in the bug report, this PR solves the following problems: >> >> * Eliminates the "Secure coding is not enabled for restorable state" warning >> on macOS 14 >> * The assertion error reported in >> [JDK-8318129](https://bugs.openjdk.org/browse/JDK-8318129) >> >> * The FX application stops getting messages when the application is hidden, >> deactivated, reactivated, etc. We currently don't do anything with these >> messages once the application is running, but we might do so in the future. >> >> * Probably related to the above, we sometimes get an odd behavior when >> trying to hide an application on macOS 13 using the CMD-H key after the AWT >> Toolkit has been initialized. Instead of hiding the window, it pops up a >> finder window with a folder icon and a label that shows the version of Java. >> >> * If AWT and FX return a different answer from their delegate's >> `applicationSupportsSecureRestorableState` method, it will crash on macOS >> 13.x. >> >> This is the FX equivalent of >> [JDK-8318854](https://bugs.openjdk.org/browse/JDK-8318854) in AWT. > > Kevin Rushforth has updated the pull request incrementally with one > additional commit since the last revision: > > Fix typo: remove redundant `isEmbedded =` assignement Tested it on both x86 and arm based macOS 14 works fine. - Marked as reviewed by kizune (Author). PR Review: https://git.openjdk.org/jfx/pull/1280#pullrequestreview-1730849783
Re: RFR: 8303478: DatePicker throws uncatchable exception on tab out from garbled text [v3]
On Fri, 10 Nov 2023 16:51:22 GMT, brunesto wrote:
>> The fix prevents the DatePicker from losing focus if the date is not
>> parsable.
>
> brunesto has updated the pull request incrementally with one additional
> commit since the last revision:
>
> minor
thank you for adding a test case!
the testing looks good. have a couple of minor suggestions.
modules/javafx.controls/src/main/java/javafx/scene/control/DatePicker.java line
151:
> 149: focusedProperty().addListener(o -> {
> 150: if (!isFocused()) {
> 151: commitValueOnFocusLost();
minor: I don't know if it's worth creating a new method here - try .. catch can
simply be moved here.
modules/javafx.controls/src/main/java/javafx/scene/control/DatePicker.java line
164:
> 162: }
> 163:
> 164:
minor: extra newline
-
PR Review: https://git.openjdk.org/jfx/pull/1274#pullrequestreview-1730840393
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1393322392
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1393318322
Re: RFR: 8274967: KeyCharacterCombinations for punctuation and symbols fail on non-US keyboards
On Tue, 17 Oct 2023 20:21:30 GMT, Martin Fox wrote: > After finding the Window virtual key code for a character getKeyCodeForChar > was using a mapping table that only works correctly for U.S. English to > retrieve the Java key code. This caused getKeyCodeForChar to encode keys > differently than the original key event handling machinery. > > With this fix the Robot, getKeyCodeForChar, and the code that handles > platform key events all agree on how Windows VK codes should map to Java > codes. > > The manual KeyboardTest app can be used to test this (tests/manual/events). > Run the tests by selecting "without keypad combinations" in the second > dropdown. This will use a Robot to test KeyCharacterCombinations excluding > the numeric keypad (which is a separate issue). Looks good! I've tested this PR on Windows 11 with my Spanish keyboard. Without the patch from this PR, the KeyboardTest app (without keypad combinations) gives: [Win] Testing 80 keys on Spanish without keypad combinations Failed: code Delete did not match combination U+007F Failed: code Quote did not match combination ' Failed: code Quote did not match combination ? Failed: code Inverted Exclamation Mark did not match combination ¡ Failed: code Inverted Exclamation Mark did not match combination ¿ Failed: code Plus did not match combination + Failed: code Plus did not match combination * Failed: code Plus did not match combination ] Tested 80 keys with 8 failures with the patch: [Win] Testing 80 keys on Spanish without keypad combinations Tested 80 keys with 0 failures The same goes with US English, German and French (after setting those as local keyboards). - Marked as reviewed by jpereda (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1264#pullrequestreview-1730839151
Re: RFR: 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows [v2]
On Tue, 14 Nov 2023 12:47:51 GMT, Kevin Rushforth wrote: >> This PR updates the compiler on Windows to Visual Studio 2022 17.6.5 (from >> 17.5.0) to match JDK 22. >> >> I've run headless and headful tests. > > Kevin Rushforth has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Merge branch 'master' into 8319762-vs2022-17.6.5 > - 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows Marked as reviewed by sykora (Author). - PR Review: https://git.openjdk.org/jfx/pull/1285#pullrequestreview-1730826931
Re: RFR: 8087700: [KeyCombination, Mac] KeyCharacterCombinations behave erratically
On Tue, 14 Nov 2023 20:26:00 GMT, Martin Fox wrote:
>> It looks like the scope of this change went beyond the original problem in
>> JBS.
>> Would it be possible to update the JBS ticket with the information in this
>> PR description?
>
> @andy-goryachev-oracle You're right, this PR also deals with the problem of
> the map retaining stale entries when the user switches layout. I added
> details about this to the JBS ticket. We could treat it as a separate bug if
> you want, to me it's just the sort of standard housekeeping one does when
> caching information.
>
> I also added a note about the KeyboardTest app. This PR scales back that test
> a bit but this is the first time we've been able to run the
> KeyCharacterCombinations on the Mac so that portion is basically a new test.
>
> Let me know if you want further information in the JBS ticket.
@beldenfox thank you. I still not entirely sure what issue is being tested and
how to fix it, between this PR and JBS.
For example, when I run the application listed in JBS description with your
fix, I get one "HI" - is it expected? (I also get 1 "HI" on Windows)
On macOS, new KeyCodeCombination(KeyCode.MINUS, KeyCombination.CONTROL_DOWN)
does not equal to new KeyCharacterCombination("-", KeyCombination.CONTROL_DOWN)
(same on Windows)
Interestingly, Ctrl+- on macOS with the US keyboard produces
KeyEvent{type=KEY_PRESSED, character=<\u00>, text=, code=CONTROL, control}
KeyEvent{type=KEY_PRESSED, character=<\u00>, text=, code=MINUS, control}
KeyEvent{type=KEY_TYPED, character=<\u1F>, text=, code=UNDEFINED, control}
KeyEvent{type=KEY_RELEASED, character=<\u00>, text=, code=MINUS, control}
KeyEvent{type=KEY_RELEASED, character=<\u00>, text=, code=CONTROL}
but on Windows, we don't get KEY_TYPED event:
-
PR Comment: https://git.openjdk.org/jfx/pull/1209#issuecomment-1811296143
Re: RFR: 8319669: [macos14] Running any JavaFX app prints Secure coding warning [v2]
On Wed, 8 Nov 2023 19:43:24 GMT, Kevin Rushforth wrote: >> Fix [JDK-8319669](https://bugs.openjdk.org/browse/JDK-8319669) as follows: >> >> 1. Override the `NSApplicationDelegate` method >> `applicationSupportsSecureRestorableState` in `GlassApplication` and return >> `YES`. This silences the warning that FX applications now get on macOS 14. >> 2. Create and initialize an `NSApplicationFX` subclass of `NSApplication` >> with no additional functionality. This stops AWT from overwriting the >> NSApplicationDelegate (`GlassApplication`) that JavaFX sets during toolkit >> initialization in the case where the AWT toolkit is used from a JavaFX >> Application (e.g., when using SwingNode or other Swing functionality), and >> is necessary in order to safely do the above. It also fixes other problems >> that can result from the delegate being overwritten. >> >> As noted in the bug report, this PR solves the following problems: >> >> * Eliminates the "Secure coding is not enabled for restorable state" warning >> on macOS 14 >> * The assertion error reported in >> [JDK-8318129](https://bugs.openjdk.org/browse/JDK-8318129) >> >> * The FX application stops getting messages when the application is hidden, >> deactivated, reactivated, etc. We currently don't do anything with these >> messages once the application is running, but we might do so in the future. >> >> * Probably related to the above, we sometimes get an odd behavior when >> trying to hide an application on macOS 13 using the CMD-H key after the AWT >> Toolkit has been initialized. Instead of hiding the window, it pops up a >> finder window with a folder icon and a label that shows the version of Java. >> >> * If AWT and FX return a different answer from their delegate's >> `applicationSupportsSecureRestorableState` method, it will crash on macOS >> 13.x. >> >> This is the FX equivalent of >> [JDK-8318854](https://bugs.openjdk.org/browse/JDK-8318854) in AWT. > > Kevin Rushforth has updated the pull request incrementally with one > additional commit since the last revision: > > Fix typo: remove redundant `isEmbedded =` assignement Marked as reviewed by prr (Reviewer). - PR Review: https://git.openjdk.org/jfx/pull/1280#pullrequestreview-1730781202
[jfx21u] RFR: 8318984: Update to Xcode 14.3.1 on macOS
Clean backport of compiler update to jfx21u - Commit messages: - Backport d24e96a66f1908b5a1a1a7d48ee938ba1c782e6c Changes: https://git.openjdk.org/jfx21u/pull/27/files Webrev: https://webrevs.openjdk.org/?repo=jfx21u&pr=27&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318984 Stats: 5 lines in 3 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jfx21u/pull/27.diff Fetch: git fetch https://git.openjdk.org/jfx21u.git pull/27/head:pull/27 PR: https://git.openjdk.org/jfx21u/pull/27
Re: RFR: 8087700: [KeyCombination, Mac] KeyCharacterCombinations behave erratically
On Tue, 14 Nov 2023 16:52:16 GMT, Andy Goryachev wrote: >> A KeyCharacterCombination should match a key if the target character is >> printed on that key. For example, the user should be able to invoke the >> `Shortcut+'+' ` combination by holding down the Shortcut key and pressing a >> key that has '+' printed on it. This should work even if '+' is a shifted >> symbol but the user doesn't hold down the Shift key. >> >> The Mac implements KeyCharacterCombinations by monitoring keystrokes to >> discover the relationship between keys and characters. Currently the system >> only records the character the user typed and no other characters on the >> same key. This means a shortcut targeting a shifted character may not work >> until the user types that character using Shift so the system learns the >> relationship. >> >> This PR keeps the same mechanism in place but always records the shifted and >> unshifted character for each keystroke. >> >> For the Mac the KeyboardTest app was modified to remove tests for characters >> accessed using Option. We don't look for these characters because under the >> hood just about every key has some symbol assigned to the Option modifier >> that the user probably isn't even aware of. For these character we fall back >> to the existing logic; once the user types the character it will start >> working as a shortcut. > > It looks like the scope of this change went beyond the original problem in > JBS. > Would it be possible to update the JBS ticket with the information in this PR > description? @andy-goryachev-oracle You're right, this PR also deals with the problem of the map retaining stale entries when the user switches layout. I added details about this to the JBS ticket. We could treat it as a separate bug if you want, to me it's just the sort of standard housekeeping one does when caching information. I also added a note about the KeyboardTest app. This PR scales back that test a bit but this is the first time we've been able to run the KeyCharacterCombinations on the Mac so that portion is basically a new test. Let me know if you want further information in the JBS ticket. - PR Comment: https://git.openjdk.org/jfx/pull/1209#issuecomment-1811194141
Re: RFR: 8319996: Update to GCC 13.2.0 on Linux
On Tue, 14 Nov 2023 00:24:30 GMT, Kevin Rushforth wrote: > This PR updates the compiler on Linux to GCC 13.2.0 (from 12.2.0) to match > JDK 22. > > I've run headless and headful tests on Ubuntu 16.04, 20.04, and 22.04. Reviewers: @arapte @johanvos @tiainen - PR Comment: https://git.openjdk.org/jfx/pull/1286#issuecomment-1810741902
RFR: 8319996: Update to GCC 13.2.0 on Linux
This PR updates the compiler on Linux to GCC 13.2.0 (from 12.2.0) to match JDK 22. I've run headless and headful tests on Ubuntu 16.04, 20.04, and 22.04. - Commit messages: - 8319996: Update to GCC 13.2.0 on Linux Changes: https://git.openjdk.org/jfx/pull/1286/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1286&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8319996 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jfx/pull/1286.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1286/head:pull/1286 PR: https://git.openjdk.org/jfx/pull/1286
Re: RFR: 8319341: [Linux] Remove operation to show or hide children because it is unnecessary
On Thu, 2 Nov 2023 19:49:52 GMT, Thiago Milczarek Sayao wrote: > Observed that the window manager takes care of showing and hiding children > because we set `gtk_window_set_transient_for`. > > Works since Ubuntu 16.04. > > This PR removes the code to show or hide children because it "duplicates" the > operation and might lead to unknown bugs. Looks good. I tested it on Ubuntu 16.04 and 22.04. - Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1279#pullrequestreview-1730279277
Re: RFR: 8087700: [KeyCombination, Mac] KeyCharacterCombinations behave erratically
On Mon, 14 Aug 2023 16:28:20 GMT, Martin Fox wrote: > A KeyCharacterCombination should match a key if the target character is > printed on that key. For example, the user should be able to invoke the > `Shortcut+'+' ` combination by holding down the Shortcut key and pressing a > key that has '+' printed on it. This should work even if '+' is a shifted > symbol but the user doesn't hold down the Shift key. > > The Mac implements KeyCharacterCombinations by monitoring keystrokes to > discover the relationship between keys and characters. Currently the system > only records the character the user typed and no other characters on the same > key. This means a shortcut targeting a shifted character may not work until > the user types that character using Shift so the system learns the > relationship. > > This PR keeps the same mechanism in place but always records the shifted and > unshifted character for each keystroke. > > For the Mac the KeyboardTest app was modified to remove tests for characters > accessed using Option. We don't look for these characters because under the > hood just about every key has some symbol assigned to the Option modifier > that the user probably isn't even aware of. For these character we fall back > to the existing logic; once the user types the character it will start > working as a shortcut. It looks like the scope of this change went beyond the original problem in JBS. Would it be possible to update the JBS ticket with the information in this PR description? - PR Comment: https://git.openjdk.org/jfx/pull/1209#issuecomment-1810682913
Re: RFR: JDK-8269921 TextFlow: listeners on bounds can throw NPE while computing text bounds [v6]
On Mon, 30 Oct 2023 15:03:05 GMT, Florian Kirmaier wrote: >> It's "a bit" complicated. >> In some situations, getRuns get's called because listeners on bounds are set. >> This causes TextFlow to layout to compute the runs. >> Afterward, the bounds of the parents get updated. >> This triggers a call to compute bounds - which cascades up to the children. >> When the geometry of the previous Text gets computed in this big stack - it >> throws an nullpointer. >> The Text doesn't have its runs, and calling TextFlow.layout is now a noop >> (it detects repeated calls in the same stack) >> >> In the case it happened - it didn't repair and the application kinda crashed. >> This bug most likely can also be triggered by ScenicView or similar tools, >> which sets listeners to the bounds. >> It also can cause unpredictable performance issues. >> >> Unit test and example stacktrace are in the ticket. >> >> The suggested fix makes sure that recomputing the geometry of the Text, >> doesn't trigger the layout of the TextFlow. >> The Textflow should be layouting by the Parent. >> This might change the behavior in some cases, but as far as I've tested it >> works without issues in TextFlow Heavy applications. >> >> Benefits: >> * Better Tooling Support For ScenicView etc. >> * Fixes complicated but reproducible crashes >> * Might fix some rare crashes, which are hard to reproduce >> * Likely improves performance - might fix some edge cases with >> unpredictable bad performance > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > updated test to junit5 > improved name for method onEveryNode -> addBoundsListener As long as subsequent layout cycles fix the bounds (and apps like ScenicView show them correctly), I am ok with this fix. - Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/564#pullrequestreview-1730205022
Re: RFR: JDK-8269921 TextFlow: listeners on bounds can throw NPE while computing text bounds [v6]
On Mon, 30 Oct 2023 15:03:05 GMT, Florian Kirmaier wrote: >> It's "a bit" complicated. >> In some situations, getRuns get's called because listeners on bounds are set. >> This causes TextFlow to layout to compute the runs. >> Afterward, the bounds of the parents get updated. >> This triggers a call to compute bounds - which cascades up to the children. >> When the geometry of the previous Text gets computed in this big stack - it >> throws an nullpointer. >> The Text doesn't have its runs, and calling TextFlow.layout is now a noop >> (it detects repeated calls in the same stack) >> >> In the case it happened - it didn't repair and the application kinda crashed. >> This bug most likely can also be triggered by ScenicView or similar tools, >> which sets listeners to the bounds. >> It also can cause unpredictable performance issues. >> >> Unit test and example stacktrace are in the ticket. >> >> The suggested fix makes sure that recomputing the geometry of the Text, >> doesn't trigger the layout of the TextFlow. >> The Textflow should be layouting by the Parent. >> This might change the behavior in some cases, but as far as I've tested it >> works without issues in TextFlow Heavy applications. >> >> Benefits: >> * Better Tooling Support For ScenicView etc. >> * Fixes complicated but reproducible crashes >> * Might fix some rare crashes, which are hard to reproduce >> * Likely improves performance - might fix some edge cases with >> unpredictable bad performance > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > updated test to junit5 > improved name for method onEveryNode -> addBoundsListener Marked as reviewed by kcr (Lead). - PR Review: https://git.openjdk.org/jfx/pull/564#pullrequestreview-1730179439
Re: RFR: 8319341: [Linux] Remove operation to show or hide children because it is unnecessary
On Thu, 2 Nov 2023 19:49:52 GMT, Thiago Milczarek Sayao wrote: > Observed that the window manager takes care of showing and hiding children > because we set `gtk_window_set_transient_for`. > > Works since Ubuntu 16.04. > > This PR removes the code to show or hide children because it "duplicates" the > operation and might lead to unknown bugs. @lukostyra Can you be the second reviewer? - PR Comment: https://git.openjdk.org/jfx/pull/1279#issuecomment-1810583263
Re: RFR: 8274967: KeyCharacterCombinations for punctuation and symbols fail on non-US keyboards
On Tue, 17 Oct 2023 20:21:30 GMT, Martin Fox wrote: > After finding the Window virtual key code for a character getKeyCodeForChar > was using a mapping table that only works correctly for U.S. English to > retrieve the Java key code. This caused getKeyCodeForChar to encode keys > differently than the original key event handling machinery. > > With this fix the Robot, getKeyCodeForChar, and the code that handles > platform key events all agree on how Windows VK codes should map to Java > codes. > > The manual KeyboardTest app can be used to test this (tests/manual/events). > Run the tests by selecting "without keypad combinations" in the second > dropdown. This will use a Robot to test KeyCharacterCombinations excluding > the numeric keypad (which is a separate issue). @jperedadnr Would you be able to be the second reviewer? - PR Comment: https://git.openjdk.org/jfx/pull/1264#issuecomment-1810497971
Re: RFR: 8087700: [KeyCombination, Mac] KeyCharacterCombinations behave erratically
On Mon, 14 Aug 2023 16:28:20 GMT, Martin Fox wrote: > A KeyCharacterCombination should match a key if the target character is > printed on that key. For example, the user should be able to invoke the > `Shortcut+'+' ` combination by holding down the Shortcut key and pressing a > key that has '+' printed on it. This should work even if '+' is a shifted > symbol but the user doesn't hold down the Shift key. > > The Mac implements KeyCharacterCombinations by monitoring keystrokes to > discover the relationship between keys and characters. Currently the system > only records the character the user typed and no other characters on the same > key. This means a shortcut targeting a shifted character may not work until > the user types that character using Shift so the system learns the > relationship. > > This PR keeps the same mechanism in place but always records the shifted and > unshifted character for each keystroke. > > For the Mac the KeyboardTest app was modified to remove tests for characters > accessed using Option. We don't look for these characters because under the > hood just about every key has some symbol assigned to the Option modifier > that the user probably isn't even aware of. For these character we fall back > to the existing logic; once the user types the character it will start > working as a shortcut. @andy-goryachev-oracle Can you be the primary reviewer on this? - PR Comment: https://git.openjdk.org/jfx/pull/1209#issuecomment-1810498692
Re: RFR: 8319779: SystemMenu: memory leak due to listener never being removed
On Fri, 10 Nov 2023 10:34:08 GMT, Johan Vos wrote: > A listener was added but never removed. > This patch removes the listener when the menu it links to is cleared. Fix for > https://bugs.openjdk.org/browse/JDK-8319779 @arapte can you be the primary reviewer for this? - PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1810497039
Re: RFR: 8092272: [D3D 3D] Need a robust 3D states management for texture [v2]
On Fri, 10 Nov 2023 23:39:21 GMT, Nir Lisker wrote: >> Moves the filter setting of the samplers from the device parameters >> configuration to the use-site, allowing for dynamic changes in the sampler. >> This PR does internal plumbing work only to bring it close to the ES2 >> pipeline. A followup PR will create the public API. >> >> Summary of the changes: >> * Created a new (internal for now) `TextureData` object that is intended to >> contain all the data of texture (map) of `PhongMaterial`, such as filters, >> addressing, wrapping mode, mipmaps etc. **This PR deals only with filters** >> as a starting point, more settings can be added later. >> * Creates an update mechanism from the Java side material to the native D3D >> layer. The public API `PhoneMaterial` is *not* changed yet. The peer >> `NGPhongMaterial` is configured to receive update from the public >> `PhongMaterial` when the public API is created via new >> `ObjectProperty` properties. >> * Small refactoring in the D3D layer with a new map types enum to control >> the texture settings more easily. >> >> The JBS issue lists some regressions in a comment, but I couldn't reproduce >> them. It looks like the sampler settings needed to be added anywhere, and >> that was the easiest to do at the time. Now they were just moved. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments Reviewers: @arapte @lukostyra - PR Comment: https://git.openjdk.org/jfx/pull/1281#issuecomment-1810480847
Re: RFR: 8311895: CSS Transitions [v9]
On Tue, 31 Oct 2023 17:24:05 GMT, Michael Strauß wrote:
>> Implementation of [CSS
>> Transitions](https://gist.github.com/mstr2/c72f8c9faa87de14926978f517a6018a).
>>
>> ### Example
>>
>> .button {
>> -fx-background-color: dodgerblue;
>> }
>>
>> .button:hover {
>> -fx-background-color: red;
>> -fx-scale-x: 1.1;
>> -fx-scale-y: 1.1;
>>
>> transition: -fx-background-color 0.5s ease,
>> -fx-scale-x 0.5s ease,
>> -fx-scale-y 0.5s ease;
>> }
>>
>> > src="https://user-images.githubusercontent.com/43553916/184781143-0520fbfe-54bf-4b8d-93ac-834708e46500.gif";
>> width="200"/>
>>
>> ### Limitations
>> This implementation supports both shorthand and longhand notations for the
>> `transition` property. However, due to limitations of JavaFX CSS, mixing
>> both notations doesn't work:
>>
>> .button {
>> transition: -fx-background-color 1s;
>> transition-easing-function: linear;
>> }
>>
>> This issue should be addressed in a follow-up enhancement.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Test whether two Interpolatable instances are compatible
modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionTimer.java
line 179:
> 177: if (newTimer.delay < 0) {
> 178: double adjustedDelay = nanosToMillis(newTimer.delay) *
> newTimer.reversingShorteningFactor;
> 179: newTimer.startTime = now + millisToNanos(adjustedDelay);
I don't see the value of converting these back and forth to milliseconds. Why
not just:
Suggestion:
newTimer.startTime = now + newTimer.delay *
newTimer.reversingShorteningFactor;
I checked the calculations, and it makes no difference (the millis aren't
rounded or anything), so you're just moving the decimal point before/after
doing a multiplication that doesn't care where the decimal point is.
If there is a reason that I missed, then I think this really needs a comment.
modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionTimer.java
line 196:
> 194: double frac = (double)(nanos - (millis * 1_000_000L)) /
> 1_000_000D;
> 195: return (double)millis + frac;
> 196: }
This function seems to want to preserve extra decimals in some way. If the
original long has a magnitude that is so large that some least significant
digits get lost in the double conversion, then trying to add them later
(`millis + frac`) will not restore them.
In other words, you can just do:
Suggestion:
private static double nanosToMillis(long nanos) {
return nanos / 1_000_000.0;
}
Or avoiding the (generally slower) division completely:
Suggestion:
private static double nanosToMillis(long nanos) {
return nanos * 0.000_001;
}
All the extra operations are only likely to introduce small errors.
modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionTimer.java
line 207:
> 205: long wholeMillis = (long)millis;
> 206: double frac = millis - (double)wholeMillis;
> 207: return wholeMillis * 1_000_000L + (long)(frac * 1_000_000D);
I'd recommend keeping this simple (it seems to want to recover extra
significant digits, but that's not possible):
Suggestion:
return ((long)(millis * 1_000_000.0);
modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionTimer.java
line 234:
> 232:
> 233: Node node = (Node)timer.getProperty().getBean();
> 234: node.fireEvent(new TransitionEvent(eventType,
> timer.getProperty(), elapsedTime));
minor:
Suggestion:
long elapsedTime;
// Elapsed time specification:
https://www.w3.org/TR/css-transitions-1/#event-transitionevent
if (eventType == TransitionEvent.RUN || eventType ==
TransitionEvent.START) {
elapsedTime = Math.min(Math.max(-timer.delay, 0),
timer.duration);
} else if (eventType == TransitionEvent.CANCEL) {
elapsedTime = Math.max(0, timer.currentTime - timer.startTime);
} else if (eventType == TransitionEvent.END) {
elapsedTime = timer.duration;
} else {
throw new IllegalArgumentException("eventType");
}
Node node = (Node)timer.getProperty().getBean();
node.fireEvent(new TransitionEvent(eventType, timer.getProperty(),
Duration.millis(nanosToMillis(elapsedTime;
modules/javafx.graphics/src/main/java/javafx/css/StyleableDoubleProperty.java
line 78:
> 76:
> 77: if (transition != null) {
> 78: timer = TransitionTimer.run(new TransitionTimerImpl(this, v),
> transition);
Would it be possible to check here if this timer is going to the same target?
Also see other comment about a simplification.
if (timer == null || timer.newValue.equals(v))) {
timer = TransitionTimer.run(new TransitionTimerImpl(this, v),
transition);
Re: RFR: 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows [v2]
> This PR updates the compiler on Windows to Visual Studio 2022 17.6.5 (from > 17.5.0) to match JDK 22. > > I've run headless and headful tests. Kevin Rushforth has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into 8319762-vs2022-17.6.5 - 8319762: Update to Visual Studio 2022 version 17.6.5 on Windows - Changes: https://git.openjdk.org/jfx/pull/1285/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1285&range=01 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jfx/pull/1285.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1285/head:pull/1285 PR: https://git.openjdk.org/jfx/pull/1285
Integrated: 8318984: Update to Xcode 14.3.1 on macOS
On Mon, 13 Nov 2023 18:45:13 GMT, Kevin Rushforth wrote: > This PR updates the micro version of the compiler on macOS to 14.3.1 (from > 14.3) to match JDK 22. > > I've run headless and headful tests. This pull request has now been integrated. Changeset: d24e96a6 Author:Kevin Rushforth URL: https://git.openjdk.org/jfx/commit/d24e96a66f1908b5a1a1a7d48ee938ba1c782e6c Stats: 5 lines in 3 files changed: 0 ins; 0 del; 5 mod 8318984: Update to Xcode 14.3.1 on macOS Reviewed-by: arapte, sykora - PR: https://git.openjdk.org/jfx/pull/1284
Re: RFR: 8318984: Update to Xcode 14.3.1 on macOS
On Mon, 13 Nov 2023 18:45:13 GMT, Kevin Rushforth wrote: > This PR updates the micro version of the compiler on macOS to 14.3.1 (from > 14.3) to match JDK 22. > > I've run headless and headful tests. Marked as reviewed by sykora (Author). - PR Review: https://git.openjdk.org/jfx/pull/1284#pullrequestreview-1729599986
RFR: 8242616: Regression: RTL - TextField Cursor Movement Via Keyboard
The change listener on caretPositionProperty() was not getting invoked on replacing the content with same text as there was no change in caret position. Since the textProperty invalidation sets the forward bias to true by default, incorrect caret position was calculated when the same text was replaced after clicking on the trailing edge of last character or empty space in the TextField. Since caretShapeProperty invalidation listener gets invoked without changing the caret position, updating the caretBiasProperty on this listener solves the issue. Since the caret position value will be same when the caret is present after the last character or before the last character, it can not be validated using unit test. The fix can be validated using MonkeyTester. Steps to select TextField option in Monkey Tester. - Open the MonkeyTester app and select TextField from the left option pane. - Select Short from Text selection option and click on the TextField to bring it into focus. - Select all using cmd(ctrl) + a and copy using cmd(ctrl) + c - Click on empty space in the TextField after the present content. - Select all again using the keyboard shortcut and paste using cmd(ctrl) + v - The caret should be displayed after the last character. Without the fix caret gets displayed before the last character. - Commit messages: - Textfield cursor issue fix Changes: https://git.openjdk.org/jfx/pull/1287/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1287&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8242616 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jfx/pull/1287.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1287/head:pull/1287 PR: https://git.openjdk.org/jfx/pull/1287
