Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v3]
On Mon, 1 Jul 2024 20:43:44 GMT, Andy Goryachev wrote: > Kevin is right, this fix does not solve the issue mentioned in the ticket. > Once the fxPanel is added back, its content is not visible. Does not matter > whether removing/adding happens at startup or the button event handler. > > (attaching a slightly modified test case to the ticket, notice lines 30 and > 44. > > Also, I think swing requires validate() and repaint() called after modifying > the component's children. OK..I guess I probably misunderstood the expectation...it says EXPECTED - The JFXPanel is visible to the user of the application and no Exceptions are thrown. ACTUAL - The JFXPanel is visible but the following Exception is thrown which I deciphered as the JFXPanel window being visible and I guess in original testcase execution, the "TestButton" was still visible, only NPE was thrown.. ANyway, I guess it was mentioned that "We should file a follow-on Enhancement to consider doing this" and regarding synchro mentioned at line58, I guess we already have `QuantumToolkit.runWithRenderLock` in the problematic code - PR Comment: https://git.openjdk.org/jfx/pull/1493#issuecomment-2201826620
Re: RFR: 8326712: Robot tests fail on XWayland [v2]
On Fri, 28 Jun 2024 18:12:37 GMT, Alexander Zvegintsev wrote: >> Most of the headful test failures on XWayland are due to screen capture is >> not working. >> >> Wayland, unlike X11, does not allow arbitrary applications to capture the >> screen contents directly. >> Instead, screen capture functionality is managed by the compositor, which >> can enforce stricter controls and permissions, requiring explicit user >> approval for screen capture operations. >> >> This issue is already resolved in OpenJDK ([base >> issue](https://bugs.openjdk.org/browse/JDK-8280982), there are subsequent >> fixes) by using the [ScreenCast XDG >> portal](https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.ScreenCast.html). >> >> The XDG ScreenCast portal is utilized for capturing screen data as it >> provides a secure and standardized method for applications to access screen >> content. >> Additionally, it allows for the reuse of user permissions without requiring >> repeated confirmations, streamlining the user experience and enhancing >> convenience. >> >> >> >> >> So this changeset is a copy of the OpenJDK fixes with the addition of the >> JavaFX customization. >> For ease of review, you can skip the changes in the first two commits: >> - First commit is a direct copy of files from OpenJDK >> - Second commit removes the `gtk-` prefix before the `gtk_...` and `g_...` >> function calls (in AWT all gtk functions are dynamically loaded, we don't >> need that in JavaFX). >> >> properties added: >> >> - `javafx.robot.screenshotMethod`, accepts `gtk`(existing gtk method) and >> `dbusScreencast`(added by this changeset, used by default for Wayland) >> - `javafx.robot.screenshotDebug` prints debug info if it is set to `true` >> >> >> >> What are the remaining issues? >> >> 1. https://bugs.openjdk.org/browse/JDK-8335468 >> >> After applying this fix, system tests will pass except for the >> `SwingNodeJDialogTest` test. >> >> This interop test calls `java.awt.Robot#getPixelColor` which internally >> `gtk->g_main_context_iteration(NULL, TRUE);` causes a blocking of javafx gtk >> loop, so the test hangs. >> So a change is required on OpenJDK side to fix this issue. >> >> 2. https://bugs.openjdk.org/browse/JDK-8335470 >> >> Even after solving the `#1`, the >> `SwingNodeJDialogTest.testNodeRemovalBeforeShow` case is still failing. >> >> 3. https://bugs.openjdk.org/browse/JDK-8335469 >> >> Internally the ScreenCast session keeps open for >> [2s](https://github.com/openjdk/jdk/blob/d457609f700bbb1fed233f1a04501c995852e5ac/src/java.desktop/unix/classes/sun/awt/screencast/ScreencastHelper.java#L62). > ... > > Alexander Zvegintsev has updated the pull request incrementally with two > additional commits since the last revision: > > - do not throw SecurityException > - Revert "remove HEADLESS check" The code changes all look good. I've done a fair bit of testing already, and will finish up my testing tomorrow. - PR Comment: https://git.openjdk.org/jfx/pull/1490#issuecomment-2201171104
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v3]
On Mon, 1 Jul 2024 09:16:52 GMT, Prasanta Sadhukhan wrote: >> Adding, then removing, and then adding a JFXPanel to the same component in >> the Swing scene graph leads to a NullPointerException in GlassScene because >> the sceneState is null. >> Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which >> calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose >> -> GlassScene.dispose which sets "sceneState" to null... >> so when GlassScene.updateSceneState is called, it results in NPE. >> Fix is to check if `host` (which is usually >> javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been >> reset/deleted which is done when `EmbeddedScene.dispose` is called during >> removeNotify and abstain from updating the scene state > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Test fix tests/system/src/test/java/test/javafx/embed/swing/JFXPanelNPETest.java line 106: > 104: frame.remove(fxPanel); > 105: // fxPanel added to frame again > 106: frame.add(fxPanel); // <-- leads to NullPointerException You can use JUnit5 to write this test (class) and use `assertDoesNotThrow(() -> frame.add(fxPanel));`, which makes this a bit better to understand - PR Review Comment: https://git.openjdk.org/jfx/pull/1493#discussion_r1661580655
Re: Should we document Styleable properties?
Further testing shows that while the javadoc tool does support custom tags, it doesn’t lift custom tags defined on JavaFX property fields to the property method documentation. Lifting documentation from the property field to property methods is a JavaFX-specific feature of Javadoc. At first glance, there seems to be no particular reason why custom tags are ignored in this situation; probably it just wasn’t implemented. So if we add this custom tag, it won’t show up on the generated HTML. We could start using a custom tag, and then file an enhancement request for Javadoc to lift custom tags to methods just as it does with other tags. What’s your opinion on that? Andy Goryachev schrieb am Mo. 1. Juli 2024 um 23:20: > Would even work with Eclipse out of the box: > > > > > > I also like the fact that we won't need to maintain links manually as > there would not be any. > > > > -andy >
Re: Should we document Styleable properties?
Sure, something like this would be possible if it helps minimize
boilerplate.
-- Kevin
On 7/1/2024 1:58 PM, Michael Strauß wrote:
The javadoc tool already supports custom tags out of the box with the
"-tag" command line option. For example, adding this line in the
gradle javadoc task (build.gradle L4241) would introduce a custom tag:
options.tags("styleableProperty:a:CSS property name:")
Of course, there's no special processing of custom tags, so we would
have to add a link manually if we want one. But that's certainly not a
worse situation compared to adding a manual link in prose text.
On Mon, Jul 1, 2024 at 9:16 PM Andy Goryachev wrote:
Thank you for your feedback, Michael!
Let me first make sure I understand your suggestion correctly:
add an annotation, let's say @css-prop
modify javadoc tool to create an automated link from the property name to a
place in cssref.html
javadoc will render this as
@css-prop this property can be styled with CSS using -fx-prop-name
I don't know whether javadoc people will be happy about this idea: javadoc is a
part of jdk and unfortunately javafx is not, though javadoc does offer certain
features to make it play nice with javafx properties.
This would also require non-trivial modification to cssref.html to add anchors where each property
name is defined. Alternatively, it could simply point to a class, for which we do have an id (e.g.
Cell)
Overall, it is much more extensive | expensive proposition with external
dependencies. Meaning the probability of it happening is very low.
Having said that, this is a great idea, very developer-friendly.
-andy
Re: Should we document Styleable properties?
The javadoc tool already supports custom tags out of the box with the
"-tag" command line option. For example, adding this line in the
gradle javadoc task (build.gradle L4241) would introduce a custom tag:
options.tags("styleableProperty:a:CSS property name:")
Of course, there's no special processing of custom tags, so we would
have to add a link manually if we want one. But that's certainly not a
worse situation compared to adding a manual link in prose text.
On Mon, Jul 1, 2024 at 9:16 PM Andy Goryachev wrote:
>
> Thank you for your feedback, Michael!
>
>
>
> Let me first make sure I understand your suggestion correctly:
>
>
>
> add an annotation, let's say @css-prop
> modify javadoc tool to create an automated link from the property name to a
> place in cssref.html
> javadoc will render this as
>
>
>
> @css-prop this property can be styled with CSS using href="...">-fx-prop-name
>
>
>
> I don't know whether javadoc people will be happy about this idea: javadoc is
> a part of jdk and unfortunately javafx is not, though javadoc does offer
> certain features to make it play nice with javafx properties.
>
>
>
> This would also require non-trivial modification to cssref.html to add
> anchors where each property name is defined. Alternatively, it could simply
> point to a class, for which we do have an id (e.g. Cell)
>
>
>
> Overall, it is much more extensive | expensive proposition with external
> dependencies. Meaning the probability of it happening is very low.
>
>
>
> Having said that, this is a great idea, very developer-friendly.
>
>
>
> -andy
Re: Should we document Styleable properties?
A new javadoc tag seems unlikely. Especially for something like this
where the ask would be to automatically link it to some html file
(cssref.html) somewhere in the current JavaFX sources.
One more thing to point out, is that even if we could get such a tag
added to the JDK javadoc doclet, it would be quite a while before we
could use it in JavaFX. We would have to get it into JDK 24 or 25 and
then wait until we update to that JDK as the minimum needed to build
JavaFX. So maybe in JavaFX 27 we could start using it.
So it's a nice idea, but doesn't seem practical to implement.
-- Kevin
On 7/1/2024 12:16 PM, Andy Goryachev wrote:
Thank you for your feedback, Michael!
Let me first make sure I understand your suggestion correctly:
1. add an annotation, let's say @css-prop
2. modify javadoc tool to create an automated link from the property
name to a place in cssref.html
3. javadoc will render this as
@css-prop this property can be styled with CSS using href="...">-fx-prop-name
I don't know whether javadoc people will be happy about this idea:
javadoc is a part of jdk and unfortunately javafx is not, though
javadoc does offer certain features to make it play nice with javafx
properties.
This would also require non-trivial modification to cssref.html to add
anchors where each property name is defined. Alternatively, it could
simply point to a class, for which we do have an id (e.g. id="cell">Cell)
Overall, it is much more extensive | expensive proposition with
external dependencies. Meaning the probability of it happening is
very low.
Having said that, this is a great idea, very developer-friendly.
-andy
*From: *openjfx-dev on behalf of
Michael Strauß
*Date: *Monday, July 1, 2024 at 09:45
*To: *
*Cc: *openjfx-dev@openjdk.org
*Subject: *Re: Should we document Styleable properties?
If we do this, I suggest to add a custom tag for this purpose instead
of repeating prose for every property.
The javadoc tool will then render the content of this tag in the
specification list section of the documentation (where tags such as
@defaultValue will also appear).
On Fri, Jun 28, 2024 at 11:21 PM Andy Goryachev
wrote:
>
> Dear fellow developers:
>
>
>
> Should we document which properties are styleable with CSS in
javadoc? Would that be useful?
>
>
>
> Example:
>
>
>
> /**
>
> * Determines the caret blink period. This property cannot be
set to {@code null}.
>
> *
>
> * This is a {@link StyleableProperty} with the name {@code
-fx-caret-blink-period}.
>
> *
>
>
>
> alternative:
>
>
>
> * This property can be styled with CSS using {@code
-fx-caret-blink-period} name.
>
> * @implNote The property object implements {@link
StyleableProperty} interface.
>
>
>
>
>
> Other ideas are also welcome.
>
>
>
> -andy
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v3]
On Mon, 1 Jul 2024 09:16:52 GMT, Prasanta Sadhukhan wrote: >> Adding, then removing, and then adding a JFXPanel to the same component in >> the Swing scene graph leads to a NullPointerException in GlassScene because >> the sceneState is null. >> Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which >> calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose >> -> GlassScene.dispose which sets "sceneState" to null... >> so when GlassScene.updateSceneState is called, it results in NPE. >> Fix is to check if `host` (which is usually >> javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been >> reset/deleted which is done when `EmbeddedScene.dispose` is called during >> removeNotify and abstain from updating the scene state > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Test fix Kevin is right, this fix does not solve the issue mentioned in the ticket. Once the fxPanel is added back, its content is not visible. Does not matter whether removing/adding happens at startup or the button event handler. (attaching a slightly modified test case to the ticket, notice lines 30 and 44. Also, I think swing requires validate() and repaint() called after modifying the component's children. - PR Comment: https://git.openjdk.org/jfx/pull/1493#issuecomment-2200985904
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v3]
On Mon, 1 Jul 2024 09:16:52 GMT, Prasanta Sadhukhan
wrote:
>> Adding, then removing, and then adding a JFXPanel to the same component in
>> the Swing scene graph leads to a NullPointerException in GlassScene because
>> the sceneState is null.
>> Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which
>> calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose
>> -> GlassScene.dispose which sets "sceneState" to null...
>> so when GlassScene.updateSceneState is called, it results in NPE.
>> Fix is to check if `host` (which is usually
>> javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been
>> reset/deleted which is done when `EmbeddedScene.dispose` is called during
>> removeNotify and abstain from updating the scene state
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Test fix
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/EmbeddedScene.java
line 160:
> 158: Platform.runLater(() -> {
> 159: QuantumToolkit.runWithRenderLock(() -> {
> 160: if (host != null) {
I think we need a different solution here, as mentioned in line 58
// TODO: synchronize access to embedder from ET and RT
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1493#discussion_r1661510943
Re: Should we document Styleable properties?
Thank you for your feedback, Michael!
Let me first make sure I understand your suggestion correctly:
1. add an annotation, let's say @css-prop
2. modify javadoc tool to create an automated link from the property name to
a place in cssref.html
3. javadoc will render this as
@css-prop this property can be styled with CSS using -fx-prop-name
I don't know whether javadoc people will be happy about this idea: javadoc is a
part of jdk and unfortunately javafx is not, though javadoc does offer certain
features to make it play nice with javafx properties.
This would also require non-trivial modification to cssref.html to add anchors
where each property name is defined. Alternatively, it could simply point to a
class, for which we do have an id (e.g. Cell)
Overall, it is much more extensive | expensive proposition with external
dependencies. Meaning the probability of it happening is very low.
Having said that, this is a great idea, very developer-friendly.
-andy
From: openjfx-dev on behalf of Michael Strauß
Date: Monday, July 1, 2024 at 09:45
To:
Cc: openjfx-dev@openjdk.org
Subject: Re: Should we document Styleable properties?
If we do this, I suggest to add a custom tag for this purpose instead
of repeating prose for every property.
The javadoc tool will then render the content of this tag in the
specification list section of the documentation (where tags such as
@defaultValue will also appear).
On Fri, Jun 28, 2024 at 11:21 PM Andy Goryachev
wrote:
>
> Dear fellow developers:
>
>
>
> Should we document which properties are styleable with CSS in javadoc? Would
> that be useful?
>
>
>
> Example:
>
>
>
> /**
>
> * Determines the caret blink period. This property cannot be set to
> {@code null}.
>
> *
>
> * This is a {@link StyleableProperty} with the name {@code
> -fx-caret-blink-period}.
>
> *
>
>
>
> alternative:
>
>
>
> * This property can be styled with CSS using {@code
> -fx-caret-blink-period} name.
>
> * @implNote The property object implements {@link StyleableProperty}
> interface.
>
>
>
>
>
> Other ideas are also welcome.
>
>
>
> -andy
Re: Should we document Styleable properties?
Thank you for your feedback!
I agree: referring to places in cssref.html is a good idea. And perhaps even
referring from cssref.html to javadoc might be also a good idea, though
cssref.html does suggest the class which owns the corresponding properties, or
at least the class in question can be found in most cases.
-andy
From: openjfx-dev on behalf of Markus Mack
Date: Monday, July 1, 2024 at 04:48
To: openjfx-dev@openjdk.org
Subject: Re: Should we document Styleable properties?
I'd say this is a good idea. Both variants are good, we might want to
mention CSS in the first one, though.
Not sure how others do it, but I regularly struggle to find out which
code properties correspond to which CSS styles. This is particularly the
case for names like "background", "border", or "stroke", where there is
no clean 1:1 correspondence, or similar names are used in multiple contexts.
Some way of linking to the current CSS reference might also be helpful
to find out about allowed arguments. If I google something like "javafx
css" I typically only get links to outdated versions of the CSS
reference. Also, linking back from the CSS reference to the code
properties' javadoc pages is something I've been missing - maybe this
could be added without much additional effort if javadocs are updated
according to this proposal?
Am 28.06.2024 um 23:04 schrieb Andy Goryachev:
>
> Dear fellow developers:
>
> Should we document which properties are styleable with CSS in
> javadoc? Would that be useful?
>
> Example:
>
> /**
>
> * Determines the caret blink period. This property cannot be set
> to {@code null}.
>
> *
>
> * This is a {@link StyleableProperty} with the name {@code
> -fx-caret-blink-period}.
>
> *
>
> alternative:
>
> * This property can be styled with CSS using {@code
> -fx-caret-blink-period} name.
>
> * @implNote The property object implements {@link
> StyleableProperty} interface.
>
> Other ideas are also welcome.
>
> -andy
>
Re: Should we document Styleable properties?
If we do this, I suggest to add a custom tag for this purpose instead
of repeating prose for every property.
The javadoc tool will then render the content of this tag in the
specification list section of the documentation (where tags such as
@defaultValue will also appear).
On Fri, Jun 28, 2024 at 11:21 PM Andy Goryachev
wrote:
>
> Dear fellow developers:
>
>
>
> Should we document which properties are styleable with CSS in javadoc? Would
> that be useful?
>
>
>
> Example:
>
>
>
> /**
>
> * Determines the caret blink period. This property cannot be set to
> {@code null}.
>
> *
>
> * This is a {@link StyleableProperty} with the name {@code
> -fx-caret-blink-period}.
>
> *
>
>
>
> alternative:
>
>
>
> * This property can be styled with CSS using {@code
> -fx-caret-blink-period} name.
>
> * @implNote The property object implements {@link StyleableProperty}
> interface.
>
>
>
>
>
> Other ideas are also welcome.
>
>
>
> -andy
Re: RFR: 8325445: [macOS] Colors are not displayed in sRGB color space [v2]
> When drawing to the screen JavaFX is producing sRGB colors but on macOS > that’s not necessarily what the user is seeing. Since the pixels are not > tagged as sRGB the OS is copying them unmodified to the frame buffer to be > displayed in the screen’s color space. In general Mac’s don’t default to sRGB > so the colors will be wrong. The fix for this is a one-liner; we just need to > declare that our CALayer uses the sRGB color space so the OS will convert it > to the screen’s space (presumably with a slight performance penalty). > > In the reverse direction the Robot should be returning sRGB colors. The > getPixelColor calls were making no conversion. The getScreenCapture calls > were converting to genericRGB, not sRGB, and so the results didn’t match the > getPixelColor calls. This PR fixes these bugs; getPixelColor and > getScreenCapture both return sRGB. > > Now that everything is working in the same space when JavaFX writes out a > pixel and then reads it back in the colors should match within a limited > tolerance (due to rounding issues when converting from float to int and > back). But that just means the various glass code paths are using the same > space to perform conversions, not that it’s sRGB. AWT is color space aware > and so the automated test employs an AWT Robot to double-check the results. > > I swept through the rest of the Mac glass code and found a few places where > colors were being converted to deviceRGB instead of sRGB e.g. when reading > colors for PlatformPreferences or creating images for drag and drop. I could > not think of a good way of writing automated tests for these cases. > > I started investigating this since Robot tests were failing unless the > monitor’s profile was set to sRGB. Unfortunately this PR doesn’t entirely fix > that. My monitor uses Display P3 and I’m still seeing failures on the > SwingNodeJDialogTest. The test writes out pure BLUE colors and gets back > colors with a surprising amount of red. I’ve verified that this has nothing > to do with JavaFX, it happens when I use CoreGraphics to make the sRGB => > Display P3 color conversions directly. I guess this is a cautionary tale > about what happens when you work near the edges of a color space’s gamut. Martin Fox 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 20 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into colormgmt - Color space is two words. - Merge remote-tracking branch 'upstream/master' into colormgmt - Merge remote-tracking branch 'upstream/master' into colormgmt - Merge remote-tracking branch 'upstream/master' into colormgmt - Robot code is now executed inside autorelease pool - Cleaned up SRGBTest - Merge remote-tracking branch 'upstream/master' into colormgmt - We don't need to manually release the color space. - Merge remote-tracking branch 'upstream/master' into colormgmt - ... and 10 more: https://git.openjdk.org/jfx/compare/330ec3fa...41e3baf9 - Changes: - all: https://git.openjdk.org/jfx/pull/1473/files - new: https://git.openjdk.org/jfx/pull/1473/files/ae06f5a4..41e3baf9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1473&range=01 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1473&range=00-01 Stats: 5789 lines in 84 files changed: 5650 ins; 49 del; 90 mod Patch: https://git.openjdk.org/jfx/pull/1473.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1473/head:pull/1473 PR: https://git.openjdk.org/jfx/pull/1473
Re: RFR: 8325445: [macOS] Colors are not displayed in sRGB color space
On Fri, 7 Jun 2024 18:29:00 GMT, Martin Fox wrote: > When drawing to the screen JavaFX is producing sRGB colors but on macOS > that’s not necessarily what the user is seeing. Since the pixels are not > tagged as sRGB the OS is copying them unmodified to the frame buffer to be > displayed in the screen’s color space. In general Mac’s don’t default to sRGB > so the colors will be wrong. The fix for this is a one-liner; we just need to > declare that our CALayer uses the sRGB color space so the OS will convert it > to the screen’s space (presumably with a slight performance penalty). > > In the reverse direction the Robot should be returning sRGB colors. The > getPixelColor calls were making no conversion. The getScreenCapture calls > were converting to genericRGB, not sRGB, and so the results didn’t match the > getPixelColor calls. This PR fixes these bugs; getPixelColor and > getScreenCapture both return sRGB. > > Now that everything is working in the same space when JavaFX writes out a > pixel and then reads it back in the colors should match within a limited > tolerance (due to rounding issues when converting from float to int and > back). But that just means the various glass code paths are using the same > space to perform conversions, not that it’s sRGB. AWT is color space aware > and so the automated test employs an AWT Robot to double-check the results. > > I swept through the rest of the Mac glass code and found a few places where > colors were being converted to deviceRGB instead of sRGB e.g. when reading > colors for PlatformPreferences or creating images for drag and drop. I could > not think of a good way of writing automated tests for these cases. > > I started investigating this since Robot tests were failing unless the > monitor’s profile was set to sRGB. Unfortunately this PR doesn’t entirely fix > that. My monitor uses Display P3 and I’m still seeing failures on the > SwingNodeJDialogTest. The test writes out pure BLUE colors and gets back > colors with a surprising amount of red. I’ve verified that this has nothing > to do with JavaFX, it happens when I use CoreGraphics to make the sRGB => > Display P3 color conversions directly. I guess this is a cautionary tale > about what happens when you work near the edges of a color space’s gamut. As a datapoint, we were doing some testing in the [jfx-sandbox:metal](https://github.com/openjdk/jfx-sandbox/tree/metal) branch, and had over a hundred test failures that we tracked down to the color profile not being set to sRGB (the test system had the default "Color LCD profile"). I applied the fix from this patch (and resolving a couple merge conflicts due to some Metal-specific changes in the sandbox branch), and ran one of the failing tests with the default "Color LCD" profile and it now passes. Even if this PR doesn't fix all of the problems, it seems like a good fix to consider. @beldenfox Can you merge in the latest master so we will see a test build on macOS / aarch64? - PR Comment: https://git.openjdk.org/jfx/pull/1473#issuecomment-2200290351 PR Comment: https://git.openjdk.org/jfx/pull/1473#issuecomment-2200295936
Re: RFR: 8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v9]
On Thu, 27 Jun 2024 13:43:37 GMT, Marius Hanl wrote: >> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` >> is broken when `sizeToScene()` was called before or after. >> >> The approach here is to ignore the `sizeToScene()` request when the `Stage` >> is maximized or set to fullscreen. >> Otherwise the Window Manager of the OS will be confused and you will get >> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' >> button still shows the 'Restore' Icon, while we already resized the `Stage` >> to not be maximized). > > Marius Hanl has updated the pull request incrementally with one additional > commit since the last revision: > > add delta for assertStageScreenBounds Marked as reviewed by lkostyra (Committer). - PR Review: https://git.openjdk.org/jfx/pull/1382#pullrequestreview-2151457529
Re: Should we document Styleable properties?
I'd say this is a good idea. Both variants are good, we might want to
mention CSS in the first one, though.
Not sure how others do it, but I regularly struggle to find out which
code properties correspond to which CSS styles. This is particularly the
case for names like "background", "border", or "stroke", where there is
no clean 1:1 correspondence, or similar names are used in multiple contexts.
Some way of linking to the current CSS reference might also be helpful
to find out about allowed arguments. If I google something like "javafx
css" I typically only get links to outdated versions of the CSS
reference. Also, linking back from the CSS reference to the code
properties' javadoc pages is something I've been missing - maybe this
could be added without much additional effort if javadocs are updated
according to this proposal?
Am 28.06.2024 um 23:04 schrieb Andy Goryachev:
Dear fellow developers:
Should we document which properties are styleable with CSS in
javadoc? Would that be useful?
Example:
/**
* Determines the caret blink period. This property cannot be set
to {@code null}.
*
* This is a {@link StyleableProperty} with the name {@code
-fx-caret-blink-period}.
*
alternative:
* This property can be styled with CSS using {@code
-fx-caret-blink-period} name.
* @implNote The property object implements {@link
StyleableProperty} interface.
Other ideas are also welcome.
-andy
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v3]
> Adding, then removing, and then adding a JFXPanel to the same component in > the Swing scene graph leads to a NullPointerException in GlassScene because > the sceneState is null. > Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which > calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose > -> GlassScene.dispose which sets "sceneState" to null... > so when GlassScene.updateSceneState is called, it results in NPE. > Fix is to check if `host` (which is usually > javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been > reset/deleted which is done when `EmbeddedScene.dispose` is called during > removeNotify and abstain from updating the scene state Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Test fix - Changes: - all: https://git.openjdk.org/jfx/pull/1493/files - new: https://git.openjdk.org/jfx/pull/1493/files/6ddb5ab3..2872159b Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1493&range=02 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1493&range=01-02 Stats: 6 lines in 1 file changed: 1 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jfx/pull/1493.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1493/head:pull/1493 PR: https://git.openjdk.org/jfx/pull/1493
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException
On Fri, 28 Jun 2024 08:49:49 GMT, Prasanta Sadhukhan wrote: > Adding, then removing, and then adding a JFXPanel to the same component in > the Swing scene graph leads to a NullPointerException in GlassScene because > the sceneState is null. > Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which > calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose > -> GlassScene.dispose which sets "sceneState" to null... > so when GlassScene.updateSceneState is called, it results in NPE. > Fix is to check if `host` (which is usually > javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been > reset/deleted which is done when `EmbeddedScene.dispose` is called during > removeNotify and abstain from updating the scene state Check is not without precedence in this file..There are instance of `assert host != null;` and `host != null` check like below https://github.com/openjdk/jfx/blob/6a586b662592be3eb81670f0c5ce48061c2fc07c/modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/EmbeddedScene.java#L185 Automated test added - PR Comment: https://git.openjdk.org/jfx/pull/1493#issuecomment-2199627928 PR Comment: https://git.openjdk.org/jfx/pull/1493#issuecomment-2199628476
Re: RFR: 8334593: Adding, removing and then adding a JFXPanel again leads to NullPointerException [v2]
> Adding, then removing, and then adding a JFXPanel to the same component in > the Swing scene graph leads to a NullPointerException in GlassScene because > the sceneState is null. > Removing JFXPanel calls JFXPanel.removeNotify which calls Window.hide which > calls SceneHelper.disposePeer -> Scene.disposePeer -> EmbeddedScene.dispose > -> GlassScene.dispose which sets "sceneState" to null... > so when GlassScene.updateSceneState is called, it results in NPE. > Fix is to check if `host` (which is usually > javafx.embed.swing.JFXPanel$HostContainer for active JFXPanel) has been > reset/deleted which is done when `EmbeddedScene.dispose` is called during > removeNotify and abstain from updating the scene state Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Test added - Test added - Changes: - all: https://git.openjdk.org/jfx/pull/1493/files - new: https://git.openjdk.org/jfx/pull/1493/files/14ac6ef3..6ddb5ab3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1493&range=01 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1493&range=00-01 Stats: 121 lines in 1 file changed: 121 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jfx/pull/1493.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1493/head:pull/1493 PR: https://git.openjdk.org/jfx/pull/1493
Re: RFR: 8289115: Touch events is not dispatched after upgrade to JAVAFX17+
Thank you Michael, Jose, Florian, and Markus In my mind the "Too many touch points reported" exception fix (#394) appears to not be the correct fix for that issue and is what caused this regression. There seems to be two possible paths forward, either revert #394 or integrate this. In both cases the "Too many touch points reported" issue will need to be readdressed. It would be nice to see this move forward somehow. Thanks again, Jurgen On Tue, 04 Jun 2024 18:28:47 +0200, Markus Mack wrote: On Tue, 21 May 2024 14:25:51 GMT, Michael Strauß wrote: This PR fixes a bug ([JDK-8289115](https://bugs.openjdk.org/browse/JDK-8289115)) that was introduced with #394, which changed the following line in the `NotifyTouchInput` function (ViewContainer.cpp): - const bool isDirect = true; + const bool isDirect = IsTouchEvent(); `IsTouchEvent` is a small utility function that uses the Windows SDK function `GetMessageExtraInfo` to distinguish between mouse events and pen events as described in this article: https://learn.microsoft.com/en-us/windows/win32/tablet/system-events-and-mouse-messages I think that using this function to distinguish between touchscreen events and pen events is erroneous. The linked article states: _"When your application receives a **mouse message** (such as WM_LBUTTONDOWN) [...]"_ But we are not receiving a mouse message in `NotifyTouchInput`, we are receiving a `WM_TOUCH` message. I couldn't find any information on whether this API usage is also supported for `WM_TOUCH` messages, but my testing shows that that's probably not the case (I tested this with a XPS 15 touchscreen laptop, where `GetMessageExtraInfo` returns 0 for `WM_TOUCH` messages). My suggestion is to check the `TOUCHEVENTF_PEN` flag of the `TOUCHINPUT` structure instead: const bool isDirect = (ti->dwFlags & TOUCHEVENTF_PEN) == 0; I attempted to test this with the sample app in [JDK-8249737](https://bugs.openjdk.org/browse/JDK-8249737) and [Microsoft.Windows.Simulator](https://stackoverflow.com/questions/40274660/windows-10-how-do-i-test-touch-events-without-a-touchscreen) using the simulator's "Basic touch mode". The "Too many touch points reported" exception seems to be thrown consistently with `const bool isDirect = true;` (original before #394) and `const bool isDirect = (ti->dwFlags & TOUCHEVENTF_PEN) == 0;` (this PR) I didn't see the exception with `const bool isDirect = IsTouchEvent();` (#394) and `const bool isDirect = false` (which is probably what IsTouchEvent() returns here). Not sure what this simulator actually sends, but someone more knowledgeable might want to have a look at what's happening. - PR Comment: https://git.openjdk.org/jfx/pull/1459#issuecomment-2147933056
