Re: Validation Support
On 3/9/24 10:30 AM, Marius Hanl wrote: However, I found this old email thread. Two answers which reflect what we said: https://mail.openjdk.org/pipermail/openjfx-dev/2013-September/010067.html https://mail.openjdk.org/pipermail/openjfx-dev/2013-September/010094.html Wow, good find! Thank you, Marius. I am pleasantly surprised to see some of the same people in that old discussion now participating in this one. In reading the thread, it seems the decision to keep the JavaFX API so closed was made primarily by Richard Bair based on his own experience, and is perhaps best explained in Item 19 of /Effective Java, Third Edition./ by Joshua Bloch. Summary: inheritance violates encapsulation, so designing a class for inheritance "is not a decision to be undertaken lightly." I don't have the experience to offer my own opinion, but with all the talk of opening things up, I thought it might be a good time to revisit the history. You know, "Those who cannot remember the past are condemned to repeat it," and all that. 🙂 John
Aw: Re: Validation Support
Regarding the comment that the design of JavaFX is vastly superior to Swing.. I would argue against bluntly stating that (not criticizing who said that). There are obviously cons and pros to each approach. On the one side Swing was much easier for developers to extend and to add functionality on top whereas JavaFX is harder. JavaFX on the other hand can evolve in a more safe way since the implementation details are less exposed to developers. I wouldn't say one is better than the other, I guess it depends on the scenario you're in. There are pros and cons to each. There are certainly lots of developers that preferred the Swing approach (myself included) and others would probably prefer the JavaFX one (not sure which one the majority prefers). When the JavaFX team first made this decision there were some conditions that don't hold true anymore, so the underlying scenario has changed. Back then it was assumed that the user would be the one responsible for installing and updating the JRE that was going to be used in the app (so you were never sure which jre the user was on). That's no longer the case as almost all apps ship with its own JRE. More importantly, back then the javafx team was much bigger, so changes could be made at a faster pace. Yes JavaFX was closed but if the JavaFX team saw there was a need from developers for some feature/API it could be added much more quickly while still not exposing too much of the implementation. I wonder if in some parts we can compromise if we see there's a need. Like exposing some parts but adding a annotation or something saying that we're still experimenting or/and some scenarios might not be supported, so be careful using this, etc... (that's somewhat already happened in javafx 8 with the @treatAsPrivate annotation) Just some food for thought... I like the compromise that John Hendrikx suggested of copy pasting code and changing the copy pasted code (given the internal API usage is solved). I have done that sometimes. I just have one question though: does the license of JavaFX permit a commercial app to have that in its own app (copy pasted code from the JavaFX SDK)? I have already said that, but what I really would like JavaFX to provide would be more points of extension, things like the possibility to extend the CSS parser and add more CSS primitives, etc... Bottom line: make the API more powerful in terms of what the developer can do. Right now if you want to do something slightly less standard you're likely to hit a bunch of hurdles... My 2 cents.. -- Pedro Duque Vieira - https://www.pixelduke.com
Re: RFR: JDK-8322795 CSS performance regression up to 10x
On Wed, 3 Jan 2024 18:33:29 GMT, Kevin Rushforth wrote: >> The regression is caused by the `Collections.unmodifiableSet` wrapper not >> being recognized by `BitSet`, and a fall back is done to a less optimized >> version of `containsAll`. >> >> As this is a regression fix, I've kept the fix as small as reasonable. I'll >> provide a further optimized version as part of >> https://bugs.openjdk.org/browse/JDK-8322964 which eliminates the need for >> `BitSet` and `StyleClass` in the hot code paths which should result in a >> further CSS performance improvement. >> >> I've tested this solution with the JFXCentral application locally and the >> regression seems resolved. > >> As this is a regression fix, I've kept the fix as small as reasonable > > Thanks. That seems like the best solution. @kevinrushforth should this perhaps be backported to 21? - PR Comment: https://git.openjdk.org/jfx/pull/1314#issuecomment-1986992136
Re: Validation Support
On 08/03/2024 07:37, Robert Lichtenberger wrote: One major pain point that we have at the moment is CSS performance, I think (but did not investigate yet) that some kind of performance regression happened somewhere between 17 and 21. I think this was addressed already, see here: https://github.com/openjdk/jfx/pull/1314 The fix is in JFX22, not sure if it should perhaps be back ported to JFX21. While I was looking at that regression, I also checked if there was more performance to be gained. You can find those changes in https://github.com/openjdk/jfx/pull/1316 --John
Re: RFR: JDK-8218745: TableView: visual glitch at borders on horizontal scrolling
On Wed, 10 Jan 2024 19:21:16 GMT, Marius Hanl wrote: > This PR fixes the border glitch/gap as explained in both linked tickets. > > I noted that the `ScrollPane` control does not suffer from this problem, > although the code is mostly the same as in `VirtualFlow`. The `ScrollPane` > snaps the content correctly, no matter which scale. I carefully checked the > code and it seems that the container structure in `ScrollPane` was explicitly > written to handle this perfectly. There was definitely some thought on that. > > So to finally fix this issue, I rewrote the `VirtualFlow` container/scrolling > code to be **exactly** the same as in `ScrollPane`(Skin). > And this also fixes the issue, while behaving the same as before. > > In the future it may makes sense to try to somewhat unify the code from > `ScrollPane` and `VirtualFlow`. I already have some ideas. > > Unfortunately though, one more container is introduced to `VirtualFlow`, so > the css needs to be changed since it is very strictly written in the first > place: > Before: `.list-view:focused > .virtual-flow > .clipped-container > .sheet > > .list-cell` > After: `.list-view:focused > .virtual-flow > .viewport > .clipped-container > > .sheet > .list-cell` > > To better understand this, the container structure (tree) is shown below: > > - ScrollPane > - viewRect -> `setClip` -> clipRect (viewContent size) > - viewContent -> `setLayoutX` > - Content > - vsb > - hsb > - corner > > --- > - VirtualFlow > - viewRect **(->NEW IN THIS PR<-)** -> `setClip` -> clipRect > (clippedContainer size) > - clippedContainer/clipView -> `setLayoutX` (when scrolling) > - sheet > - Cell > - vsb > - hsb > - corner Note that I found out something interesting which can MAY help us here to solve the problem in another way. What I found out: If I set the the opacity of the clip rect to something under 1, this bug disappears.  So something in the low level clip rendering is different when an opacity is set. Will investigate if there could be another solution. - PR Comment: https://git.openjdk.org/jfx/pull/1330#issuecomment-1986971595
RFR: JDK-8327727: Changing the row factory of a TableView does not recreate the rows
The fix is very straightfoward. We request a recreation of all cells in the `VirtualFlow` by calling `flow.recreateCells()` when the `tableRowFactory` was changed. Funny enough, this was done in the `TreeTableViewSkin` again, hence it did not suffer from this bug. Removed as the table row factory is already handled in the `TableViewSkinBase`, now with the correct recreation. - Commit messages: - JDK-8327727: Changing the row factory of a TableView does not recreate the rows Changes: https://git.openjdk.org/jfx/pull/1396/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1396&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8327727 Stats: 98 lines in 4 files changed: 93 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jfx/pull/1396.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1396/head:pull/1396 PR: https://git.openjdk.org/jfx/pull/1396
Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v3]
> 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: improve tests - Changes: - all: https://git.openjdk.org/jfx/pull/1382/files - new: https://git.openjdk.org/jfx/pull/1382/files/bbdb4db7..e58a2fda Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1382&range=02 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1382&range=01-02 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jfx/pull/1382.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1382/head:pull/1382 PR: https://git.openjdk.org/jfx/pull/1382
Aw: Re: Validation Support
Yes, John made some good points about how the Behaviour API could look like, which I also agree on. CSS will get performance improvements with the changes made by John in JDK-8322964, but I would agree that there is more in the CSS code which should/could be optimized. -- Marius Gesendet: Freitag, 08. März 2024 um 07:37 Uhr Von: "Robert Lichtenberger" An: openjfx-...@openjdk.java.net Betreff: Re: Validation Support Am 08.03.24 um 00:40 schrieb Marius Hanl: One note here regarding that a lot of things are final in JavaFX: The problem is not that everything is final - this is intended and makes sense, since we speak mostly of properties here. Overriding those will not give you any benefit. You mostly want to override e.g. Controls if you add something new to it (a new property) or may just want to set another default skin. This design is mostly superior than e.g. Swing, where you can override things and easily break something if not too careful. Pretty sure that this were some lessons learned from the development of Swing. The biggest problem is rather that a lot of methods are package private or 'Accessors' are used to call specific methods, which is not just a questionable design but also restrict the usage. It is often not possible to override even a minor feature inside the skin. So you may rather want to recreate the skin then, and copy the existing skin and just change some stuff. But this will mostly not work either, as there is a lot of internal API usage, e.g. Accessors or some com.sun.javafx.scene.control.skin.Utils usage. Yes, I had such a problem yesterday. TableView uses + as shortcut to toggle selection of current row. I want to use that key combination as a "global" shortcut however. I have a regular menu bar with a menu item that has + as accelerator. Now I need to remove that bit from the TableView's behavior. I've seen that there are efforts under way to make behavior public API. I would really welcome that :-). BTW, https://bugs.openjdk.org/browse/JDK-8088068 is also an issue concerning accelerators that I currently need a separate class as workaround. Other issues that I currently work around somehow (inconvenient, but no showstopper): JDK-8092315, JDK-8087981, maybe also JDK-8123117, but I think that on is correctly closed as "not an issue", will have to check. To put this into perspective, we have a lot more references to JavaFX issues in our code, which have all been fixed :-). One major pain point that we have at the moment is CSS performance, I think (but did not investigate yet) that some kind of performance regression happened somewhere between 17 and 21. Overall speaking, with the ongoing efforts for a RichText-control and making behavior public API, I think JavaFX is definitely on the right track, at least from my perspective :-). --Robert
Aw: Re: Re: Validation Support
I also remember that but not sure where I got this from as well. However, I found this old email thread. Two answers which reflect what we said: https://mail.openjdk.org/pipermail/openjfx-dev/2013-September/010067.html https://mail.openjdk.org/pipermail/openjfx-dev/2013-September/010094.html -- Marius Gesendet: Freitag, 08. März 2024 um 02:49 Uhr Von: "John Neffenger" An: openjfx-dev@openjdk.org Betreff: Re: Aw: Re: Validation Support On 3/7/24 3:40 PM, Marius Hanl wrote: This design is mostly superior than e.g. Swing, where you can override things and easily break something if not too careful. Pretty sure that this were some lessons learned from the development of Swing. That's what I remember, too, but I can't find any sources for it online, so maybe I'm just imagining things. 🙂 Rather than subclasses breaking things, though, I remember reading that the decision to allow the Swing classes to be so extensible had made it difficult for Oracle to maintain and enhance Swing itself. The JavaFX developers decided early on not to repeat that mistake. Do I remember correctly? Does anyone have links or even first-hand knowledge? John
Aw: Re: Re: Validation Support
I second this. By no means we should open up everything in the Skins. Some things may makes sense, some of them probably do not and I also would rather advocate copying and adjusting it as needed for bigger changes. -- Marius Gesendet: Samstag, 09. März 2024 um 05:46 Uhr Von: "John Hendrikx" An: openjfx-dev@openjdk.org Betreff: Re: Aw: Re: Validation Support I think there is definitely room for improvement here, but I think most controls can currently be build externally, if you are willing to do all the work. Your control may suffer however when you need to integrate with event, focus handling or keyboard navigation. Skins and Behaviors are not something that are a requirement to use to build a custom control, although they can make it easier to adapt existing controls, up to a point. The problem with making things public is that this means they must be specified, and that we must be **very** sure that it is a solid API that will stand the test of time. That's often a significant time investment that could seriously delay building a complicated control like TableView. I think Skins especially are very hard to design in such a way that they have sensible public/protected methods. They often deal with very specific layout or design problems, that could restrict the evolution of the control if there is a fundamental change in how the Skin is constructed. Just imagine if the very first TableViewSkin had all its methods public, and that they couldn't be changed up to this day because the Skin may have been subclassed by 3rd parties. This is why I would advocate instead to make it easier to replace Skins entirely (by being able to copy the code). The biggest issue there is that Skins contain many non-public helpers, primarily their Behaviors. Making Behaviors public would first require unblurring the lines between what is Skin and what is Behavior (not doing so will just make it a bad API that will cause many bugs and problems over the coming years). That would also mean that some controls need some rework to divide the work over their Skin and Behavior correctly, as for many controls the split is there only in name, and is discarded as soon as it was inconvenient. With Behaviors public and some helpful API's (like Font measurement) having a public equivalent, the route to just copy only a Behavior or Skin opens up. Being able to copy them means that you don't need to fully open up and specify every private method in Skins/Behaviors (designing API's is hard, but designing API's that can be sub classed while not restricting an endless set of possibilities that a Skin subclass may need is impossible). --John On 08/03/2024 00:40, Marius Hanl wrote: One note here regarding that a lot of things are final in JavaFX: The problem is not that everything is final - this is intended and makes sense, since we speak mostly of properties here. Overriding those will not give you any benefit. You mostly want to override e.g. Controls if you add something new to it (a new property) or may just want to set another default skin. This design is mostly superior than e.g. Swing, where you can override things and easily break something if not too careful. Pretty sure that this were some lessons learned from the development of Swing. The biggest problem is rather that a lot of methods are package private or 'Accessors' are used to call specific methods, which is not just a questionable design but also restrict the usage. It is often not possible to override even a minor feature inside the skin. So you may rather want to recreate the skin then, and copy the existing skin and just change some stuff. But this will mostly not work either, as there is a lot of internal API usage, e.g. Accessors or some com.sun.javafx.scene.control.skin.Utils usage. Want to set the width of a TableColumn? Well, the internal com.sun.javafx.scene.control.TableColumnBaseHelper.TableColumnBaseAccessor can do that, you can not. Get the width of a text String? Well, you can use com.sun.javafx.scene.control.skin.Utils.computeTextWidth... Offtopic: Maybe we should collect the things which should be public in our opinion at one point to (better) get this started as well? -- Marius Gesendet: Montag, 04. März 2024 um 08:13 Uhr Von: "Robert Lichtenberger" An: openjfx-dev@openjdk.org Betreff: Re: Validation Support First off, as the original author of ValidatorFX I feel flattered by the suggestion of including it into the JavaFX core :-). Some thoughts / insights I gained from developing ValidatorFX: * ValidatorFX is (with the possible exception of GraphicDecoration.java) rather trivial code and also very small (< 1k lines of library code). But it seems to have filled a market gap according (a few thousand unique ip downloads every month, issues from ca. a dozen people so far, even a few PRs). These "market gaps" get filled rarely because they are not a commercially attractive opportunity. On the other
Integrated: JDK-8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations
On Thu, 8 Feb 2024 18:55:54 GMT, Marius Hanl wrote: > `TreeTableRow` does not check the item with `isItemChanged(..)`, unlike all > other implementations of the cell. > > This also means that the `TreeTableRow` always updates the item, although it > should not, resulting in a performance loss as a `TreeTableRow` will always > call `updateItem(..)`. > > It looks like that this was forgotten in > [JDK-8092593](https://bugs.openjdk.org/browse/JDK-8092593). > > Checking the whole history, it looks like the following was happening: > 1. There was a check if the item is the same in all cell implementations > (with `.equals(..)`) > 2. Check was removed as it caused bugs > 3. Check was readded, but instead we first check the index (same index) and > then if we also have the same item (this time with `.isItemChanged(..)`, to > allow developers to subclass it) > 4. Forgotten for `TreeTableRow` inbetween this chaos. > > Added many tests for the case where an item should be changed and where it > should not. > Improved existing tests as well. Using `stageLoader`, as before the tests > only created a stage when doing the assert. > > NOTE: The update logic is now the same as for all other 5 cell > implementations. Especially `TreeCell` (since it is for a tree structure as > well). This pull request has now been integrated. Changeset: 11305843 Author:Marius Hanl URL: https://git.openjdk.org/jfx/commit/11305843300cdbfed9bfa2120da1c7ecd361df39 Stats: 539 lines in 11 files changed: 511 ins; 2 del; 26 mod 8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations Reviewed-by: angorya, kpk - PR: https://git.openjdk.org/jfx/pull/1360
Re: RFR: JDK-8322964 Optimize performance of CSS selector matching [v6]
On Sat, 9 Mar 2024 12:35:22 GMT, Nir Lisker wrote:
>> John Hendrikx has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Remove commented code in BitSetTest
>> - Remove unnecessary addAll overrides
>
> modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 32:
>
>> 30: * @deprecated for removal
>> 31: */
>> 32: public final class StyleClass {
>
> Needs the `@Deprecated(forRemoval = true)` annotation on the class, and the
> doc tag needs an explanation of why it's being removed ("erroneously
> exposed", "no longer needed", "flawed from the start"...) and what should be
> used instead, if at all.
I added it.
Technically, we could leave this class in, but it would just be a dead class
that serves no purpose within FX, at least, once the `SimpleSelector` and
`CompoundSelector` classes have been moved to internal packages.
I'm wondering if I should add any deprecations in this PR at all, as now it
requires a CSR. This could be all handled in the PR that moves these classes
to internal packages.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1518575434
Re: RFR: JDK-8322964 Optimize performance of CSS selector matching [v7]
> Improves performance of selector matching in the CSS subsystem. This is done > by using custom set implementation which are highly optimized for the most > common cases where the number of selectors is small (most commonly 1 or 2). > It also should be more memory efficient for medium sized and large > applications which have many style names defined in various CSS files. > > Due to the optimization, the concept of `StyleClass`, which was only > introduced to assign a fixed bit index for each unique style class name > encountered, is no longer needed. This is because style classes are no longer > stored in a `BitSet` which required a fixed index per encountered style class. > > The performance improvements are the result of several factors: > - Less memory use when only very few style class names are used in selectors > and styles from a large pool of potential styles (a `BitSet` for potentially > 1000 different style names needed 1000 bits (worst case) as it was not > sparse). > - Specialized sets for small number of elements (0, 1, 2, 3-9 and 10+) > - Specialized sets are append only (reduces code paths) and can be made read > only without requiring a wrapper > - Iterator creation is avoided when doing `containsAll` check thanks to the > inverse function `isSuperSetOf` > - Avoids making a copy of the list of style class names to compare (to > convert them to `StyleClass` and put them into a `Set`) -- this copy could > not be cached and was always discarded immediately after... > > The overall performance was tested using the JFXCentral application which > displays about 800 nodes on its start page with about 1000 styles in various > style sheets (and which allows to refresh this page easily). > > On JavaFX 20, the fastest refresh speed was 121 ms on my machine. With the > improvements in this PR, the fastest refresh had become 89 ms. The speed > improvement is the result of a 30% faster `Scene#doCSSPass`, which takes up > the bulk of the time to refresh the JFXCentral main page (about 100 ms before > vs 70 ms after the change). John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Add @Deprecated tag - Changes: - all: https://git.openjdk.org/jfx/pull/1316/files - new: https://git.openjdk.org/jfx/pull/1316/files/dc59c4db..16da6eb0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1316&range=06 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1316&range=05-06 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jfx/pull/1316.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1316/head:pull/1316 PR: https://git.openjdk.org/jfx/pull/1316
Re: RFR: JDK-8322964 Optimize performance of CSS selector matching [v6]
On Sat, 9 Mar 2024 06:48:25 GMT, John Hendrikx wrote:
>> Improves performance of selector matching in the CSS subsystem. This is done
>> by using custom set implementation which are highly optimized for the most
>> common cases where the number of selectors is small (most commonly 1 or 2).
>> It also should be more memory efficient for medium sized and large
>> applications which have many style names defined in various CSS files.
>>
>> Due to the optimization, the concept of `StyleClass`, which was only
>> introduced to assign a fixed bit index for each unique style class name
>> encountered, is no longer needed. This is because style classes are no
>> longer stored in a `BitSet` which required a fixed index per encountered
>> style class.
>>
>> The performance improvements are the result of several factors:
>> - Less memory use when only very few style class names are used in selectors
>> and styles from a large pool of potential styles (a `BitSet` for potentially
>> 1000 different style names needed 1000 bits (worst case) as it was not
>> sparse).
>> - Specialized sets for small number of elements (0, 1, 2, 3-9 and 10+)
>> - Specialized sets are append only (reduces code paths) and can be made read
>> only without requiring a wrapper
>> - Iterator creation is avoided when doing `containsAll` check thanks to the
>> inverse function `isSuperSetOf`
>> - Avoids making a copy of the list of style class names to compare (to
>> convert them to `StyleClass` and put them into a `Set`) -- this copy could
>> not be cached and was always discarded immediately after...
>>
>> The overall performance was tested using the JFXCentral application which
>> displays about 800 nodes on its start page with about 1000 styles in various
>> style sheets (and which allows to refresh this page easily).
>>
>> On JavaFX 20, the fastest refresh speed was 121 ms on my machine. With the
>> improvements in this PR, the fastest refresh had become 89 ms. The speed
>> improvement is the result of a 30% faster `Scene#doCSSPass`, which takes up
>> the bulk of the time to refresh the JFXCentral main page (about 100 ms
>> before vs 70 ms after the change).
>
> John Hendrikx has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Remove commented code in BitSetTest
> - Remove unnecessary addAll overrides
modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 32:
> 30: * @deprecated for removal
> 31: */
> 32: public final class StyleClass {
Needs the `@Deprecated(forRemoval = true)` annotation on the class, and the doc
tag needs an explanation of why it's being removed ("erroneously exposed", "no
longer needed", "flawed from the start"...) and what should be used instead, if
at all.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1518566872
Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v2]
On Fri, 8 Mar 2024 22:59:42 GMT, Andy Goryachev wrote: > This is another, unrelated issue: If I specify a number instead of duration > like `-fx-show-delay: 1;` we'll get an exception every time the tooltip is > about to be shown. This is because the CSS implementation thinks it is a PX value and therefore parse it wrong. You can check the CssParser to see why this happens. Fix might be simple, but the code in the parser is very error prone for this kind of stuff. - PR Comment: https://git.openjdk.org/jfx/pull/1394#issuecomment-1986842087
Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v3]
On Sat, 9 Mar 2024 10:27:21 GMT, Marius Hanl wrote:
>> This PR fixes a long standing issue where the `Tooltip` will always wait one
>> second until it appears the very first time, even if the
>> `-fx-show-delay` was set to another value.
>>
>> The culprit is, that the `cssForced` flag is not inside `Tooltip`, but
>> inside the `TooltipBehaviour`. So the very first `Tooltip` gets processed
>> correctly, but after no `Tooltip` will be processed by CSS before showing,
>> resulting in the set `-fx-show-delay` to not be applied immediately.
>>
>> Added a bunch of headful tests for the behaviour since there were none
>> before.
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Allow Tooltip to process the owner styles first so that also global
> stylesheets are considered for the e.g. tooltip show-delay
modules/javafx.controls/src/main/java/javafx/scene/control/Tooltip.java line
175:
> 173:
> 174: @Override
> 175: protected void show() {
Another idea I was trying that worked fine is to have a new method inside
`PopupWindow`, which we can call rather than `show()`:
protected final void applySceneStylesFromOwner(Window owner) {
Scene sceneValue = getScene();
final Scene ownerScene = getRootWindow(owner).getScene();
if (ownerScene != null) {
if (ownerScene.getUserAgentStylesheet() != null) {
sceneValue.setUserAgentStylesheet(ownerScene.getUserAgentStylesheet());
}
sceneValue.getStylesheets().setAll(ownerScene.getStylesheets());
if (sceneValue.getCursor() == null) {
sceneValue.setCursor(ownerScene.getCursor());
}
}
}
The `show()` method inside `PopupWindow` is also doing excactly the code above,
thats why I ended up calling it.
But we can also think about just calling that code directly and not `show`, we
just need to extract that part of the code and create a `protected` method with
it.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1394#discussion_r1518551314
Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v3]
> This PR fixes a long standing issue where the `Tooltip` will always wait one > second until it appears the very first time, even if the > `-fx-show-delay` was set to another value. > > The culprit is, that the `cssForced` flag is not inside `Tooltip`, but inside > the `TooltipBehaviour`. So the very first `Tooltip` gets processed correctly, > but after no `Tooltip` will be processed by CSS before showing, resulting in > the set `-fx-show-delay` to not be applied immediately. > > Added a bunch of headful tests for the behaviour since there were none before. Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: Allow Tooltip to process the owner styles first so that also global stylesheets are considered for the e.g. tooltip show-delay - Changes: - all: https://git.openjdk.org/jfx/pull/1394/files - new: https://git.openjdk.org/jfx/pull/1394/files/a4a1a49f..093b36f6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1394&range=02 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1394&range=01-02 Stats: 59 lines in 3 files changed: 39 ins; 10 del; 10 mod Patch: https://git.openjdk.org/jfx/pull/1394.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1394/head:pull/1394 PR: https://git.openjdk.org/jfx/pull/1394
Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v2]
On Sat, 9 Mar 2024 04:07:45 GMT, John Hendrikx wrote: > Perhaps we should open an issue to solve this for all controls in one go? A > solution that would prevent **any** `Node` from being displayed without CSS > applied? Yes, that sounds like a good idea. Checking the calls to `applyCss`, it is mostly in table related classes and I remember that I already asked myself the question whether they are really needed there. > Although the change is an improvement, I think a similar problem will still > be present when CSS changes. The cssForced flag I think needs to reset to > false whenever a CSS styleable property of the tool tip is modified. That > would include all such properties... Will check. `Tooltip` is unfortunately a corner case where we to get some CSS properties BEFORE we show it in order to correctly setup the animation timeline. - PR Comment: https://git.openjdk.org/jfx/pull/1394#issuecomment-1986803140
Re: RFR: JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed [v2]
On Fri, 8 Mar 2024 22:36:19 GMT, Andy Goryachev wrote: > What I see is the very first time the tooltip appears it's using the old > show-delay value of 1000ms. Any subsequent attempts the new delay is in > effect. > > Then, change back to 1000ms. Same thing happens: the first time the old > timeout value is used instead of the new. There seems to be a difference when using `setStyle` and a global stylesheet. I'm investigating. - PR Comment: https://git.openjdk.org/jfx/pull/1394#issuecomment-1986802288
