Re: RFR: 8330559: Trailing space not rendering correctly in TextFlow in RTL mode [v2]

2024-06-06 Thread John Hendrikx
On Thu, 6 Jun 2024 15:35:07 GMT, Andy Goryachev wrote: >> CTGlyphLayout is not common code. It is mac only (so no need to mention mac) > > I see, PrismFontFactory:164 getNativeFactoryName(). > It would be nice to place platform-specific code in a package bearing the > platform name, or at least

Re: RFR: 8330559: Trailing space not rendering correctly in TextFlow in RTL mode

2024-06-04 Thread John Hendrikx
On Tue, 4 Jun 2024 05:32:23 GMT, Karthik P K wrote: >> I think if this is specific to mac, and doesn't occur on other platforms >> that the supplied `positions` may be incorrect by the underlying >> `*GlyphLayout` code. Under Windows it will likely use the `DWGlyphLayout` >> to supply

Re: RFR: 8324232: KeyEvent.getCode() is null inside JFXPanel

2024-06-01 Thread John Hendrikx
On Sat, 1 Jun 2024 18:15:56 GMT, Martin Fox wrote: > JavaFX uses a table to map from an AWT integer key code to a JavaFX KeyCode. > If AWT provides a code that's not in the table JavaFX attempts to use a > KeyCode of null when constructing the KeyEvent which raises an exception. > This PR

Re: RFR: 8330559: Trailing space not rendering correctly in TextFlow in RTL mode

2024-05-31 Thread John Hendrikx
On Fri, 31 May 2024 16:54:58 GMT, Andy Goryachev wrote: >> The issue is specific to Mac. The glyph positions returned from native side >> for complex text is not handled in the text render logic. This issue is >> observed only when trailing spaces are present along with LTR text mixed >> with

Integrated: 8322964: Optimize performance of CSS selector matching

2024-05-30 Thread John Hendrikx
On Thu, 4 Jan 2024 12:21:15 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 common

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

2024-05-28 Thread John Hendrikx
On Tue, 28 May 2024 21:37:43 GMT, Andy Goryachev wrote: >> modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/FixedCapacitySetTest.java >> line 1: >> >>> 1: package test.com.sun.javafx.css; >> >> The Copyright is missing from this class and the `if` and `for` clauses have >> no

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

2024-05-28 Thread John Hendrikx
k 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: Add copyright header - Changes: - all: https://git.openjdk.org/jfx/pul

Re: RFR: 8087444: CornerRadii with different horizontal and vertical values treated as uniform

2024-05-28 Thread John Hendrikx
On Tue, 28 May 2024 16:15:25 GMT, Michael Strauß wrote: > This PR fixes the incorrect computation of the `CornerRadii.uniform` flag for > the 16-argument constructor. > > I've also added tests for all other constructors. LGTM

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

2024-05-28 Thread John Hendrikx
On Tue, 28 May 2024 09:43:28 GMT, Michael Strauß wrote: >> John Hendrikx has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 21 commits: >> >> - Merge branch 'openjdk:master' into >> feature/se

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

2024-05-27 Thread John Hendrikx
On Sat, 25 May 2024 13:50:56 GMT, Kevin Rushforth wrote: >> The code looks good. I didn't test it, but I'm fine with integrating. > >> Also some clarification on the contributing rules: "all Reviewers who have >> requested the chance to review have done so" -- does the indication at the >> top

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

2024-05-27 Thread John Hendrikx
k 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 with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Merge branch 'openjdk:master' into feature/selector-perfor

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

2024-05-27 Thread John Hendrikx
k 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 with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Merge remote-tracking branch 'upstream/master' i

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

2024-05-27 Thread John Hendrikx
k 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: Added 100% coverage test for FixedCapacitySet - Changes: - all: htt

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

2024-05-24 Thread John Hendrikx
On Fri, 24 May 2024 12:12:27 GMT, Kevin Rushforth wrote: > > > I wonder if we may want to add some tests for the `FixedCapacitySet`? > > > > > > Yeah, now that it is more likely that this will make it into FX, I will add > > a small set of unit tests for this class. > > Since this PR is

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

