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

Okay after testing more, I found one more case where I got a CSS warning.
Added a test and fix. It is a very similar test where the hierarchy is just a 
bit different to trigger the problem.

So thanks @andy-goryachev-oracle for you test case, that really helped for this 
particular usecase.

I'm quite happy with the solution and also the performance.
Very interesting: I just retested if 
[JDK-8291853](https://bugs.openjdk.org/browse/JDK-8291853) is now fixed as well 
(so with the PR reverted), and indeed, it is fixed now. 
Back then I already had the suspicion that is has something to do with the CSS 
processing. Which makes me even more confident that I hopefully found the 
correct solution for all problems and all performance cases.

So we could also revert the fix as a follow-up, as now we found the underlying 
probem / root cause.
The PR as reference: https://git.openjdk.org/jfx/pull/947.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/2225#issuecomment-5574479227

Reply via email to