RFR: 8345188: Support tree-structural pseudo-classes [v6]

[Michael Strauß]

On Wed, 8 Jan 2025 11:57:44 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß 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 seven additional commits since the last revision:
>> 
>>  - Merge branch 'master' into feature/root-pseudo-class
>>  - whitespace error
>>  - add child-indexed pseudo-classes
>>  - add pseudo-class table to Parent
>>  - update cssref
>>  - Set :root pseudo-class on sub-scene root node
>>  - Set :root pseudo-class on root node
>
> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 479:
> 
>> 477:                     n.pseudoClassStateChanged(NTH_EVEN_CHILD_PSEUDO_CLASS, i % 2 != 0);
>> 478:                     n.pseudoClassStateChanged(NTH_ODD_CHILD_PSEUDO_CLASS, i % 2 == 0);
>> 479:                 }
> 
> With this new code, Nodes with many children being manipulated may incur some significant new costs now that these pseudo classes need to be changed for potentially many/all children.  Where before adding a new first child mostly had a cost in shifting the child array with a copy, now we also potentially have two listener call backs per child (and I'm unsure, but these may even trigger some CSS recalculations or CSS cache invalidations).  Nodes with many children may suffer from this, and mostly needlessly as these pseudo classes will see very limited use.
> 
> I think there's some potential for performance regression here that is perhaps sufficient to warrant the exclusion of this feature or making it optional.

I think this concern is out of proportion. Generally, nodes will not have such a huge number of children that simply flipping a few pseudo-classes will be prohibitively expensive. CSS recalculation will, in most cases, only happen once per pulse anyway.

I don't know why we would be optimizing for pathological applications that register two pseudo-class listeners for each of their tens of thousands of children in a container, _and_ do heavy work in those listeners.

> modules/javafx.graphics/src/test/java/test/javafx/scene/Parent_pseudoClasses_Test.java line 60:
> 
>> 58:         assertNotPseudoClass(ONLY_CHILD, child1);
>> 59:         assertPseudoClass(ONLY_CHILD, child2);
>> 60:     }
> 
> How about a case where the last child is removed, and seeing if the first and now only child gets the correct states?  I think there currently is definitely a bug where the odd/even may not be reapplied in that case...

I've added more tests, including the scenario you point out.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907558879
PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907559803