2024-05-24 Thread John Hendrikx
On Thu, 23 May 2024 22:54:15 GMT, Marius Hanl wrote: > Code looks good to me. As mentioned above, I tested it already and everything > was good, so I'm certain that there is no regression here. > > I'm generally not a big fan of 'reimplementing' Collections, but I can see > why it is needed

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

2024-05-24 Thread John Hendrikx
On Thu, 23 May 2024 22:44:08 GMT, Marius Hanl wrote: >> 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 > > modules/javafx

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

2024-05-22 Thread John Hendrikx
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

Re: RFR: 8322619: Parts of SG no longer update during rendering - overlapping - culling - dirty

2024-05-17 Thread John Hendrikx
On Mon, 6 May 2024 14:14:05 GMT, eduardsdv wrote: > This is an alternative solution to the PR: > https://github.com/openjdk/jfx/pull/1310. > > This solution is based on the invariant that if a node is marked as dirty, > all ancestors must also be marked as dirty and that if an ancestor is

Re: RFR: 8332313: Update code review guidelines

2024-05-16 Thread John Hendrikx
On Thu, 16 May 2024 07:30:31 GMT, Johan Vos wrote: >> Yeah, that was a typo (which I didn't notice when copying the block from the >> other doc). I'll fix it. And I agree with your concern, so I'll remove the >> last sentence. > > I agree with the concern, but I still think it's much better to

Re: Proposal: Public InputMap (v2)

2024-05-15 Thread John Hendrikx
I can only second this. If anything, the proposal highlights several fundamental problems in current JavaFX by how it is trying to work around them. The work around works for one area (key events) but does not address similar problems for other events. The work around will also further cement

Re: RFR: 8332313: Update code review guidelines

2024-05-15 Thread John Hendrikx
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 >

Re: RFR: 8332313: Update code review guidelines

2024-05-15 Thread John Hendrikx
On Wed, 15 May 2024 19:53:43 GMT, Andy Goryachev wrote: >> README-code-reviews.md line 68: >> >>> 66: * Consider any compatibility concerns >>> 67: * Check whether there is an automated test; if not, ask for one, if it >>> is feasible >>> 68: * Make sure that the PR has executed the GHA tests

Integrated: 8331616: ChangeListener is not triggered when the InvalidationListener is removed

2024-05-07 Thread John Hendrikx
On Fri, 3 May 2024 14:45:24 GMT, John Hendrikx wrote: > This PR provides a fix for the linked issue. > > The issue was that when an invalidation listener is removed, and the > `ExpressionHelper` type changes from `Generic` to `SingleChange` that it > would copy th

RFR: 8331616: ChangeListener is not triggered when the InvalidationListener is removed

2024-05-03 Thread John Hendrikx
This PR provides a fix for the linked issue. The issue was that when an invalidation listener is removed, and the `ExpressionHelper` type changes from `Generic` to `SingleChange` that it would copy the current value of the `Generic` instance before it was updated (this is because invalidation

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

2024-05-03 Thread John Hendrikx
erhead|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 &g

Integrated: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text

2024-04-30 Thread John Hendrikx
On Sun, 10 Sep 2023 20:50:18 GMT, John Hendrikx wrote: > There are a number of tickets open related to text rendering: > > https://bugs.openjdk.org/browse/JDK-8314215 > > https://bugs.openjdk.org/browse/JDK-8145496 > > https://bugs.openjdk.org/browse/JDK-8129014 >

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v18]

2024-04-26 Thread John Hendrikx
On Wed, 24 Apr 2024 15:23:39 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update copyright years > > One meta comment (not directly related to this re

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v18]

2024-04-23 Thread John Hendrikx
B CCC`, and the text gets wrapped to > `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not show up. If you > cursor back, the cursor may be outside the control bounds because so many > spaces are trailing `AAA`. > > The above behavior has NOT changed, is pretty standard

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v17]

2024-04-23 Thread John Hendrikx
On Tue, 23 Apr 2024 17:40:21 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix mac test values > > tests/system/src/test/java/test/com/sun/javafx/text/T

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v17]

2024-04-23 Thread John Hendrikx
On Tue, 23 Apr 2024 13:41:55 GMT, John Hendrikx wrote: >> There are a number of tickets open related to text rendering: >> >> https://bugs.openjdk.org/browse/JDK-8314215 >> >> https://bugs.openjdk.org/browse/JDK-8145496 >> >> https://bugs.openj

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v17]

