On Mon, 25 Nov 2024 12:24:06 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> When the initial value of a styleable property is not specified in a
>> stylesheet, no transition is started:
>>
>> .button {
>> transition: -fx-opacity 1s;
>> }
>>
>> .button:hover {
>> -fx-opacity: 0.5;
>> }
>>
>> The expected behavior is that a transition is started in this case, since
>> the default value of `-fx-opacity` is 1.
>>
>> The reason for this bug is that `StyleableProperty` implementations do not
>> start a CSS transition when the value is applied for the first time. The
>> intention behind this is that a node that is added to the scene graph should
>> not start transitions. CSS transitions should only be started _after_ the
>> node has been shown for the first time.
>>
>> The logic to detect this situation is currently as follows:
>>
>> // If this.origin == null, we're setting the value for the first time.
>> // No transition should be started in this case.
>> TransitionDefinition transition = this.origin != null && getBean()
>> instanceof Node node ?
>> NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
>>
>>
>> However, this does not work. When no initial style is specified in the
>> stylesheet, `this.origin` will not be set, and thus no transition will be
>> started even after the node has been shown. The new logic works like this:
>>
>> A `Node.initialCssState` flag is added. Initially, this is `true`. Manually
>> calling `applyCss` or similar methods will not clear this flag, as we
>> consider all manual CSS processing to be part of the "initial CSS state".
>> Only at the end of `Scene.doCSSPass` will this flag be cleared on all nodes
>> that have expressed their interest. This mechanism ensures that a node will
>> be eligible for CSS transitions only after the following conditions have
>> been satisfied:
>> 1. The node was added to a scene graph
>> 2. CSS processing was completed for a scene pulse
>
> modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 607:
>
>> 605: * CSS state (see {@link Node#initialCssState}.
>> 606: */
>> 607: private final Set<Node> clearInitialCssStateNodes =
>> Collections.newSetFromMap(new IdentityHashMap<>());
>
> why not use a normal `HashSet`? What advantage has this approach?
`HashSet` uses `Object.equals`, which can be overridden by user code, and this
would break the logic. It's the _instance_ that wants to be notified.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1607#discussion_r1856544027
