On Mon, 7 Sep 2026 18:29:40 GMT, Marius Hanl <[email protected]> wrote:
>> Another much better try to fix the issue. >> I recommend to read: https://github.com/openjdk/jfx/pull/2201 first. All >> tests from there are included. >> I added some new ones that succeed before and after, a first step for more >> CSS tests as discussed in: >> https://github.com/openjdk/jfx/pull/2218#issuecomment-5094495306 >> >> My new idea is now the following constraint, which I think is also a much >> better approach: >> - A `CssStyleHelper` always has a correct `firstStyleableAncestor`. We can >> at any time trust and rely on it. >> - We will also reuse the existing loop for the `isUserSetFont` check to >> improve the performance a bit >> >> Implementation: >> - We now save a flag to exactly know in which CSS state the `Node` is. >> - We will collect all `Node`s in the scene tree once and then reuse the list >> when we need to process a stale `styleHelper` from a parent. >> - This will make sure we do no process the parent again and again when we >> have some stale `styleHelper`s in the chain >> - Not every `Node` has a `styleHelper` - it is only created when needed, so >> we can not attach the flag in there. >> >> This fixes the issue while a deep (optionally unstyled) scene graph has no >> performance penality. >> The approach is similar than my previous PR, but more smart. And with the >> set constraint mentioned above. >> >> --- >> >> I do think we can improve the `CssStyleHelper` more. But for another day. >> Maybe at one point, with more tests and when all requirements are clear, we >> can find a way without `CssHelperState` and without creating an empty >> `CssStyleHelper` just to hold trigger states (because of that, we need to >> check `styleHelper.cacheContainer != null` a lot of times). >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Marius Hanl has updated the pull request incrementally with one additional > commit since the last revision: > > Fix another broken case modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 96: > 94: */ > 95: static CssStyleHelper createStyleHelper(final Node node) { > 96: Styleable[] path = styleablePath(node); similar problem with a stale `path`: a listener might make this path invalid. while `findMatchingStyles()` uses the new hierarchy, while CacheContainer, `updateParentTriggerStates()`, `firstStyleableAncestor` still use old path. modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 100: > 98: // We first recreate the style helper that are stale. > 99: // This usually only happens when a child changes the scene tree > while its CSS is processed. > 100: for (int index = path.length - 1; index > 0; index--) { so my ai buddy suggests the process remains quadratic: while `getStyleableParent()` is used only once, its overloads are called for each stale ancestor. each scans the path to find a user-set font, following to the root, and a new `CacheContainer` scans again. modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 104: > 102: ancestor.cssHelperStale = false; > 103: ancestor.cssHelperResolvedEarly = true; > 104: ancestor.styleHelper = createStyleHelper(ancestor, path, > index); `cssHelperStale` is reset before the the new helper is created, which will cause problems if the new helper resets css properties, invoking listeners. if such a listener styles another descendant, or adds another descendant, the CSS code will see `cssHelperStale=false` but observe the old helper instance. we might want to consider either a "resolving" state or detect this condition and retry. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2225#discussion_r3974005584 PR Review Comment: https://git.openjdk.org/jfx/pull/2225#discussion_r3974055984 PR Review Comment: https://git.openjdk.org/jfx/pull/2225#discussion_r3973981561