2024-04-23 Thread John Hendrikx
On Tue, 23 Apr 2024 17:30:39 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix mac test values > > tests/system/src/test/java/test/com/sun/javafx/text/T

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v15]

2024-04-23 Thread John Hendrikx
On Tue, 23 Apr 2024 12:40:57 GMT, Karthik P K wrote: > Here is the result of TextLayoutTest in MacOS M1 system. Thanks very much, I've updated the values now, so no tests should fail anymore on Mac. > Following are the skipped tests: > > ``` > TextLayoutTest > caseTest(Case) > [10]

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v17]

2024-04-23 Thread John Hendrikx
B CCC`, and the text gets wrapped to > `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not show up. If you > cursor back, the cursor may be outside the control bounds because so many > spaces are trailing `AAA`. > > The above behavior has NOT changed, is pretty standard

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v15]

2024-04-22 Thread John Hendrikx
On Mon, 22 Apr 2024 16:52:16 GMT, Andy Goryachev wrote: > 2. resolve the problems with the fonts (could we pick the font(s) that are > available via some utility method?) I already left a comment on this before, see here: https://github.com/openjdk/jfx/pull/1236#issuecomment-1939200760 The

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v16]

2024-04-22 Thread John Hendrikx
B CCC`, and the text gets wrapped to > `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not show up. If you > cursor back, the cursor may be outside the control bounds because so many > spaces are trailing `AAA`. > > The above behavior has NOT changed, is pretty standard for

Re: RFR: 8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v15]

2024-04-22 Thread John Hendrikx
On Mon, 12 Feb 2024 10:00:57 GMT, Karthik P K wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change test to use the more common Arial font and add assumptions > > I ran the tests in

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

2024-04-21 Thread John Hendrikx
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 (mo

Re: Possible leak on setOnAction

2024-04-20 Thread John Hendrikx
FontIcon()); items.add(item); } } for (MenuItem item : btnWindows.getItems()) { item.setOnAction(null); } btnWindows.getItems().setAll(items); } Maybe there's a bug, because the old list of items is collectable. Em sex., 19 de abr. de 2024 às 01:37, John Hendrikx escreveu:

Re: Possible leak on setOnAction

2024-04-18 Thread John Hendrikx
This probably is a common mistake, however the Weak wrapper is also easy to use wrongly.  You can't just wrap it like you are doing in your example, because this is how the references look: menuItem ---> WeakEventHandler ---weakly---> Lambda In effect, the Lambda is weakly referenced,

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

2024-03-25 Thread John Hendrikx
On Mon, 25 Mar 2024 13:32:11 GMT, Michael Strauß wrote: > `ListenerManager` is an obvious improvement, as it fixes incorrect behavior > and allows listeners to veto changes. However, the behavior of > `ListenerManager` is also an implementation detail and not documented > anywhere. This leads

Re: Skin ListenerHelper

2024-03-12 Thread John Hendrikx
With subscriptions these kinds of helpers are really not needed anymore. Just do: Subscription combined = Subscription.combine( property1.subscribe( ... ), property2.subscribe( ... ) ); And clean it up with `combined.unsubscribe` in `dispose`. For non-property

Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v15]

2024-03-11 Thread John Hendrikx
On Sat, 10 Feb 2024 17:24:17 GMT, John Hendrikx wrote: >> There are a number of tickets open related to text rendering: >> >> https://bugs.openjdk.org/browse/JDK-8314215 >> >> https://bugs.openjdk.org/browse/JDK-8145496 >> >> https://bugs.openj

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

