Re: Proposal: Public InputMap (v2)
Hi there, Thank you Robert for your feedback! As a fellow application and custom component developer, I can't wait for this feature to get in. I wonder if anyone else have any objections or feedback, specifically John Hendrikx and Martin Fox. I hope the new proposal addresses the concerns you've raised earlier (support for stateless behaviors and explicit priority of event handlers in Controls), and we can move forward with the feature. Cheers, -andy From: openjfx-dev on behalf of Robert Lichtenberger Date: Monday, March 11, 2024 at 23:23 To: openjfx-dev@openjdk.org Subject: Re: Proposal: Public InputMap (v2) I've had a look into the proposal and I like it a lot. I've had numerous encounters with "little tweaks" to keyboard handling and/or behaviour that I had to work around, all of which would (as far as I can tell) have been avoidable, if this API were in existence. So while there is quite some "API surface", as an application developer I can't wait to see this happening. It will go a long way to make JavaFX more easily usable. --Robert Am 11.03.24 um 16:22 schrieb Andy Goryachev: Dear JavaFX developers: Thank you all for your feedback on my earlier Behavior/InputMap proposal [6], [7]. There was some positive reaction to the idea of allowing for easy customization of JavaFX controls behavior, as well as some push back. Some of the objections were: * desire for some static behavior to avoid the per-instance penalty * clearer separation of concerns between controls, skins, and behaviors * desire to have some sort of public API for behaviors * need for addressing an issue with the event handler execution order between application and skins I would like to restart the discussion by submitting the updated proposal [0] which addresses the issues raised earlier. The new proposal retains the idea of allowing wider customization of key mappings via the InputMap. The new features include: * separation of SkinInputMap from the control's InputMap * enabling static skin input maps for stateless behaviors * explicit priority levels for event handlers and key mappings created by the application and by the skin The ideas put forth in this proposal have been validated with the proof-of-concept migration of some non-trivial controls: * complex popup-like controls: ComboBox, ColorPicker, DatePicker * complex text input controls: PasswordField, TextArea, TextField * control with a stateless behavior: TabPane as well as a brand new RichTextArea control being incubated [8]. Please take a look at the proposal [0], a list of discussion points [1], and the API Specification (javadoc) [2]. While the proposed API is ready for review, it isn't complete nor set in stone. We are looking for feedback, and will update the proposal based on the suggestions we receive from the community. We encourage you to comment either in the mailing list, or by leaving comments inline in a draft pull request [3]. For context, the links to the original RFE [4] and the earlier proposals [6], [7] are provided below. What do you think? -andy References [0] Proposal: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/InputMap/InputMapV2.md [1] Discussion points: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/InputMap/InputMapV2-Discussion.md [2] API specification (javadoc): https://cr.openjdk.org/~angorya/InputMapV2/javadoc/ [3] Draft Pull Request for API comments and feedback: https://github.com/openjdk/jfx/pull/1395 [4] Public InputMap RFE: https://bugs.openjdk.org/browse/JDK-8314968 [5] Documenting behaviors (WIP): https://github.com/openjdk/jfx/tree/master/doc-files/behavior [6] Earlier proposal: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/InputMap/BehaviorInputMapProposal.md [7] Earlier proposal in the OpenJFX mailing list: https://mail.openjdk.org/pipermail/openjfx-dev/2023-September/042819.html [8] RichTextArea (Incubator) https://github.com/andy-goryachev-oracle/Test/blob/rich.jep.review/doc/RichTextArea/RichTextArea.md
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v23]
On Thu, 25 Apr 2024 21:41:14 GMT, Oliver Kopp wrote:
>> Fixes https://bugs.openjdk.org/browse/JDK-8330462.
>>
>> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`,
>> then an addition of `start` to it leads to a negative value. This is "fixed"
>> by using `Math.max` comparing the `maxLength` and `maxLength + start`.
>
> Oliver Kopp has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Make use of Utils.clamp function
(I switched to TextField; is a bit easier for me to debug)
Intermedidate testing result:
When pressing "space", the rectangle moves correctly. When moving the cursor
"inside" these much spaces, it also moves.
---
Ctrl+A leads to the box borders be of length of the complete text, not the
visible text
---
I thought, following would help, but does not:
*/
public void selectRange(int anchor, int caretPosition) {
caretPosition = Utils.clamp(0, caretPosition, getLength());
anchor = Utils.clamp(0, anchor, getLength());
+notifyAccessibleAttributeChanged(AccessibleAttribute.SELECTION_START);
+notifyAccessibleAttributeChanged(AccessibleAttribute.SELECTION_END);
+notifyAccessibleAttributeChanged(AccessibleAttribute.CARET_OFFSET);
+
TextFormatter.Change change = new TextFormatter.Change(this,
getFormatterAccessor(), anchor, caretPosition);
TextFormatter<
-
PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078282435
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v23]
On Thu, 25 Apr 2024 21:41:14 GMT, Oliver Kopp wrote: >> Fixes https://bugs.openjdk.org/browse/JDK-8330462. >> >> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, >> then an addition of `start` to it leads to a negative value. This is "fixed" >> by using `Math.max` comparing the `maxLength` and `maxLength + start`. > > Oliver Kopp has updated the pull request incrementally with one additional > commit since the last revision: > > Make use of Utils.clamp function I tested with Oracle OpenJDK 22.0.1 and JavaFX 22.0.1+7 Tested moving the cursor around but the narrator focus wont change. - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078260806
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v23]
> Fixes https://bugs.openjdk.org/browse/JDK-8330462. > > If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, then > an addition of `start` to it leads to a negative value. This is "fixed" by > using `Math.max` comparing the `maxLength` and `maxLength + start`. Oliver Kopp has updated the pull request incrementally with one additional commit since the last revision: Make use of Utils.clamp function - Changes: - all: https://git.openjdk.org/jfx/pull/1442/files - new: https://git.openjdk.org/jfx/pull/1442/files/527e1ecf..968e3f05 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1442=22 - incr: https://webrevs.openjdk.org/?repo=jfx=1442=21-22 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jfx/pull/1442.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1442/head:pull/1442 PR: https://git.openjdk.org/jfx/pull/1442
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v22]
On Thu, 25 Apr 2024 20:09:53 GMT, Carl Christian Snethlage wrote: > Tried on my local machine (win10) w/o patch with narrator: Thank you for checking this out! so you see the windows cursor decouple from the narrator focus rectangle? And when you press left/right, the narrator focus does not change? just to confirm, you are running the latest master branch, or if now, which version of jfx/jdk? - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078178436
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v22]
On Thu, 25 Apr 2024 19:54:37 GMT, Oliver Kopp wrote: > ... and no luck with Eclipse ... I assume you imported the whole jfx repository into Eclipse as a gradle build as described here https://wiki.openjdk.org/display/OpenJFX/Using+an+IDE#UsinganIDE-UsingEclipse If not, you could still use command line to build and execute tests, and that is described here https://wiki.openjdk.org/display/OpenJFX/Building+OpenJFX I admit, there is a bit of pain configuring Eclipse - the main build uses gradle and Eclipse has other ideas when it sees a gradle project, but once you figure it out (I can help) you can debug in eclipse and it is much easier, in my opinion. On the other hand, we could completely take the IDE out of the picture and just run the command line to build and test. - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078173790
[jfx22u] Integrated: 8322251: [Linux] JavaFX is not displaying CJK on Ubuntu 23.10 and later
On Thu, 25 Apr 2024 19:28:47 GMT, Phil Race wrote: > Backport to jfx22u This pull request has now been integrated. Changeset: 77e7e251 Author:Phil Race URL: https://git.openjdk.org/jfx22u/commit/77e7e2514c25fa5f653938a6d0cbd4b1b6abe74f Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod 8322251: [Linux] JavaFX is not displaying CJK on Ubuntu 23.10 and later Backport-of: 5182ea16ace78c4f61e2c38981aab62f6153294e - PR: https://git.openjdk.org/jfx22u/pull/27
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v22]
On Thu, 25 Apr 2024 19:39:02 GMT, Oliver Kopp wrote: >> Fixes https://bugs.openjdk.org/browse/JDK-8330462. >> >> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, >> then an addition of `start` to it leads to a negative value. This is "fixed" >> by using `Math.max` comparing the `maxLength` and `maxLength + start`. > > Oliver Kopp 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 30 additional > commits since the last revision: > > - Merge remote-tracking branch 'upstream/master' into patch-1 > - Switch from `@EnabledOnOS` to `assumeTrue` > - Re-order for more clearness > - Fix tests > - Remove unused import > - classpath > - Fix JavaDoc formatting > - Discard changes to modules/javafx.graphics/src/test/addExports > - Add missing exports > - Try to use StubToolkit > - ... and 20 more: https://git.openjdk.org/jfx/compare/a7c1c59f...527e1ecf Tried on my local machine (win10) w/o patch with narrator:  - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078090611
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v22]
On Thu, 25 Apr 2024 19:39:02 GMT, Oliver Kopp wrote: >> Fixes https://bugs.openjdk.org/browse/JDK-8330462. >> >> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, >> then an addition of `start` to it leads to a negative value. This is "fixed" >> by using `Math.max` comparing the `maxLength` and `maxLength + start`. > > Oliver Kopp 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 30 additional > commits since the last revision: > > - Merge remote-tracking branch 'upstream/master' into patch-1 > - Switch from `@EnabledOnOS` to `assumeTrue` > - Re-order for more clearness > - Fix tests > - Remove unused import > - classpath > - Fix JavaDoc formatting > - Discard changes to modules/javafx.graphics/src/test/addExports > - Add missing exports > - Try to use StubToolkit > - ... and 20 more: https://git.openjdk.org/jfx/compare/15c0f043...527e1ecf ... and no luck with Eclipse ... WARNING: package test.com.sun.javafx.scene.control.test not in javafx.controls Graphics Device initialization failed for : d3d, sw Error initializing QuantumRenderer: no suitable pipeline found - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078066723
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v22]
> Fixes https://bugs.openjdk.org/browse/JDK-8330462. > > If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, then > an addition of `start` to it leads to a negative value. This is "fixed" by > using `Math.max` comparing the `maxLength` and `maxLength + start`. Oliver Kopp 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 30 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into patch-1 - Switch from `@EnabledOnOS` to `assumeTrue` - Re-order for more clearness - Fix tests - Remove unused import - classpath - Fix JavaDoc formatting - Discard changes to modules/javafx.graphics/src/test/addExports - Add missing exports - Try to use StubToolkit - ... and 20 more: https://git.openjdk.org/jfx/compare/5d390712...527e1ecf - Changes: - all: https://git.openjdk.org/jfx/pull/1442/files - new: https://git.openjdk.org/jfx/pull/1442/files/ade506c0..527e1ecf Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1442=21 - incr: https://webrevs.openjdk.org/?repo=jfx=1442=20-21 Stats: 285 lines in 4 files changed: 188 ins; 62 del; 35 mod Patch: https://git.openjdk.org/jfx/pull/1442.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1442/head:pull/1442 PR: https://git.openjdk.org/jfx/pull/1442
[jfx22u] RFR: 8322251: [Linux] JavaFX is not displaying CJK on Ubuntu 23.10 and later
Backport to jfx22u - Commit messages: - Backport 5182ea16ace78c4f61e2c38981aab62f6153294e Changes: https://git.openjdk.org/jfx22u/pull/27/files Webrev: https://webrevs.openjdk.org/?repo=jfx22u=27=00 Issue: https://bugs.openjdk.org/browse/JDK-8322251 Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jfx22u/pull/27.diff Fetch: git fetch https://git.openjdk.org/jfx22u.git pull/27/head:pull/27 PR: https://git.openjdk.org/jfx22u/pull/27
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v21]
On Thu, 25 Apr 2024 18:03:17 GMT, Andy Goryachev wrote: > focus rectangle follows the windows cursor and announces the letter at the > cursor. I did not change branches, but downloaded a binary from the net. I type "abcdef" in the text field and then press cursor left - no change of the letter. Maybe, this behavior was fixed in the main branch meanwhile. The binary uses JavaFX 21. - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2078027281
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v21]
On Thu, 25 Apr 2024 06:37:00 GMT, Oliver Kopp wrote: >> Fixes https://bugs.openjdk.org/browse/JDK-8330462. >> >> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, >> then an addition of `start` to it leads to a negative value. This is "fixed" >> by using `Math.max` comparing the `maxLength` and `maxLength + start`. > > Oliver Kopp has updated the pull request incrementally with one additional > commit since the last revision: > > Switch from `@EnabledOnOS` to `assumeTrue` Ok, let's try to narrow down the issue, using the sample TextAreaTest_Narrator_8330462 (it does not matter whether we have a TextArea or a TextField there). In the master branch, with the Narrator (N.) enabled, the first thing I see it that the whole text is highlighted and the N. reads it. When the user navigates using left/right arrow keys, the N. focus rectangle follows the windows cursor and announces the letter at the cursor. With the fix, the windows cursor moves, but the N. rectangle does not move, and that is the issue. Do you see the same with and without your fix (making sure you do a clean build when switching branches)? - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2077857093
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v20]
On Thu, 25 Apr 2024 15:37:09 GMT, Andy Goryachev wrote: > Nope, this fix breaks Narrator. I think, I do not get what Narrator is doing. If I type "Testx" into a Text field, what should be highlighted?  If moving the cursor in the tool [javafxreproducer](https://github.com/Siedlerchr/javafxreproducer) also always the last letter is highlighted. Does my patch sneakly sneak in in all JavaFX code I am running on my machine? - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2077813261
Re: RFR: 8273657 : TextField: all text content must be selected initially
On Thu, 25 Apr 2024 14:26:06 GMT, Karthik P K wrote: > The text was not getting selected on adding the `TextField` to the scene > initially, subsequently removing and adding the `TextField` to the scene > selects the entire text present in the `TextField`. > > Made changes in the `TextFieldBehavior` constructor to select the text on > adding the `TextField`. > > Added unit test to validate the fix the fix looks good. - Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1446#pullrequestreview-2023008742
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v7]
On Thu, 25 Apr 2024 12:55:48 GMT, Oliver Kopp wrote:
> Can you help me
Sure! So I copied the test class (renamed, since we have another one with the
name of TextAreaTest to `tests\system\src\test\java\test\com\sun\glass\ui\win`,
same dir where WinTextRangeProviderTest.java resides. Here is the file:
package test.com.sun.glass.ui.win;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.TextArea;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
public class TextAreaTest_Narrator_8330462 extends Application {
public static void main(String[] args) throws Throwable {
launch(args);
}
@Override
public void start(Stage primaryStage) {
primaryStage.setScene(new Scene(new VBox(new TextArea("javafx test";
primaryStage.show();
}
}
then, launch it in Eclipse. this creates a run configuration, but fails to
launch.
Edit the run configuration - select [Dependencies] tab, click on [Override
Dependencies] button and replace the text in the bottom area with this:
--add-modules=javafx.base,javafx.graphics,javafx.controls,javafx.swing
--add-opens javafx.controls/test.com.sun.javafx.scene.control.test=javafx.base
--add-exports javafx.base/com.sun.javafx=ALL-UNNAMED
-Djava.library.path="../../../../build/sdk/lib"
like so:
then run again. this time it *should* launch. ;-)
-
PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2077616263
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v20]
On Thu, 25 Apr 2024 06:49:24 GMT, Oliver Kopp wrote: > I tested it with a JFX distribution without the fix. Also happens there. Nope, this fix breaks Narrator. The window cursor is moving but the narrator outlines the trailing 't' as Ambarish described. (every time I change branches for the purposes of review, I do a complete gradle build preceded by nuking the build directory: rm -rf ./build ; gradle clean sdk apps javadoc ```) - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2077595117
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote:
> JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are
> no longer needed.
>
> In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I
> changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it
> doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was
> leveraged to transparently use the Ex device in the backend) but now we don't
> have the non-Ex device, so that keeps it a bit more consistent and clear IMO.
>
> Verified by running tests on Windows 11, did not notice any regressions.
> Unfortunately I have no way to test this on older systems.
Tested the functionality with the 3DLighting app, things look the same as
before the patch. Left a minor comment.
modules/javafx.graphics/src/main/native-prism-d3d/D3DPipeline.cc line 237:
> 235: }
> 236:
> 237: int getMaxSampleSupport(IDirect3D9Ex *d3d9, UINT adapter) {
Minor: In some cases you also change the name of the variable to add the "Ex"
suffix., like in
D3DContext::D3DContext(IDirect3D9Ex *pd3dEx, UINT adapter)
^
Here and In `PiplineManager.h` it's left as `IDirect3D9Ex * pd3d9;` without
"Ex".
I don't mind it either way (I would probably not bother changing them myself),
but perhaps we should remain consistent.
-
Marked as reviewed by nlisker (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1445#pullrequestreview-2022879147
PR Review Comment: https://git.openjdk.org/jfx/pull/1445#discussion_r1579699815
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote: > JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are > no longer needed. > > In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I > changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it > doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was > leveraged to transparently use the Ex device in the backend) but now we don't > have the non-Ex device, so that keeps it a bit more consistent and clear IMO. > > Verified by running tests on Windows 11, did not notice any regressions. > Unfortunately I have no way to test this on older systems. Good to hear. - PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2077410544
RFR: 8273657 : TextField: all text content must be selected initially
The text was not getting selected on adding the `TextField` to the scene initially, subsequently removing and adding the `TextField` to the scene selects the entire text present in the `TextField`. Made changes in the `TextFieldBehavior` constructor to select the text on adding the `TextField`. Added unit test to validate the fix - Commit messages: - Fix textfield selection issue Changes: https://git.openjdk.org/jfx/pull/1446/files Webrev: https://webrevs.openjdk.org/?repo=jfx=1446=00 Issue: https://bugs.openjdk.org/browse/JDK-8273657 Stats: 25 lines in 2 files changed: 25 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jfx/pull/1446.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1446/head:pull/1446 PR: https://git.openjdk.org/jfx/pull/1446
Re: RFR: 8329820: [Linux] Prefer EGL over GLX [v3]
On Fri, 19 Apr 2024 14:42:23 GMT, Thiago Milczarek Sayao
wrote:
>> Wayland implementation will require EGL.
>>
>> EGL works with Xorg as well. The idea is to be EGL first and if it fails,
>> fallback to GLX. A force flag `prism.es2.forceGLX=true` is available.
>>
>>
>> See:
>> [Switching the Linux graphics stack from GLX to
>> EGL](https://mozillagfx.wordpress.com/2021/10/30/switching-the-linux-graphics-stack-from-glx-to-egl/)
>> [Prefer EGL to GLX for the GL support on
>> X11](https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3540)
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Forgot debug message
I left a few comments, mostly for adjusting error-checking. Once those are
fixed and others have their input I'll test those changes.
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c
line 252:
> 250: eglGetProcAddress("glGetProgramInfoLog");
> 251: ctxInfo->glTexImage2DMultisample = (PFNGLTEXIMAGE2DMULTISAMPLEPROC)
> 252: dlsym(RTLD_DEFAULT,"glTexImage2DMultisample");
I think these three functions should also be available via `eglGetProcAddress`
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c
line 258:
> 256: dlsym(RTLD_DEFAULT,"glBlitFramebuffer");
> 257:
> 258: eglSwapInterval(eglDisplay, 0);
`eglSwapInterval` can potentially fail, we should check for errors here
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c
line 267:
> 265:
> 266: // Release context once we are all done
> 267: eglMakeCurrent(eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE,
> EGL_NO_CONTEXT);
Similar as above, `eglMakeCurrent` can also fail
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c
line 301:
> 299:
> 300: if (!eglMakeCurrent(ctxInfo->eglDisplay, dInfo->eglSurface,
> dInfo->eglSurface, ctxInfo->context)) {
> 301: fprintf(stderr, "Failed in eglMakeCurrent");
I think you might be missing a `return` statement here.
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c
line 310:
> 308: interval = (vSyncNeeded) ? 1 : 0;
> 309: ctxInfo->state.vSyncEnabled = vSyncNeeded;
> 310: eglSwapInterval(ctxInfo->eglDisplay, interval);
Check for errors like above
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLDrawable.c
line 58:
> 56: }
> 57:
> 58: EGLSurface eglSurface = eglCreateWindowSurface(pfInfo->eglDisplay,
> pfInfo->eglConfig,
I would add a check to make sure the surface was created successfully, as it
can potentially return `EGL_NO_SURFACE`.
Some additional information from `eglGetError` could also come in handy if
`eglCreateWindowSurface` does fail.
modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLDrawable.c
line 119:
> 117:
> 118: eglSwapBuffers(eglGetCurrentDisplay(), dInfo->eglSurface);
> 119: return JNI_TRUE;
eglSwapBuffers can fail, so I'd `return (eglSwapBuffers(...) == EGL_TRUE);` or
something similar
-
PR Review: https://git.openjdk.org/jfx/pull/1381#pullrequestreview-2022336712
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579372523
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579386732
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579387499
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579431452
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579387989
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579376454
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579379362
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote: > JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are > no longer needed. > > In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I > changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it > doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was > leveraged to transparently use the Ex device in the backend) but now we don't > have the non-Ex device, so that keeps it a bit more consistent and clear IMO. > > Verified by running tests on Windows 11, did not notice any regressions. > Unfortunately I have no way to test this on older systems. Found the problem. That merge bumped the min JDK to 21, which I was using anyway, but it required recompiling the native D3D resources with the new JDK. The application works now. - PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2077287504
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote: > JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are > no longer needed. > > In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I > changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it > doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was > leveraged to transparently use the Ex device in the backend) but now we don't > have the non-Ex device, so that keeps it a bit more consistent and clear IMO. > > Verified by running tests on Windows 11, did not notice any regressions. > Unfortunately I have no way to test this on older systems. > I traced the issue to commit id > [3914db2](https://github.com/openjdk/jfx/commit/3914db26f3abb573ed0e320a361477e1d3e7a9ac), > which is a merge Kevin did ~6 weeks ago. Placing the head on this commit or > after causes the above error, but moving 1 commit back to "Create release > notes for JavaFX 22" lets the application start normally. That merge was to bring in the CPU fixes for the April CPU. This could indicate a problem with one of the fixes. How easy is it to reproduce? Does it reproduce on startup or when you turn on lighting of some sort? Can you reproduce this on more than one system? - PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2077286537
Integrated: 8146918: ConcurrentModificationException in MediaPlayer
On Thu, 22 Feb 2024 17:16:42 GMT, n-gabe wrote: > There is a ConcurrentModificationException in MediaPlayer when removing a > MediaView from it. The root cause is that you can't iterate over a HashSet > with for (WeakReference vref : viewRefs) and removing items from > the collection by viewRefs.remove(vref); within this loop. This pull request has now been integrated. Changeset: d8ca38a6 Author:n-gabe <11182122+n-g...@users.noreply.github.com> Committer: Kevin Rushforth URL: https://git.openjdk.org/jfx/commit/d8ca38a6b7ed918318b956add150a5ae9c4c0981 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod 8146918: ConcurrentModificationException in MediaPlayer Reviewed-by: almatvee - PR: https://git.openjdk.org/jfx/pull/1377
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v7]
On Fri, 19 Apr 2024 17:33:39 GMT, Andy Goryachev wrote: >> I am new to testing in the JFX project. It seems that `test.` is required as >> package prefix. Thus, I did not use the approach of package-private methods >> and classes, but needed to make the class and the tested method public. >> >> However, now I get the (expected) error that the module `javafx.graphics` >> does not export `com.sun.glass.ui.win` to unnamed module @0x1e6454ec. >> >> >> class test.com.sun.glass.ui.win.WinTextRangeProviderTest (in unnamed module >> @0x1e6454ec) cannot access class com.sun.glass.ui.win.WinTextRangeProvider >> (in module javafx.graphics) because module javafx.graphics does not export >> com.sun.glass.ui.win to unnamed module @0x1e6454ec >> java.lang.IllegalAccessError: class >> test.com.sun.glass.ui.win.WinTextRangeProviderTest (in unnamed module >> @0x1e6454ec) cannot access class com.sun.glass.ui.win.WinTextRangeProvider >> (in module javafx.graphics) because module javafx.graphics does not export >> com.sun.glass.ui.win to unnamed module @0x1e6454ec >> at >> test.com.sun.glass.ui.win.WinTextRangeProviderTest.getValidStringIndex > >> module `javafx.graphics` does not export `com.sun.glass.ui.win` to unnamed >> module @0x1e6454ec. > > I think you need to add > > --add-exports javafx.graphics/com.sun.glass.ui.win=ALL-UNNAMED > > to graphics/src/test/addExports > (see line 7) > > @kevinrushforth I wonder if there was a way to auto-generate addExports > somehow, at least the part needed for the tests. I put the `TextAreaTest.java` into `tests\system\src\test\java\test\com\sun\glass\ui\win`. Cannot get it into run in Eclipse. Can you help me @andy-goryachev-oracle (refs https://github.com/openjdk/jfx/pull/1442#discussion_r1578005144). Background: To work on the issue mentioned by Ambarish, I need to do code and fix. Cleanroom development takes too much time for me here. - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2077116786
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote: > JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are > no longer needed. > > In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I > changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it > doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was > leveraged to transparently use the Ex device in the backend) but now we don't > have the non-Ex device, so that keeps it a bit more consistent and clear IMO. > > Verified by running tests on Windows 11, did not notice any regressions. > Unfortunately I have no way to test this on older systems. I traced the issue to commit id 3914db26f3abb573ed0e320a361477e1d3e7a9ac, which is a merge Kevin did ~6 weeks ago. Placing the head on this commit or after causes the above error, but moving 1 commit back to "Create release notes for JavaFX 22" lets the application start normally. I understand that this PR has nothing to do with this, I just can't test it considering this problem. - PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2076940263
Re: RFR: 8320563: Remove D3D9 code paths in favor of D3D9Ex
On Tue, 23 Apr 2024 10:33:58 GMT, Lukasz Kostyra wrote: > JFX minimum requirements guarantee 9Ex availability, so old non-Ex paths are > no longer needed. > > In multiple parts (ex. Mesh, Graphics, etc.) where the Device is acquired I > changed the type to explicitly use `IDirect3DDevice9Ex`. Technically it > doesn't matter much (`IDirect3DDevice9Ex` inherits `IDirect3DDevice` - it was > leveraged to transparently use the Ex device in the backend) but now we don't > have the non-Ex device, so that keeps it a bit more consistent and clear IMO. > > Verified by running tests on Windows 11, did not notice any regressions. > Unfortunately I have no way to test this on older systems. I tried to run the 3DLighting application and I'm getting an error. However, it looks like it's not this patch that is causing it since going back a few commits also gives this error. The jfx22 branch doesn't have this issue so it will take some investigation to find out what's going on. I'm on Win 10. # # A fatal error has been detected by the Java Runtime Environment: # # EXCEPTION_ACCESS_VIOLATION (0xc005) at pc=0x01b44b9ad260, pid=6332, tid=18096 # # JRE version: OpenJDK Runtime Environment (21.0.1+12) (build 21.0.1+12-29) # Java VM: OpenJDK 64-Bit Server VM (21.0.1+12-29, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, windows-amd64) # Problematic frame: # J 170 c2 java.lang.String.length()I java.base@21.0.1 (11 bytes) @ 0x01b44b9ad260 [0x01b44b9ad220+0x0040] # # No core dump will be written. Minidumps are not enabled by default on client versions of Windows # # An error report file with more information is saved as: # C:\Users\Nir\git\jfx\tests\performance\3DLighting\hs_err_pid6332.log [5.830s][warning][os] Loading hsdis library failed # # If you would like to submit a bug report, please visit: # https://bugreport.java.com/bugreport/crash.jsp # - PR Comment: https://git.openjdk.org/jfx/pull/1445#issuecomment-2076903265
Update on the headless platform (sandbox)
Hi, I did some more work on the JavaFX Headless platform that is available in [1] and I am using it to run the systemtests. I am not yet running the robot-based systemtests, but there is already some robot code in the Headless platform. The other systemtests are going pretty well. On my linux, export _JAVA_OPTIONS="-Dglass.platform=Headless" sh gradlew --info -PFULL_TEST=true :systemTests:cleanTest :systemTests:test is currently returning 552 tests completed, 3 failed, 24 skipped I understand why the 3 failed tests are failing, and I will move to robot-based tests soon. [1] https://github.com/openjdk/jfx-sandbox/tree/johanvos-headless
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v20]
On Wed, 24 Apr 2024 15:28:30 GMT, Ambarish Rapte wrote: > Windows Narrator reads only the last character of the Text in a TextArea, > when moving the cursor, and the focus rect does not move with cursor. I tested it with a JFX distribution without the fix. Also happens there. Can you check, too? - If yes, would it be possible to handle this in a separate issue? - PR Comment: https://git.openjdk.org/jfx/pull/1442#issuecomment-2076488937
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v15]
On Wed, 24 Apr 2024 15:26:27 GMT, Kevin Rushforth wrote: >> Interesting = @kevinrushforth what do you think? > > An excellent question. If it is robust, then it seems OK. Using Junit5 > Assumptions is more flexible, though, so might lean toward wanting to use > that. Either way, test this on the other platforms to ensure that the test is > skipped (and marked as skipped in the report). +1 for flexibility. Switched in [`ade506c` (#1442)](https://github.com/openjdk/jfx/pull/1442/commits/ade506c0e44544f3d61a164765e19b983a99373d). Note that `assumeTrue` also works in `@BeforeAll` to skip the whole class. (I wonder whether I should rewrite the other tests following that pattern, but that is maybe a job for a follow-up issue) - PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1578941716
Re: RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v21]
> Fixes https://bugs.openjdk.org/browse/JDK-8330462. > > If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, then > an addition of `start` to it leads to a negative value. This is "fixed" by > using `Math.max` comparing the `maxLength` and `maxLength + start`. Oliver Kopp has updated the pull request incrementally with one additional commit since the last revision: Switch from `@EnabledOnOS` to `assumeTrue` - Changes: - all: https://git.openjdk.org/jfx/pull/1442/files - new: https://git.openjdk.org/jfx/pull/1442/files/1159e3dd..ade506c0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1442=20 - incr: https://webrevs.openjdk.org/?repo=jfx=1442=19-20 Stats: 6 lines in 1 file changed: 3 ins; 3 del; 0 mod Patch: https://git.openjdk.org/jfx/pull/1442.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1442/head:pull/1442 PR: https://git.openjdk.org/jfx/pull/1442
