Re: Can't restart my app with the latest JDK, regression.
Thank you for sharing this info,
I think it can be very interesting for the JavaFX community since most
of us use JPackage to bundle our JavaFX apps :)
Regards,
Davide
Il 05/02/2024 20:03, Kevin Rushforth ha scritto:
For others who might be curious, Davide filed a bug via
bugreport.java.com and it is now available here:
https://bugs.openjdk.org/browse/JDK-8325203
It is almost certainly a bug in jpackage.
-- Kevin
On 2/5/2024 2:01 AM, Davide Perini wrote:
Hi John,
thanks for the answer.
The problem happen even with other JDKs like the Azul, corretto...
The weird thing is that to reproduce it you need to package the
program with jpackage and then execute the program using the
generated exe.
This is the only code needed to reproduce the issue.
public static void main(String... args) throws IOException {
String[] cmdToRunUsingArgs = {"cmd.exe", "/C", "C:\\Program
Files\\Notepad++\\notepad++.exe"};
Runtime.getRuntime().exec(cmdToRunUsingArgs);
Executors.newSingleThreadScheduledExecutor().schedule(() -> {
System.exit(0);
}, 30, TimeUnit.SECONDS);
}
It's very weird because the problem does not happen with the previous
minor version of the JDK21, the JDK 21.01_12.
I'm pretty sure now that this isn't related to JavaFX since the
problem can be reproduced even without JavaFX.
Problem must be in the JPackage.
Thanks
Davide
Il 04/02/2024 18:17, John Hendrikx ha scritto:
Hi,
Does that Temurin build contain JavaFX, or is it just a new Temurin
build? Did you change anything else besides the JDK used? You
could try other JDK's (openjdk, corretto, etc) as well.
Is there anything being logged that may indicate what went wrong?
That all said, it seems really unlikely this is related to JavaFX.
If System.exit(0) is killing both the new and old application, then
it wasn't really spawned independently, so you can look into that
(you could put a delay there, and see with a tool like `top` or Task
Manager says, and see whether the restarted app is a child process
or not).
It's also possible the problem is in the tools/settings used to
create the ".exe", so see if you changed anything there.
For more info, tips or other ways of doing restarts:
https://stackoverflow.com/questions/4159802/how-can-i-restart-a-java-application
--John
On 04/02/2024 02:49, Davide Perini wrote:
Hi there,
my JavaFX 21.0.2 app experienced a very weird regression when
switching from Temurin
OpenJDK21U-jdk_x64_windows_hotspot_21.0.1_12
to
OpenJDK21U-jdk_x64_windows_hotspot_21.0.2_13
I use this method to restart my app.
public void restart() {
// path to my exe app plus one string argument
String[] cmdToRunUsingArgs = {"C:\Users\sblantipodi\AppData\Local\Firefly Luciferin\Firefly
Luciferin.exe","string argument"};
try {
Runtime.getRuntime().exec(cmdToRunUsingArgs);
}catch (SecurityException | IOException e) {
log.error(e.getMessage());
}
javafx.application.Platform.exit();
System.exit(0);
}
It worked well since years until JDK 21.0.2_13.
If I revert back to 21.0.1_12 it works well.
That method should start a new instance of the app, and close the
"old instance".
With JDK 21.0.2_13, it closed the running instance but does not
open a new instance.
I don't know if open a bugreport to JDK but before opening the bug
report I would like to ask you if that method to restart the app is
correct or if you think that I can use a better way.
Thanks
Davide
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v9]
> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation > conditions were not considered, hence hit test values such as character index > and insertion index values were incorrect. > > Added checks for RTL orientation of nodes and fixed the issue in > `getHitInfo()` to calculate correct hit test values. > > Added system tests to validate the changes. Karthik P K has updated the pull request incrementally with one additional commit since the last revision: Inline node issue fix - Changes: - all: https://git.openjdk.org/jfx/pull/1323/files - new: https://git.openjdk.org/jfx/pull/1323/files/3012fae8..cc89f12a Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1323&range=08 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1323&range=07-08 Stats: 33 lines in 1 file changed: 13 ins; 5 del; 15 mod Patch: https://git.openjdk.org/jfx/pull/1323.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1323/head:pull/1323 PR: https://git.openjdk.org/jfx/pull/1323
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v8]
On Thu, 1 Feb 2024 09:20:35 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of nodes and fixed the issue in >> `getHitInfo()` to calculate correct hit test values. >> >> Added system tests to validate the changes. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Code review changes Fixed the above issues. I have simplified the logic little bit so that it doesn't get confusing as I add more conditions. I tested using the monkey tester and automated tests. Please let me know if you find issue in any scenario. - PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1929218388
Re: RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v5]
On Tue, 6 Feb 2024 00:06:58 GMT, Laurent Bourgès wrote:
>> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java
>
> Laurent Bourgès has updated the pull request incrementally with one
> additional commit since the last revision:
>
> improved rendez-vous to collect stderr
Provided two minor comments. Otherwise ready to approve.
tests/system/src/test/java/test/com/sun/marlin/ScaleX0Test.java line 109:
> 107: Stage stage = new Stage();
> 108: stage.setScene(scene);
> 109: stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e ->
> Platform.runLater(launchLatch::countDown));
Not mandatory, but can be changed to `stage.setOnShown(e ->
Platform.runLater(launchLatch::countDown));`
The import `import javafx.stage.WindowEvent;` can be removed with this change.
tests/system/src/test/java/test/com/sun/marlin/ScaleX0Test.java line 116:
> 114: if (!launchLatch.await(TIMEOUT, TimeUnit.MILLISECONDS)) {
> 115: Assert.fail("Timeout waiting for stage to show");
> 116: }
The Util class contains helper methods to bring consistency in tests that use
such common calls,
So would recommend to use: `Util.waitForLatch(launchLatch, TIMEOUT, "Failed to
show the stage");`
The error message gets prefixed with `Timeout:` in `Util.waitForLatch()`
the import `import java.util.concurrent.TimeUnit;` can be removed with this
change
-
PR Review: https://git.openjdk.org/jfx/pull/1348#pullrequestreview-1864882224
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1479602847
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1479621791
Re: RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v2]
On Mon, 29 Jan 2024 09:09:40 GMT, Karthik P K wrote:
>> Laurent Bourgès has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fixed test package
>
> modules/javafx.graphics/src/main/java/com/sun/marlin/ArrayCacheDouble.java
> line 168:
>
>> 166: if (DO_CLEAN_DIRTY && (toIndex != 0)) {
>> 167: // clean-up array of dirty part[fromIndex; toIndex[
>> 168: fill(array, fromIndex, toIndex, /*(double)*/ 0.0);
>
> Do you think /*(double)*/ will be helpful? In my opinion, it is not required.
@bourgesl are planning to remove these comments? Wanted to check since the
comment was resolved without change.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1479636490
Re: RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v6]
> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java Laurent Bourgès has updated the pull request incrementally with one additional commit since the last revision: minor cleanup to simplify test and fixed imports - Changes: - all: https://git.openjdk.org/jfx/pull/1348/files - new: https://git.openjdk.org/jfx/pull/1348/files/20c22c91..45e786f8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1348&range=05 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1348&range=04-05 Stats: 14 lines in 1 file changed: 3 ins; 8 del; 3 mod Patch: https://git.openjdk.org/jfx/pull/1348.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1348/head:pull/1348 PR: https://git.openjdk.org/jfx/pull/1348
Re: Preview features for JavaFX
I would add that during the 6 month release cycle, there is usually some time to get new features out in an ea release and receive early feedback that way. Maybe even in time that it can be incorporated into the next release if it is reasonable. In this concrete case there was no ea build available in maven central for 3 months that contained this new feature, so barely anyone used it. As an application developer that likes to try out new features, I would like to see more frequent maven releases, although I'm not sure how much work that is and if it is automated. Even if it is not possible to publish every early access release to maven, maybe push a maven release out on important points during development of the current release rather than in fixed intervals. One such important point would have been the release of the platform preferences. I see now that there is another ea release available on maven after a gap of 3 months, but that one does not contain the fix for Windows color schemes, so any Windows ea user will get a nonfunctional feature. Plus making maybe a small announcement when a new larger feature has been released for the first time in early access and linking to the appropriate maven release would result in a good real world test coverage and discussions with other developers. Best Christopher On 2/6/2024 8:32 AM, Robert Lichtenberger wrote: Seems like a good idea to me. From an application developer point of view I don't care if new parts are super-stable (they never really are). If they are marked as preview I'll take that as an additional caveat to not rely on API stability, etc. Robert Am 06.02.24 um 04:19 schrieb Michael Strauß: The discussion around the new Platform Preferences API has brought up a potential area where the API may lack a way to detect whether a particular preference is supported on a particular operating system [0]. Discussions like these will invariably come up when new API is released, and some of the real-world insights may prove to be very valuable. However, with the current development process, we specify and implement new features largely without feedback from application developers. I know that, in principle, developers can join in on the discussion on this mailing list. But the reality is that GA is the first time that a new feature gets wider exposure. All of this makes it very hard for us to ship new features, since we must be extremely careful to get it right the first time. The JDK uses incubator modules and preview features to address these challenges. It seems that OpenJFX will also potentially use an incubator module to introduce new controls [1]. This is great for modular features, but not so great for new API that is added to existing infrastructure. Maybe we could add something akin to preview features to OpenJFX: this could be as easy as documenting the new API to be in preview, or decorate the new API with a @PreviewFeature annotation. I don't think that it is necessary to go beyond simple documentation; in particular, we don't need this to be integrated with the Java compiler. Documenting new features to be preview features will enable us to ship features quicker, and ensure that what we're building is actually useful in the real world because we can actually go back and improve aspects of a feature without worrying as much about backwards compatibility. In particular, my suggestion is to ship the new Platform Preferences API as a preview feature for jfx22. [0] https://mail.openjdk.org/pipermail/openjfx-dev/2024-February/045176.html [1] https://bugs.openjdk.org/browse/JDK-8309381
Re: RFR: 8325154: resizeColumnToFitContent is slower than it needs to be
On Tue, 6 Feb 2024 06:24:18 GMT, Robert Lichtenberger wrote: > So maybe we should: > > * Provide an API that let's the client specify how many rows should be taken > into account > * Make resizeColumnToFitContent available for clients Let's create an RFE. What would be your recommendation for the specific API? - PR Comment: https://git.openjdk.org/jfx/pull/1358#issuecomment-1930102012
Re: RFR: JDK-8324182 Deprecate for removal SimpleSelector and CompoundSelector classes [v3]
On Fri, 2 Feb 2024 13:30: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
>> directly and no other JavaFX API accepts or returns a SimpleSelector or
>> CompoundSelector.
>>
>> We should deprecate them for removal so we can move them to a non-exported
>> package, removing them from the public API.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Deprecate for 23 and add new method
Looks good, with minor changes.
This also needs a CSR (which has already been requested).
I'll re-approve once we all agree on the new method name.
modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 95:
> 93:
> 94: /**
> 95: * Gets the set of style class names of this Selector. The returned
> set
Maybe just "Gets the immutable set of style class names of this Selector." ?
modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 102:
> 100: * @since 23
> 101: */
> 102: public abstract Set getClasses();
suggestion:
`getStyleClassesSet()`
getClasses() creates expectation of returning `Class<>`es.
modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 200:
> 198: @Override
> 199: public Set getClasses() {
> 200: return
> styleClassSet.stream().map(StyleClass::getStyleClassName).collect(Collectors.toUnmodifiableSet());
minor: could we reformat this on multiple lines, like the other?
-
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1340#pullrequestreview-1865631005
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480084573
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480083671
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480081109
Re: TreeTableView / FilteredList momentary incorrect selection bug
Thank you for reporting the issue!
Is this the same scenario as described in
https://bugs.openjdk.org/browse/JDK-8321323 ?
-andy
From: openjfx-dev on behalf of Cormac Redmond
Date: Monday, February 5, 2024 at 12:31
To: openjfx-dev@openjdk.org
Subject: TreeTableView / FilteredList momentary incorrect selection bug
Hi folks,
I have noticed an issue when combining TreeTableView and FilteredLists, where a
wrong node is "selected" (I believe during some shift selection functionality
in TreeTableView). Currently using JavaFX 21-ea+5 on Windows, but occurs in
later builds too.
First noticed in a much more complex scenario with many components, I narrowed
it down quite a bit, and created the simplest example I could, to demonstrate
what I think is a bug.
Let's say you have a tree (TableTreeView) displayed like this (as per code
below):
root (invisible)
| ggg1
| ggg1.1
| xxx1.2
| ggg1.3
| bbb2
| bbb2.1
| bbb2.2
| bbb2.3
| aaa3
| aaa3.1
| aaa3.2
| aaa3.3
If you select leaf node "aaa3.2", for example, and then filter using a string
"ggg", the node "bbb2", is being selected unexpectedly/incorrectly in the
process, where it shouldn't. This is the bug.
Here's a simple way to reproduce the issue. Run the code, and look at the tree
first. Observe that a leaf node "aaa3.2" is selected for you (the code selects
this as a shortcut for you).
Hit the button to filter with string "ggg", and notice the logging showing that
"bbb2" -- the leaf node's parent's sibling, is incorrectly momentarily
selected, before "null" is settled as the final selected value (null being
correct). Why is this happening?
Sample output of running the below code:
Value of aaa3.2 from tree (for verification): aaa3.2 < printed to show
the node about to be selected is the correct node
Selecting item: aaa3.2< printed to show the code is about to select
it
Selected item (as per listener): aaa3.2 < printed by the listener,
showing it was selected
About to filter on "ggg"< printed to show you hit the button,
now the list is filtering which will change the tree
Selected item (as per listener): bbb2< printed by the
listener, showing bbb2 is selected , why is this happening along the way? This
seems like a bug. Maybe it's part of some "let's select the closest sibling"
logic, but...why? And if so, it's not a consistent pattern/logic that I can
understand.
Selected item (as per listener): null < printed by the listener,
showing null is "selected", which is fine / expected, as the *real* selected
item has been filtered out
Runnable code:
import javafx.application.Application;
import javafx.beans.binding.Bindings;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ObservableValue;
import javafx.collections.FXCollections;
import javafx.collections.transformation.FilteredList;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
public class TreeTableSelectBug extends Application {
private final TreeTableView tree = new TreeTableView<>();
private final ObjectProperty> filterPredicate = new
SimpleObjectProperty<>();
@Override
public void start(Stage primaryStage) throws Exception {
final VBox outer = new VBox();
tree.setShowRoot(false);
tree.getSelectionModel().setSelectionMode(SelectionMode.SINGLE);
tree.setRoot(createTree());
addColumn();
// Print selection changes: there should only be two (initial
selection, then final selection to "null" when nodes are filtered), but there
is an extra one ("bbb2") in the middle.
tree.getSelectionModel().selectedItemProperty().addListener((observable,
oldValue, newValue)
-> System.out.println("Selected item (as per listener): " +
(tree.getSelectionModel().getSelectedItem() == null ? "null" :
tree.getSelectionModel().getSelectedItem().getValue(;
final Button filterButton = new Button("Filter on \"ggg\"");
outer.getChildren().addAll(filterButton, tree);
final Scene scene = new Scene(outer, 640, 480);
primaryStage.setScene(scene);
primaryStage.show();
// Select a lead node: aaa3 -> aaa3.2 (as an example)
final TreeItem aaa32 =
tree.getRoot().getChildren().get(2).getChildren().get(1);
System.out.println("Value of aaa3.2 from tree (for verification): " +
aaa32.getValue());
// Expand it -- without expanding it, the bug won't occur
aaa32.getParent().setExpanded(true);
System.out.println("Selecting item: " + aaa32.getValue());
// Select an item, note it is printed. Same as a user clicking the row.
tree.getSelectionModel
Re: TreeTableView / FilteredList momentary incorrect selection bug
Hi Andy,
Thank you; yes, that report was also myself via bugreport.java.com, I think
-- I didn't actually realise it ended up anywhere.
The mail I dropped here though is somewhat easier to digest / has less
typos (for anyone interested)!
Kind Regards,
Cormac
On Tue, 6 Feb 2024 at 16:11, Andy Goryachev
wrote:
> Thank you for reporting the issue!
>
>
>
> Is this the same scenario as described in
> https://bugs.openjdk.org/browse/JDK-8321323 ?
>
>
>
> -andy
>
>
>
>
>
> *From: *openjfx-dev on behalf of Cormac
> Redmond
> *Date: *Monday, February 5, 2024 at 12:31
> *To: *openjfx-dev@openjdk.org
> *Subject: *TreeTableView / FilteredList momentary incorrect selection bug
>
> Hi folks,
>
>
>
> I have noticed an issue when combining TreeTableView and FilteredLists,
> where a wrong node is "selected" (I believe during some shift selection
> functionality in TreeTableView). Currently using JavaFX 21-ea+5 on Windows,
> but occurs in later builds too.
>
>
>
> First noticed in a much more complex scenario with many components, I
> narrowed it down quite a bit, and created the simplest example I could, to
> demonstrate what I think is a bug.
>
>
>
> Let's say you have a tree (TableTreeView) displayed like this (as per code
> below):
>
>
>
> root (invisible)
>
>| ggg1
>
>| ggg1.1
>| xxx1.2
>| ggg1.3
>| bbb2
>| bbb2.1
>| bbb2.2
>| bbb2.3
>| aaa3
>| aaa3.1
>| aaa3.2
>| aaa3.3
>
>
>
> If you select leaf node "aaa3.2", for example, and then filter using a
> string "ggg", the node "bbb2", is being selected unexpectedly/incorrectly
> in the process, where it shouldn't. This is the bug.
>
>
>
> Here's a simple way to reproduce the issue. Run the code, and look at the
> tree first. Observe that a leaf node "aaa3.2" is selected for you (the code
> selects this as a shortcut for you).
>
>
>
> Hit the button to filter with string "ggg", and notice the logging showing
> that "bbb2" -- the leaf node's parent's sibling, is incorrectly momentarily
> selected, before "null" is settled as the final selected value (null being
> correct). Why is this happening?
>
> Sample output of running the below code:
>
>
> Value of aaa3.2 from tree (for verification): aaa3.2 < printed to
> show the node about to be selected is the correct node
> Selecting item: aaa3.2< printed to show the code is about to
> select it
> Selected item (as per listener): aaa3.2 < printed by the
> listener, showing it was selected
> About to filter on "ggg"< printed to show you hit the
> button, now the list is filtering which will change the tree
> Selected item (as per listener): bbb2< printed by the
> listener, showing bbb2 is selected , why is this happening along the way?
> This seems like a bug. Maybe it's part of some "let's select the closest
> sibling" logic, but...why? And if so, it's not a consistent pattern/logic
> that I can understand.
> Selected item (as per listener): null < printed by the
> listener, showing null is "selected", which is fine / expected, as the
> *real* selected item has been filtered out
>
>
>
> Runnable code:
>
>
>
> import javafx.application.Application;
> import javafx.beans.binding.Bindings;
> import javafx.beans.property.ObjectProperty;
> import javafx.beans.property.SimpleObjectProperty;
> import javafx.beans.value.ObservableValue;
> import javafx.collections.FXCollections;
> import javafx.collections.transformation.FilteredList;
> import javafx.scene.Scene;
> import javafx.scene.control.*;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> import java.util.ArrayList;
> import java.util.List;
> import java.util.function.Predicate;
>
> public class TreeTableSelectBug extends Application {
> private final TreeTableView tree = new TreeTableView<>();
> private final ObjectProperty> filterPredicate = new
> SimpleObjectProperty<>();
>
> @Override
> public void start(Stage primaryStage) throws Exception {
> final VBox outer = new VBox();
>
> tree.setShowRoot(false);
> tree.getSelectionModel().setSelectionMode(SelectionMode.SINGLE);
> tree.setRoot(createTree());
> addColumn();
>
> // Print selection changes: there should only be two (initial
> selection, then final selection to "null" when nodes are filtered), but
> there is an extra one ("bbb2") in the middle.
>
> tree.getSelectionModel().selectedItemProperty().addListener((observable,
> oldValue, newValue)
> -> System.out.println("Selected item (as per listener): "
> + (tree.getSelectionModel().getSelectedItem() == null ? "null" :
> tree.getSelectionModel().getSelectedItem().getValue(;
>
> final Button filterButton = new Button("Filter on \"ggg\"");
>
> outer.getChildren().addAll(filterButton, tree);
> final Scene scene = new Scene(outer, 640, 480);
> primaryStage.setScene(scene);
>
Re: RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v6]
On Tue, 6 Feb 2024 13:49:13 GMT, Laurent Bourgès wrote: >> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java > > Laurent Bourgès has updated the pull request incrementally with one > additional commit since the last revision: > > minor cleanup to simplify test and fixed imports Marked as reviewed by arapte (Reviewer). - PR Review: https://git.openjdk.org/jfx/pull/1348#pullrequestreview-1865792652
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v9]
On Tue, 6 Feb 2024 10:31:18 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of nodes and fixed the issue in >> `getHitInfo()` to calculate correct hit test values. >> >> Added system tests to validate the changes. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Inline node issue fix Fails with "Emojis" data set (magenta line indicates where):  - PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1930497240
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v9]
On Tue, 6 Feb 2024 10:31:18 GMT, Karthik P K wrote: >> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation >> conditions were not considered, hence hit test values such as character >> index and insertion index values were incorrect. >> >> Added checks for RTL orientation of nodes and fixed the issue in >> `getHitInfo()` to calculate correct hit test values. >> >> Added system tests to validate the changes. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Inline node issue fix Also fails with "Tabs":  - PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1930506784
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v10]
On Wed, 31 Jan 2024 00:34:57 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 >> >> They have in common that wrapped text is taking the trailing spaces on each >> wrapped line into account when calculating where to wrap. This looks okay >> for text that is left aligned (as the spaces will be trailing the lines and >> generally aren't a problem, but looks weird with CENTER and RIGHT >> alignments. Even with LEFT alignment there are artifacts of this behavior, >> where a line like `AAA BBB CCC` (note the **double** spaces) gets split up >> into `AAA `, `BBB ` and `CCC`, but if space reduces further, it will wrap >> **too** early because the space is taken into account (ie. `AAA` may still >> have fit just fine, but `AAA ` doesn't, so the engine wraps it to `AA` + `A >> ` or something). >> >> The fix for this is two fold; first the individual lines of text should not >> include any trailing spaces into their widths; second, the code that is >> taking the trailing space into account when wrapping should ignore all >> trailing spaces (currently it is ignoring all but one trailing space). With >> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great, >> and there is no more early wrapping due to a space being taking into account >> while the actual text still would have fit (this is annoying in tight >> layouts, where a line can be wrapped early even though it looks like it >> would have fit). >> >> If it were that simple, we'd be done, but there may be another issue here >> that needs solving: wrapped aligned TextArea's. >> >> TextArea don't directly support text alignment (via a setTextAlignment >> method like Label) but you can change it via CSS. >> >> For Left alignment + wrapping, TextArea will ignore any spaces typed before >> a line that was wrapped. In other words, you can type spaces as much as you >> want, and they won't show up and the cursor won't move. The spaces are all >> getting appended to the previous line. When you cursor through these >> spaces, the cursor can be rendered out of the control's bounds. To >> illustrate, if you have the text `AAA BBB 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 wrapped text >> controls,... > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test comment ... and with RIGHT alignment:  - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930559657
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v7]
On Tue, 30 Jan 2024 00:35:24 GMT, John Hendrikx wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix bug which confused char index with glyph index > > I think that it may be wise to do some clean-up of the code surrounding > `TextRun` in some future PR. It's a mish-mash of things currently: > > - TextRun seems to be used in two ways. > - The "normal" way where multiple runs are constructed from a character > array, and it retains references to the start/end in that array (although not > a reference to the array itself). Because it has no reference to the chars > from which it was created, a lot of functionality which could be encapsulated > is externalized, and many internals of `TextRun` need to be exposed to feed > those external functions. > - For javafx-web, which just wants to render a bunch of glyphs, unrelated > to any text (so there is no character array it is based on, and its start/end > values are bogus) > - There is a lot of code that pretends there is a difference between a > `GlyphList` (clearly intended for rendering pure glyphs only) and `TextRun`, > but there is only one implementation of `GlyphList` (`TextRun`). The code > that does the actual rendering in `NGText` bluntly always casts a `GlyphList` > to a `TextRun`. It does this for the sole purpose of finding out the start > character position of the original characters the `TextRun` was created from > (needed for selection rendering), yet, for `TextRun`s created by javafx-web > this is irrelevant (it just always returns `0` for the start character > position). > - It would have been better to do a conditional cast to `TextRun` in > `NGText` so that javafx-web can use a much simpler implementation of > `GlyphList` not based on `TextRun`. > - In a lot of places in this code and surrounding code, fields have been > failed to be marked `private` or `final` even though they can be; this may > have implications for what JIT can do. > - There are unused fields (`isCanonical` always returns `false`, `TextLine` > is only stored to get its `RectBounds`, could just store that directly, it's > `final`) > > A clean-up would entail: > - Include a reference to the characters it was created from in `TextRun` > - Encapsulate a lot of code into `TextRun` (moved from `PrismTextLayout`) > that does calculations that needed both internal information of `TextRun` > (that no longer needs to be exposed) and the original characters. > - Have javafx-web just implement `GlyphList` and change `NGText` to work with > `GlyphList`. This class would be much smaller (`TextRun` has over a dozen > fields). > - In `NGText` do an `instanceof` check to see if it is a `TextRun`, in which > case its start offset can be used, oth... @hjohn are these screenshots show expected layout (with the CENTER alignment and regular spaces 0x20)? Shouldn't the leading/trailing whitespace be ignored?   - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930558059
Withdrawn: 8299753: Tree/TableView: Column Resizing With Fractional Scale
On Thu, 15 Jun 2023 19:38:00 GMT, Andy Goryachev wrote: > Modified the resize algorithm to work well with fractional scale, thanks for > deeper understanding of the problem thanks to @hjohn and @mstr2 . > > It is important to note that even though the constraints are given by the > user in unsnapped coordinates, they are converted to snapped values, since > the snapped values correspond to the actual pixels on the display. This > means the tests that validate honoring constraints should, in all the cases > where (scale != 1.0), assume possibly error not exceeding (1.0 / scale) (I > think). This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jfx/pull/1156
Re: RFR: 8299753: Tree/TableView: Column Resizing With Fractional Scale [v9]
> Modified the resize algorithm to work well with fractional scale, thanks for > deeper understanding of the problem thanks to @hjohn and @mstr2 . > > It is important to note that even though the constraints are given by the > user in unsnapped coordinates, they are converted to snapped values, since > the snapped values correspond to the actual pixels on the display. This > means the tests that validate honoring constraints should, in all the cases > where (scale != 1.0), assume possibly error not exceeding (1.0 / scale) (I > think). Andy Goryachev 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 28 additional commits since the last revision: - Merge branch 'master' into 8299753.resize - Merge remote-tracking branch 'origin/master' into 8299753.resize - Merge remote-tracking branch 'origin/master' into 8299753.resize - tolerance - Merge remote-tracking branch 'origin/master' into 8299753.resize - undo merge - no new api - Merge remote-tracking branch 'origin/master' into 8299753.resize - cleanup - using snap inner space api - ... and 18 more: https://git.openjdk.org/jfx/compare/1b970fdc...9105b009 - Changes: - all: https://git.openjdk.org/jfx/pull/1156/files - new: https://git.openjdk.org/jfx/pull/1156/files/80286f26..9105b009 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1156&range=08 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1156&range=07-08 Stats: 294533 lines in 6591 files changed: 173273 ins; 84720 del; 36540 mod Patch: https://git.openjdk.org/jfx/pull/1156.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1156/head:pull/1156 PR: https://git.openjdk.org/jfx/pull/1156
Re: RFR: JDK-8324182 Deprecate for removal SimpleSelector and CompoundSelector classes [v4]
> 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 > directly and no other JavaFX API accepts or returns a SimpleSelector or > CompoundSelector. > > We should deprecate them for removal so we can move them to a non-exported > package, removing them from the public API. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Resolve review comments - Changes: - all: https://git.openjdk.org/jfx/pull/1340/files - new: https://git.openjdk.org/jfx/pull/1340/files/382ef218..631ca587 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1340&range=03 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1340&range=02-03 Stats: 5 lines in 2 files changed: 2 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jfx/pull/1340.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1340/head:pull/1340 PR: https://git.openjdk.org/jfx/pull/1340
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v10]
On Wed, 31 Jan 2024 00:34:57 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 >> >> They have in common that wrapped text is taking the trailing spaces on each >> wrapped line into account when calculating where to wrap. This looks okay >> for text that is left aligned (as the spaces will be trailing the lines and >> generally aren't a problem, but looks weird with CENTER and RIGHT >> alignments. Even with LEFT alignment there are artifacts of this behavior, >> where a line like `AAA BBB CCC` (note the **double** spaces) gets split up >> into `AAA `, `BBB ` and `CCC`, but if space reduces further, it will wrap >> **too** early because the space is taken into account (ie. `AAA` may still >> have fit just fine, but `AAA ` doesn't, so the engine wraps it to `AA` + `A >> ` or something). >> >> The fix for this is two fold; first the individual lines of text should not >> include any trailing spaces into their widths; second, the code that is >> taking the trailing space into account when wrapping should ignore all >> trailing spaces (currently it is ignoring all but one trailing space). With >> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great, >> and there is no more early wrapping due to a space being taking into account >> while the actual text still would have fit (this is annoying in tight >> layouts, where a line can be wrapped early even though it looks like it >> would have fit). >> >> If it were that simple, we'd be done, but there may be another issue here >> that needs solving: wrapped aligned TextArea's. >> >> TextArea don't directly support text alignment (via a setTextAlignment >> method like Label) but you can change it via CSS. >> >> For Left alignment + wrapping, TextArea will ignore any spaces typed before >> a line that was wrapped. In other words, you can type spaces as much as you >> want, and they won't show up and the cursor won't move. The spaces are all >> getting appended to the previous line. When you cursor through these >> spaces, the cursor can be rendered out of the control's bounds. To >> illustrate, if you have the text `AAA BBB 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 wrapped text >> controls,... > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test comment Is this something specific to strike though? I've been testing with underline (because it conveniently shows you where the spaces are), but I haven't seen anything like the above. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930790225
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v10]
On Tue, 6 Feb 2024 21:35:43 GMT, John Hendrikx wrote: > specific to strike though I don't think so. I can turn it off and nothing changes, same for underline. I am not sure why it all suddenly started to fail - I did a clean build twice... - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930793983
Re: RFR: 8307117: TextArea: wrapText property ignored when changing font
On Mon, 21 Aug 2023 21:44:12 GMT, Andy Goryachev wrote: > Requesting content layout when font changes. > > This change makes the visual impact of > [JDK-8314683](https://bugs.openjdk.org/browse/JDK-8314683) more visible, so > perhaps both bugs should be fixed at the same time. keep alive keep alive on second thought, let's address this independently of [JDK-8314683](https://bugs.openjdk.org/browse/JDK-8314683). - PR Comment: https://git.openjdk.org/jfx/pull/1217#issuecomment-1765372038 PR Comment: https://git.openjdk.org/jfx/pull/1217#issuecomment-1852327075 PR Comment: https://git.openjdk.org/jfx/pull/1217#issuecomment-1930780284
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v10]
On Tue, 6 Feb 2024 21:38:41 GMT, Andy Goryachev wrote: > > specific to strike though > > I don't think so. I can turn it off and nothing changes, same for underline. > I am not sure why it all suddenly started to fail - I did a clean build > twice... Can you see if the new `TextLayoutTest` system test ran (with all my new tests) during the build, it should already be catching this problem? This seems to have regressed to the "before" situation. I'm not able to find the problem on my end at the moment, but I'll do a clean build and see if that gets anything. All the files I have locally are pushed. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930840916
RFR: 8307117: TextArea: wrapText property ignored when changing font
Requesting content layout when font changes. This change makes the visual impact of [JDK-8314683](https://bugs.openjdk.org/browse/JDK-8314683) more visible, so perhaps both bugs should be fixed at the same time. - Commit messages: - Merge branch 'master' into 8307117.wrap - request layout when changing font Changes: https://git.openjdk.org/jfx/pull/1217/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1217&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8307117 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jfx/pull/1217.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1217/head:pull/1217 PR: https://git.openjdk.org/jfx/pull/1217
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v10]
On Wed, 31 Jan 2024 00:34:57 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 >> >> They have in common that wrapped text is taking the trailing spaces on each >> wrapped line into account when calculating where to wrap. This looks okay >> for text that is left aligned (as the spaces will be trailing the lines and >> generally aren't a problem, but looks weird with CENTER and RIGHT >> alignments. Even with LEFT alignment there are artifacts of this behavior, >> where a line like `AAA BBB CCC` (note the **double** spaces) gets split up >> into `AAA `, `BBB ` and `CCC`, but if space reduces further, it will wrap >> **too** early because the space is taken into account (ie. `AAA` may still >> have fit just fine, but `AAA ` doesn't, so the engine wraps it to `AA` + `A >> ` or something). >> >> The fix for this is two fold; first the individual lines of text should not >> include any trailing spaces into their widths; second, the code that is >> taking the trailing space into account when wrapping should ignore all >> trailing spaces (currently it is ignoring all but one trailing space). With >> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great, >> and there is no more early wrapping due to a space being taking into account >> while the actual text still would have fit (this is annoying in tight >> layouts, where a line can be wrapped early even though it looks like it >> would have fit). >> >> If it were that simple, we'd be done, but there may be another issue here >> that needs solving: wrapped aligned TextArea's. >> >> TextArea don't directly support text alignment (via a setTextAlignment >> method like Label) but you can change it via CSS. >> >> For Left alignment + wrapping, TextArea will ignore any spaces typed before >> a line that was wrapped. In other words, you can type spaces as much as you >> want, and they won't show up and the cursor won't move. The spaces are all >> getting appended to the previous line. When you cursor through these >> spaces, the cursor can be rendered out of the control's bounds. To >> illustrate, if you have the text `AAA BBB 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 wrapped text >> controls,... > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test comment one thing: you may sync up with the latest master branch, that's the configuration I am testing. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1930945346
Re: Preview features for JavaFX
In order for preview features and incubating features to not cause more problems than they solve, there needs to be a robust way to ensure that applications and libraries don't use them without knowing that they are doing so. We know how to do that for a feature that lives in its own module (an incubating feature), but not how to do that for something like a preview feature. For incubating features, this is relatively straight-forward, since they are delivered in a separate module that has "incubator" in the name, isn't resolved by default, and warns you at runtime when those modules are resolved. Adapting what the JDK does for JavaFX should be pretty easy, and retain the benefit that an app knows when they are using incubating features. I don't think it is feasible to do the same thing for preview features. The way the JDK preview features work is that a command line option is needed both at compile time and at runtime to opt into preview features for a specific release. This prevents using a preview API from an existing module and package without knowing that it is subject to change. Without a clear "opt in" mechanism to be able to use an API, an app would be able to accidentally use a feature whose API is unstable and quite possible might change. An annotation isn't good enough (and documentation certainly isn't sufficient). IDEs will still autocomplete and show the API, and once an app uses it -- accidentally or otherwise -- there is no indication at runtime that you are using a feature that will likely stop working without any notice in the next version. I don't see a good way to do this for JavaFX given the limitations. -- Kevin On 2/5/2024 7:19 PM, Michael Strauß wrote: The discussion around the new Platform Preferences API has brought up a potential area where the API may lack a way to detect whether a particular preference is supported on a particular operating system [0]. Discussions like these will invariably come up when new API is released, and some of the real-world insights may prove to be very valuable. However, with the current development process, we specify and implement new features largely without feedback from application developers. I know that, in principle, developers can join in on the discussion on this mailing list. But the reality is that GA is the first time that a new feature gets wider exposure. All of this makes it very hard for us to ship new features, since we must be extremely careful to get it right the first time. The JDK uses incubator modules and preview features to address these challenges. It seems that OpenJFX will also potentially use an incubator module to introduce new controls [1]. This is great for modular features, but not so great for new API that is added to existing infrastructure. Maybe we could add something akin to preview features to OpenJFX: this could be as easy as documenting the new API to be in preview, or decorate the new API with a @PreviewFeature annotation. I don't think that it is necessary to go beyond simple documentation; in particular, we don't need this to be integrated with the Java compiler. Documenting new features to be preview features will enable us to ship features quicker, and ensure that what we're building is actually useful in the real world because we can actually go back and improve aspects of a feature without worrying as much about backwards compatibility. In particular, my suggestion is to ship the new Platform Preferences API as a preview feature for jfx22. [0] https://mail.openjdk.org/pipermail/openjfx-dev/2024-February/045176.html [1] https://bugs.openjdk.org/browse/JDK-8309381
Re: Preview features for JavaFX
Announcing new features and encouraging testing of those features is a good idea. We should do that more often, especially in cases where feedback on the API is important. Developers who are willing to download the ea builds (SDK or JMODs) can do so right away, since builds are published weekly. Gluon produces the maven releases, so Johan can probably answer the question about the frequency of EA snapshots in maven central. -- Kevin On 2/6/2024 6:54 AM, Christopher Schnick wrote: I would add that during the 6 month release cycle, there is usually some time to get new features out in an ea release and receive early feedback that way. Maybe even in time that it can be incorporated into the next release if it is reasonable. In this concrete case there was no ea build available in maven central for 3 months that contained this new feature, so barely anyone used it. As an application developer that likes to try out new features, I would like to see more frequent maven releases, although I'm not sure how much work that is and if it is automated. Even if it is not possible to publish every early access release to maven, maybe push a maven release out on important points during development of the current release rather than in fixed intervals. One such important point would have been the release of the platform preferences. I see now that there is another ea release available on maven after a gap of 3 months, but that one does not contain the fix for Windows color schemes, so any Windows ea user will get a nonfunctional feature. Plus making maybe a small announcement when a new larger feature has been released for the first time in early access and linking to the appropriate maven release would result in a good real world test coverage and discussions with other developers. Best Christopher On 2/6/2024 8:32 AM, Robert Lichtenberger wrote: Seems like a good idea to me. From an application developer point of view I don't care if new parts are super-stable (they never really are). If they are marked as preview I'll take that as an additional caveat to not rely on API stability, etc. Robert Am 06.02.24 um 04:19 schrieb Michael Strauß: The discussion around the new Platform Preferences API has brought up a potential area where the API may lack a way to detect whether a particular preference is supported on a particular operating system [0]. Discussions like these will invariably come up when new API is released, and some of the real-world insights may prove to be very valuable. However, with the current development process, we specify and implement new features largely without feedback from application developers. I know that, in principle, developers can join in on the discussion on this mailing list. But the reality is that GA is the first time that a new feature gets wider exposure. All of this makes it very hard for us to ship new features, since we must be extremely careful to get it right the first time. The JDK uses incubator modules and preview features to address these challenges. It seems that OpenJFX will also potentially use an incubator module to introduce new controls [1]. This is great for modular features, but not so great for new API that is added to existing infrastructure. Maybe we could add something akin to preview features to OpenJFX: this could be as easy as documenting the new API to be in preview, or decorate the new API with a @PreviewFeature annotation. I don't think that it is necessary to go beyond simple documentation; in particular, we don't need this to be integrated with the Java compiler. Documenting new features to be preview features will enable us to ship features quicker, and ensure that what we're building is actually useful in the real world because we can actually go back and improve aspects of a feature without worrying as much about backwards compatibility. In particular, my suggestion is to ship the new Platform Preferences API as a preview feature for jfx22. [0] https://mail.openjdk.org/pipermail/openjfx-dev/2024-February/045176.html [1] https://bugs.openjdk.org/browse/JDK-8309381
Re: RFR: JDK-8324182 Deprecate for removal SimpleSelector and CompoundSelector classes [v4]
On Tue, 6 Feb 2024 21:35:12 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 >> directly and no other JavaFX API accepts or returns a SimpleSelector or >> CompoundSelector. >> >> We should deprecate them for removal so we can move them to a non-exported >> package, removing them from the public API. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Resolve review comments So we can add `getStyleClassesSet` to the list of possible method names. I think the word "class" should be in the name of the method, which excludes `getStyles` and `getStyleNames`, leaving the following: getStyleClassNames getClasses getClassNames getStyleClassesSet Any of them seem OK to me. I don't have a strong preference. - PR Comment: https://git.openjdk.org/jfx/pull/1340#issuecomment-1930988290
Re: RFR: 8307117: TextArea: wrapText property ignored when changing font
On Mon, 21 Aug 2023 21:44:12 GMT, Andy Goryachev wrote: > Requesting content layout when font changes. > > This change makes the visual impact of > [JDK-8314683](https://bugs.openjdk.org/browse/JDK-8314683) more visible, so > perhaps both bugs should be fixed at the same time. Reviewers: @arapte @karthikpandelu This seems reasonable, but I'll let Ambarish and Karthik review. My only question is whether it would it be possible to create an automated test for this? - PR Comment: https://git.openjdk.org/jfx/pull/1217#issuecomment-1930991665
Re: Preview features for JavaFX
Hi Kevin, my suggestion would be to annotate and document the preview API (at least annotations do show up by default in most IDEs), and emit a one-time runtime warning when the API is used (this works for methods and constructors). This would make it quite visible to developers that they are using a preview feature, or that a third-party library uses a preview feature. The runtime warning can be suppressed with a command line parameter such as "javafx.enablePreviewFeatures". A more drastic approach would be to throw an exception from new APIs when the parameter is not specified. Given that there are very tangible benefits to previewing new API, this would seem to me like a good enough solution. On Wed, Feb 7, 2024 at 12:59 AM Kevin Rushforth wrote: > > In order for preview features and incubating features to not cause more > problems than they solve, there needs to be a robust way to ensure that > applications and libraries don't use them without knowing that they are > doing so. We know how to do that for a feature that lives in its own > module (an incubating feature), but not how to do that for something > like a preview feature. > > For incubating features, this is relatively straight-forward, since they > are delivered in a separate module that has "incubator" in the name, > isn't resolved by default, and warns you at runtime when those modules > are resolved. Adapting what the JDK does for JavaFX should be pretty > easy, and retain the benefit that an app knows when they are using > incubating features. > > I don't think it is feasible to do the same thing for preview features. > The way the JDK preview features work is that a command line option is > needed both at compile time and at runtime to opt into preview features > for a specific release. This prevents using a preview API from an > existing module and package without knowing that it is subject to > change. Without a clear "opt in" mechanism to be able to use an API, an > app would be able to accidentally use a feature whose API is unstable > and quite possible might change. An annotation isn't good enough (and > documentation certainly isn't sufficient). IDEs will still autocomplete > and show the API, and once an app uses it -- accidentally or otherwise > -- there is no indication at runtime that you are using a feature that > will likely stop working without any notice in the next version. > > I don't see a good way to do this for JavaFX given the limitations. > > -- Kevin
Re: RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v6]
On Tue, 6 Feb 2024 13:49:13 GMT, Laurent Bourgès wrote: >> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java > > Laurent Bourgès has updated the pull request incrementally with one > additional commit since the last revision: > > minor cleanup to simplify test and fixed imports Marked as reviewed by kpk (Committer). - PR Review: https://git.openjdk.org/jfx/pull/1348#pullrequestreview-1866929656
Re: RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v9]
On Tue, 6 Feb 2024 18:09:40 GMT, Andy Goryachev wrote: > Fails with "Emojis" data set (magenta line indicates where): > I forgot to mention that I didn't test with emojis. I wanted to make it work with text with all scenario and then move to emojis. Now that we have text related issues fixed, I will check this. With tabs it works properly on the text and on the tab spaces it fails now. I will look into this as well. - PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1931312819
Integrated: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0
On Wed, 24 Jan 2024 14:55:21 GMT, Laurent Bourgès wrote: > Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java This pull request has now been integrated. Changeset: 172c491d Author:Laurent Bourgès URL: https://git.openjdk.org/jfx/commit/172c491dae4b774012589d991c898f12ccb44a4c Stats: 340 lines in 14 files changed: 276 ins; 10 del; 54 mod 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 Reviewed-by: kpk, arapte - PR: https://git.openjdk.org/jfx/pull/1348
Re: RFR: 8325154: resizeColumnToFitContent is slower than it needs to be
On Tue, 6 Feb 2024 15:53:07 GMT, Andy Goryachev wrote: > > So maybe we should: > > > > * Provide an API that let's the client specify how many rows should be > > taken into account > > * Make resizeColumnToFitContent available for clients > > Let's create an RFE. What would be your recommendation for the specific API? At first I had only thought about a simple "setColumnFitSamplSize(int sampleSize)" method. However, @hjohn s suggestion of a strategy pattern seems convincing to me: It allows the client to specify how the required column width is calculated. I'm not quite sure how to approach this however. If we have a "ColumnFitStrategy" interface/baseclass with just a simple "calculateColumnWidth(int column)" this would not help clients since they don't have access to a lot of the stuff that's happening inside resizeColumnToFitContent. If, on the other hand, we only allow a "ColumnFitRowSelection" interface/baseclass that will return the rows to be used for measuring, the client code will not be able to optimize in some cases (e.g. you have graphics in your table; loading them is expensive; but you know the size of the graphics and could thus give a "correct" required size without really loading/applying css). - PR Comment: https://git.openjdk.org/jfx/pull/1358#issuecomment-1931449204
Re: RFR: 8325154: resizeColumnToFitContent is slower than it needs to be
On Mon, 5 Feb 2024 09:45:25 GMT, Marius Hanl wrote: > I will review this PR soon as well. The discussion about a "bigger" solution aside I think the PR is still of value to improve performance, so I look forward to your review. - PR Comment: https://git.openjdk.org/jfx/pull/1358#issuecomment-1931452084