2024-03-11 Thread John Hendrikx
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 (mo

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

2024-03-11 Thread John Hendrikx
On Mon, 11 Mar 2024 16:47:14 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 (mo

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

2024-03-11 Thread John Hendrikx
k 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 - C

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

2024-03-11 Thread John Hendrikx
k 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: Remove deprecations - Changes: - all: https://git.openjdk.org/jfx/pul

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

2024-03-11 Thread John Hendrikx
On Mon, 11 Mar 2024 14:54:45 GMT, Andy Goryachev wrote: >> I'm not against adding an explanation, but it's not needed as this is part >> of the `Set` contract (sets that can't be modified are specified to throw >> `UnsupportedOperationException`), for example for `add` in `Set`: >> >> *

Re: RFR: JDK-8322795 CSS performance regression up to 10x

2024-03-09 Thread John Hendrikx
On Wed, 3 Jan 2024 18:33:29 GMT, Kevin Rushforth wrote: >> The regression is caused by the `Collections.unmodifiableSet` wrapper not >> being recognized by `BitSet`, and a fall back is done to a less optimized >> version of `containsAll`. >> >> As this is a regression fix, I've kept the fix

Re: Validation Support

2024-03-09 Thread John Hendrikx
On 08/03/2024 07:37, Robert Lichtenberger wrote: One major pain point that we have at the moment is CSS performance, I think (but did not investigate yet) that some kind of performance regression happened somewhere between 17 and 21. I think this was addressed already, see here:

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

2024-03-09 Thread John Hendrikx
On Sat, 9 Mar 2024 12:35:22 GMT, Nir Lisker wrote: >> 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 > &

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

2024-03-09 Thread John Hendrikx
k 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: Add @Deprecated tag - Changes: - all: https://git.openjdk.org/jfx/pul

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

2024-03-08 Thread John Hendrikx
k 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 ---

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

2024-03-08 Thread John Hendrikx
On Sat, 9 Mar 2024 00:04:04 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Optimize performance of OpenAddressed Set just in case it is ever used > > modules

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

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 23:46:16 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Optimize performance of OpenAddressed Set just in case it is ever used > > modules/jav

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

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 23:26:05 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Optimize performance of OpenAddressed Set just in case it is ever used > > modules/jav

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

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 23:17:28 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Optimize performance of OpenAddressed Set just in case it is ever used > > modules/jav

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

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 23:14:12 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Optimize performance of OpenAddressed Set just in case it is ever used > > modules/jav

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

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 18:08:15 GMT, Marius Hanl wrote: > Tested with a big application, did not find any regression. All listeners > still work as expected, tested especially a lot of 'if something is selected, > this button should be enabled/disabled' and the like. I did not checked the > code

Re: Aw: Re: Validation Support

2024-03-08 Thread John Hendrikx
he future (which they will anyway ...). Don't take that decision away from me, it's patronizing. But I guess this is one of those things that depend very much on which side of the equation you are, so don't take my troll bait :-). --Robert Am 03.03.24 um 02:10 schrieb John Hendrikx: > Hi Dir

Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v2]

2024-03-08 Thread John Hendrikx
On Fri, 8 Mar 2024 16:10:26 GMT, Marius Hanl wrote: >> This PR fixes a long standing issue where the `Tooltip` will always wait one >> second until it appears the very first time, even if the >> `-fx-show-delay` was set to another value. >> >> The culprit is, that the `cssForced` flag is not

Re: RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v10]

2024-03-06 Thread John Hendrikx
On Tue, 5 Mar 2024 08:55:41 GMT, Jose Pereda wrote: >> This could be a problem normally, but I think in this case you won't be able >> to get this to produce incorrect results. >> >> `parseText` is accessing the property (and it calls `isMnemonicParsing`), >> but only if the text is not

Re: RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v6]

2024-03-04 Thread John Hendrikx
On Thu, 11 Jan 2024 00:20:19 GMT, John Hendrikx wrote: >> No, the explicit goal of this construction is to set active to false (in >> case it exists) so that existing listeners can be released; followed by >> creating a new active property that is by default set to true u

Re: RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v10]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 19:26:37 GMT, Jose Pereda wrote: >> Johan Vos 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 11 additional commits >> since

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 11:03:26 GMT, Karthik P K wrote: >> @andy-goryachev-oracle The coverage comes from EclEmma, an Eclipse plugin. >> Once installed, there is another way to run tests called `Coverage as...` >> just above `Run as...`. It's very useful to use it on a JUnit test to see >> if

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v17]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 11:01:09 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v17]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 16:15:02 GMT, Andy Goryachev wrote: >> Karthik P K has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add unit test > > modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1044: > >> 1042: private

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v17]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 11:01:09 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v17]

2024-03-04 Thread John Hendrikx
On Mon, 4 Mar 2024 16:12:12 GMT, Andy Goryachev wrote: >> Karthik P K has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add unit test > > modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java > line 108: > >>

