On Mon, 25 Nov 2024 17:49:37 GMT, Michael Strauß <mstra...@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
>
> 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 three additional
> commits since the last revision:
>
> - Merge branch 'master' into fixes/css-initial-value
> - use HashSet instead of IdentityHashMap
> - start transitions only when not in initial CSS state
can see the transitions failing in master with the monkey tester, and running
as expected with this fix.
left one minor suggestion - will re-approve should you decide to make the
change.
modules/javafx.graphics/src/main/java/javafx/css/StyleableBooleanProperty.java
line 74:
> 72: public void applyStyle(StyleOrigin origin, Boolean v) {
> 73: // If the value is applied for the first time, we don't start a
> transition.
> 74: TransitionDefinition transition = getBean() instanceof Node node
> && !NodeHelper.isInitialCssState(node) ?
minor: to simplify the code in many places, you could change `isInitialState`
to accept an `Object` argument instead of `Node` and do an `instanceof` there.
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1607#pullrequestreview-2535277219
PR Review Comment: https://git.openjdk.org/jfx/pull/1607#discussion_r1905986753