Re: Validation Support

2024-03-02 Thread John Hendrikx
Hi Dirk, That is a very nice framework, and although I wouldn't be against its inclusion in FX, I'm more wondering if JavaFX could do more to help make tools like ValidatorFX easy to build. I'm not quite sure how one can use not having validation as an argument against using FX, when there

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-29 Thread John Hendrikx
On Thu, 29 Feb 2024 15:59:58 GMT, Andy Goryachev wrote: >> I disagree on this. The code is complicated and full of branches. Manual >> testing is very error prone. However, since you restored most of the code >> to its original (which wasn't tested either) I could live with it. Still, >>

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v16]

2024-02-29 Thread John Hendrikx
On Thu, 29 Feb 2024 15:49:47 GMT, Andy Goryachev wrote: > > FontHelper.getNativeFont( > > perhaps we could pick a suitable font based on the platform? to make sure we > always get a valid font? The points tested will be specific to the font used, similar to the problems with the #1236 PR.

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-29 Thread John Hendrikx
On Tue, 27 Feb 2024 10:00:41 GMT, Karthik P K wrote: >> I'm not sure if we could write headless test for this. Could you please >> point me to a test which could be helpful for me? > > I believe the tests added in this PR are helpful in making sure that the > HitInfo calculation does not give

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v16]

2024-02-29 Thread John Hendrikx
On Thu, 29 Feb 2024 05:38:05 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

Re: RFR: 8326618 : Replace usage of deprecated method getId() in Thread

2024-02-27 Thread John Hendrikx
On Tue, 27 Feb 2024 17:58:13 GMT, Anirvan Sarkar wrote: > Replace Thread.currentThread().getId() with Thread.currentThread().threadId() Looks okay. `threadId` was added in JDK 19, but since we're on 21 now that should be fine. - Marked as reviewed by jhendrikx (Committer). PR

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v14]

2024-02-27 Thread John Hendrikx
On Tue, 27 Feb 2024 13:51:08 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-26 Thread John Hendrikx
On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

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

2024-02-25 Thread John Hendrikx
k 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: Optimize performance of OpenAddressed Set just in case it is ever used ---

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

2024-02-25 Thread John Hendrikx
k 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 with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Fix docs - Add missing override annotation - Merge

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

2024-02-25 Thread John Hendrikx
On Mon, 15 Jan 2024 20:48:54 GMT, Michael Strauß wrote: >> @mstr2 I've created #1333 to show how it would look when we move >> `SimpleSelector` and `CompoundSelector` to internal packages. I think that >> should alleviate most concerns, and we can either integrate this first with >> a new

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-25 Thread John Hendrikx
On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-24 Thread John Hendrikx
On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

2024-02-24 Thread John Hendrikx
On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation

Re: Proposal: RichTextArea Control (Incubator)

2024-02-22 Thread John Hendrikx
Hi, Far be it from me to tell the FX team what it should do, I am still wondering the following: - A 3rd party control, RichTextFX already exists -- what is this new proposal adding that RichTextFX does not have? - What (if anything) is stopping a 3rd party from building a RichTextArea

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

2024-02-21 Thread John Hendrikx
On Wed, 21 Feb 2024 13:03:46 GMT, Karthik P K wrote: > > To determine the runStart/runEnd, `Text` is already analyzing the runs (and > > doing coordinate tests on them). Runs however have locations, with x/y > > coordinates. Would it not be possible, and more sensible, to adjust the > > `x`,

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

2024-02-19 Thread John Hendrikx
On Mon, 19 Feb 2024 13:45:00 GMT, Karthik P K wrote: > > @karthikpandelu You mentioned there is special casing going on when a > > `Text` is part of a `TextFlow`. What are those cases and where is this > > happening? How does it even know that a `Text` is involved and that it is > > part of a

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

2024-02-19 Thread John Hendrikx
On Thu, 15 Feb 2024 10:36:51 GMT, Karthik P K wrote: >> I think we should simplify the `getHitInfo` method in the `TextLayout` >> interface. >> >> The code currently seems to be making distinctions where there are none. >> `TextFlow`s provide spans, while `Text`s provide a text. `getHitInfo`

Re: Proposal: Bump minimum JDK version for JavaFX 23 to JDK 21

2024-02-17 Thread John Hendrikx
+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

Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Fri, 16 Feb 2024 16:00:07 GMT, Andy Goryachev wrote: > As a side note, before we can start re-writing the controls' behaviors, we > should: > > a) identify all the behavioral aspects of each control and b) develop a set > of tests to exercise each one, along the lines of >

Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Thu, 15 Feb 2024 17:26:39 GMT, Andy Goryachev wrote: > > I think if we're busy here, it would be worthwhile to also remove the other > > calls to the skin to change the caret animating. > > I would rather not: it would require a much larger effort. Avoiding the caret > flickering when its

Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Thu, 15 Feb 2024 17:29:10 GMT, Andy Goryachev wrote: >> Move caret animation handling due to keyboard input to the Skin, by >> registering a listener on the caretPosition property. This will restart >> animation only when the caret position changes instead of every key press. >> >> This

Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves

2024-02-15 Thread John Hendrikx
On Wed, 14 Feb 2024 22:44:07 GMT, Andy Goryachev wrote: > Move caret animation handling due to keyboard input to the Skin, by > registering a listener on the caretPosition property. This will restart > animation only when the caret position changes instead of every key press. > > This also

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

2024-02-15 Thread John Hendrikx
On Sun, 14 Jan 2024 14:32:49 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 (mo

Integrated: JDK-8324182 Deprecate for removal SimpleSelector and CompoundSelector classes

2024-02-15 Thread John Hendrikx
On Fri, 19 Jan 2024 10:02:19 GMT, John Hendrikx wrote: > The SimpleSelector and CompoundSelector classes are public classes in an > exported package, javafx.css, but they are not intended to be used by > applications. They are implementation details. They cannot be constructed &

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

2024-02-15 Thread John Hendrikx
On Thu, 15 Feb 2024 10:36:51 GMT, Karthik P K wrote: > > I think we should simplify the `getHitInfo` method in the `TextLayout` > > interface. > > The code currently seems to be making distinctions where there are none. > > `TextFlow`s provide spans, while `Text`s provide a text. `getHitInfo`

Re: RFR: JDK-8323760 putIfAbsent documentation conflicts with itself [v3]

2024-02-14 Thread John Hendrikx
the replace the value if it > was present. As such, if it returns a value, it is the **current** value, > and in all other cases it will return `null`. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Use new suggestion a

Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

2024-02-13 Thread John Hendrikx
On Fri, 9 Feb 2024 07:59:47 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of

Re: RFR: JDK-8323760 putIfAbsent documentation conflicts with itself [v2]

2024-02-13 Thread John Hendrikx
On Tue, 13 Feb 2024 17:11:05 GMT, John Hendrikx wrote: >> Update the documentation for `@return` tag of `putIfAbsent` to match the >> main description. `putIfAbsent` uses the same wording as `put` for its >> `@return` tag, but that is incorrect. `putIfAbsent` never returns

Re: RFR: JDK-8323760 putIfAbsent documentation conflicts with itself [v2]

2024-02-13 Thread John Hendrikx
On Tue, 16 Jan 2024 13:27:31 GMT, Chen Liang wrote: >> Yeah, I wasn't sure about that, I can make it more specific, I used >> `considered` here because both unmapped keys and keys mapped to `null` are >> considered to be absent. > > I think `absent or {@code null}` is no less concise yet it is

Re: RFR: JDK-8323760 putIfAbsent documentation conflicts with itself [v2]

2024-02-13 Thread John Hendrikx
the replace the value if it > was present. As such, if it returns a value, it is the **current** value, > and in all other cases it will return `null`. John Hendrikx has updated the pull request incrementally with four additional commits since the last revision: - Add code block around

Re: RFR: JDK-8324182 Deprecate for removal SimpleSelector and CompoundSelector classes [v2]

2024-02-13 Thread John Hendrikx
On Sat, 10 Feb 2024 15:13:44 GMT, Kevin Rushforth wrote: >>> > need 2 passes, because there already is a method named like that: >>> >>> oh yeah, sorry. `getStyleClassesSet()` ? >> >> That one is still available yes :) > > @hjohn I'm now leaning towards either `getStyleClassNames` or >

  1   2   3   4   5   6   7   8   9   10   >