Skip to content

Commit 5b074c4

Browse files
author
Michael Strauß
committed
8342703: CSS transition is not started when initial value was not specified
Reviewed-by: mmack, angorya
1 parent 6ec588c commit 5b074c4

13 files changed

+170
-26
lines changed

‎modules/javafx.graphics/src/main/java/com/sun/javafx/scene/NodeHelper.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@ public static void reapplyCSS(Node node) {
268268
nodeAccessor.reapplyCSS(node);
269269
}
270270

271+
public static boolean isInitialCssState(Node node) {
272+
return nodeAccessor.isInitialCssState(node);
273+
}
274+
271275
public static void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes) {
272276
nodeAccessor.recalculateRelativeSizeProperties(node, fontForRelativeSizes);
273277
}
@@ -373,6 +377,7 @@ boolean doComputeIntersects(Node node, PickRay pickRay,
373377
void setLabeledBy(Node node, Node labeledBy);
374378
Accessible getAccessible(Node node);
375379
void reapplyCSS(Node node);
380+
boolean isInitialCssState(Node node);
376381
void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes);
377382
boolean isTreeVisible(Node node);
378383
BooleanExpression treeVisibleProperty(Node node);

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableBooleanProperty.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public StyleableBooleanProperty(boolean initialValue) {
7070
/** {@inheritDoc} */
7171
@Override
7272
public void applyStyle(StyleOrigin origin, Boolean v) {
73-
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
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) ?
7475
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7576

7677
boolean newValue = v != null && v;

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableDoubleProperty.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,8 @@ public StyleableDoubleProperty(double initialValue) {
7070
/** {@inheritDoc} */
7171
@Override
7272
public void applyStyle(StyleOrigin origin, Number v) {
73-
// If this.origin == null, we're setting the value for the first time.
74-
// No transition should be started in this case.
75-
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
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) ?
7675
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7776

7877
double newValue = v != null ? v.doubleValue() : 0;

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableFloatProperty.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public StyleableFloatProperty(float initialValue) {
7070
/** {@inheritDoc} */
7171
@Override
7272
public void applyStyle(StyleOrigin origin, Number v) {
73-
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
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) ?
7475
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7576

7677
float newValue = v != null ? v.floatValue() : 0;

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableIntegerProperty.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public StyleableIntegerProperty(int initialValue) {
7070
/** {@inheritDoc} */
7171
@Override
7272
public void applyStyle(StyleOrigin origin, Number v) {
73-
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
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) ?
7475
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7576

7677
int newValue = v != null ? v.intValue() : 0;

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableLongProperty.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,8 @@ public StyleableLongProperty(long initialValue) {
7171
/** {@inheritDoc} */
7272
@Override
7373
public void applyStyle(StyleOrigin origin, Number v) {
74-
// If this.origin == null, we're setting the value for the first time.
75-
// No transition should be started in this case.
76-
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
74+
// If the value is applied for the first time, we don't start a transition.
75+
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
7776
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7877

7978
long newValue = v != null ? v.longValue() : 0;

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableObjectProperty.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,9 @@ public void applyStyle(StyleOrigin origin, T newValue) {
103103
* @param metadata the CSS metadata of the value
104104
*/
105105
private void applyValue(T oldValue, T newValue, CssMetaData<? extends Styleable, T> metadata) {
106-
// If this.origin == null, we're setting the value for the first time.
107-
// No transition should be started in this case.
106+
// If the value is applied for the first time, we don't start a transition.
108107
TransitionDefinition transition =
109-
this.origin != null && getBean() instanceof Node node ?
108+
getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
110109
NodeHelper.findTransitionDefinition(node, metadata) : null;
111110

112111
// We only start a new transition if the new target value is different from the target
@@ -146,10 +145,9 @@ private void applyValue(T oldValue, T newValue, CssMetaData<? extends Styleable,
146145
private void applyComponents(T oldValue, T newValue,
147146
CssMetaData<? extends Styleable, T> metadata,
148147
SubPropertyConverter<T> converter) {
149-
// If this.origin == null, we're setting the value for the first time.
150-
// No transition should be started in this case.
148+
// If the value is applied for the first time, we don't start a transition.
151149
Map<CssMetaData<? extends Styleable, ?>, TransitionDefinition> transitions =
152-
this.origin != null && getBean() instanceof Node node ?
150+
getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
153151
NodeHelper.findTransitionDefinitions(node, metadata) : null;
154152

155153
List<CssMetaData<? extends Styleable, ?>> subMetadata = metadata.getSubProperties();

‎modules/javafx.graphics/src/main/java/javafx/css/StyleableStringProperty.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,9 @@ public StyleableStringProperty(String initialValue) {
7171
/** {@inheritDoc} */
7272
@Override
7373
public void applyStyle(StyleOrigin origin, String newValue) {
74-
// If this.origin == null, we're setting the value for the first time.
75-
// No transition should be started in this case.
74+
// If the value is applied for the first time, we don't start a transition.
7675
TransitionDefinition transition =
77-
this.origin != null && getBean() instanceof Node node ?
76+
getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
7877
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;
7978

8079
if (transition == null) {

‎modules/javafx.graphics/src/main/java/javafx/scene/Node.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,11 @@ public void reapplyCSS(Node node) {
595595
node.reapplyCSS();
596596
}
597597

598+
@Override
599+
public boolean isInitialCssState(Node node) {
600+
return node.initialCssState;
601+
}
602+
598603
@Override
599604
public void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes) {
600605
node.recalculateRelativeSizeProperties(fontForRelativeSizes);
@@ -1010,6 +1015,8 @@ protected void invalidated() {
10101015
}
10111016
updateDisabled();
10121017
computeDerivedDepthTest();
1018+
resetInitialCssStateFlag();
1019+
10131020
final Parent newParent = get();
10141021

10151022
// Update the focus bits before calling reapplyCss(), as the focus bits can affect CSS styling.
@@ -1104,8 +1111,14 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
11041111
getClip().setScenes(newScene, newSubScene);
11051112
}
11061113
if (sceneChanged) {
1114+
if (oldScene != null) {
1115+
oldScene.unregisterClearInitialCssStageFlag(this);
1116+
}
1117+
11071118
if (newScene == null) {
11081119
completeTransitionTimers();
1120+
} else {
1121+
resetInitialCssStateFlag();
11091122
}
11101123
updateCanReceiveFocus();
11111124
if (isFocusTraversable()) {
@@ -10083,6 +10096,25 @@ private void doProcessCSS() {
1008310096
}
1008410097
}
1008510098

10099+
/**
10100+
* A node is considered to be in its initial CSS state if it wasn't shown in a scene graph before.
10101+
* This flag is cleared after CSS processing was completed in a Scene pulse event. Note that manual
10102+
* calls to {@link #applyCss()} or similar methods will not clear this flag, since we consider all
10103+
* CSS processing before the Scene pulse to be part of the node's initial state.
10104+
*/
10105+
private boolean initialCssState = true;
10106+
10107+
private void resetInitialCssStateFlag() {
10108+
initialCssState = true;
10109+
Scene scene = getScene();
10110+
if (scene != null) {
10111+
scene.registerClearInitialCssStateFlag(this);
10112+
}
10113+
}
10114+
10115+
void clearInitialCssStateFlag() {
10116+
initialCssState = false;
10117+
}
1008610118

1008710119
/**
1008810120
* A StyleHelper for this node.

‎modules/javafx.graphics/src/main/java/javafx/scene/Scene.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,29 @@ private void doCSSPass() {
583583
sceneRoot.clearDirty(com.sun.javafx.scene.DirtyBits.NODE_CSS);
584584
sceneRoot.processCSS();
585585
}
586+
587+
if (!clearInitialCssStateNodes.isEmpty()) {
588+
for (var node : clearInitialCssStateNodes) {
589+
node.clearInitialCssStateFlag();
590+
}
591+
592+
clearInitialCssStateNodes.clear();
593+
}
594+
}
595+
596+
/**
597+
* A list of nodes that have expressed their interest to be notified when the next CSS pass
598+
* is completed. Nodes will use this event to determine whether they are in their initial
599+
* CSS state (see {@link Node#initialCssState}.
600+
*/
601+
private final Set<Node> clearInitialCssStateNodes = new HashSet<>();
602+
603+
void registerClearInitialCssStateFlag(Node node) {
604+
clearInitialCssStateNodes.add(node);
605+
}
606+
607+
void unregisterClearInitialCssStageFlag(Node node) {
608+
clearInitialCssStateNodes.remove(node);
586609
}
587610

588611
void doLayoutPass() {

‎modules/javafx.graphics/src/test/java/test/javafx/css/StyleableProperty_transition_Test.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ void setup() {
198198
scene = new Scene(new Group());
199199
stage = new Stage();
200200
stage.setScene(scene);
201-
stage.show();
202201
}
203202

204203
@AfterEach
@@ -217,6 +216,7 @@ void testRedundantTransitionIsDiscarded(TestRun testRun) {
217216
testRun.property.applyStyle(StyleOrigin.USER, testRun.defaultValue);
218217
var mediator1 = getFieldValue(testRun.property, testRun.fieldName);
219218
assertNull(mediator1);
219+
stage.show();
220220

221221
// Start the transition. This adds it to the list of running transitions.
222222
testRun.property.applyStyle(StyleOrigin.USER, testRun.newValue);
@@ -240,6 +240,7 @@ void testReversingTransitionIsNotDiscarded(TestRun testRun) {
240240
testRun.property.applyStyle(StyleOrigin.USER, testRun.defaultValue);
241241
var mediator1 = getFieldValue(testRun.property, testRun.fieldName);
242242
assertNull(mediator1);
243+
stage.show();
243244

244245
// Start the transition. This adds it to the list of running transitions.
245246
testRun.property.applyStyle(StyleOrigin.USER, testRun.newValue);
@@ -271,6 +272,7 @@ void testExistingTransitionOfComponentTransitionableIsPreserved() {
271272
// Setting a value for the first time doesn't start a transition.
272273
setAnimationTime(0);
273274
property.applyStyle(StyleOrigin.USER, border1);
275+
stage.show();
274276

275277
// Start the transition and capture a copy of the sub-property mediator list.
276278
// -fx-background-color will transition from RED to GREEN
@@ -301,6 +303,7 @@ void testIntegerTransitionsInRealNumberSpace() {
301303
// Setting a value for the first time doesn't start a transition.
302304
setAnimationTime(0);
303305
property.applyStyle(StyleOrigin.USER, 0);
306+
stage.show();
304307

305308
// Start the transition and sample the outputs.
306309
property.applyStyle(StyleOrigin.USER, 2);
@@ -324,6 +327,7 @@ void testLongTransitionsInRealNumberSpace() {
324327
// Setting a value for the first time doesn't start a transition.
325328
setAnimationTime(0);
326329
property.applyStyle(StyleOrigin.USER, 0);
330+
stage.show();
327331

328332
// Start the transition and sample the outputs.
329333
property.applyStyle(StyleOrigin.USER, 2);
@@ -347,6 +351,7 @@ void testBooleanTransitionsAsDiscrete() {
347351
// Setting a value for the first time doesn't start a transition.
348352
setAnimationTime(0);
349353
property.applyStyle(StyleOrigin.USER, false);
354+
stage.show();
350355

351356
// Start the transition and sample the outputs.
352357
property.applyStyle(StyleOrigin.USER, true);
@@ -364,6 +369,7 @@ void testStringTransitionsAsDiscrete() {
364369
// Setting a value for the first time doesn't start a transition.
365370
setAnimationTime(0);
366371
property.applyStyle(StyleOrigin.USER, "foo");
372+
stage.show();
367373

368374
// Start the transition and sample the outputs.
369375
property.applyStyle(StyleOrigin.USER, "bar");
@@ -403,6 +409,7 @@ enum Fruit { APPLE, ORANGE }
403409
// Setting a value for the first time doesn't start a transition.
404410
setAnimationTime(0);
405411
property.applyStyle(StyleOrigin.USER, Fruit.APPLE);
412+
stage.show();
406413

407414
// Start the transition and sample the outputs.
408415
property.applyStyle(StyleOrigin.USER, Fruit.ORANGE);
@@ -427,6 +434,7 @@ void testNullObjectTransitionsAsDiscrete_withInterpolatableValue() {
427434
// Setting a value for the first time doesn't start a transition.
428435
setAnimationTime(0);
429436
property.applyStyle(StyleOrigin.USER, Color.RED);
437+
stage.show();
430438

431439
// Start the transition and sample the outputs.
432440
property.applyStyle(StyleOrigin.USER, null);
@@ -451,6 +459,7 @@ void testNullObjectTransitionsAsDiscrete_withComponentTransitionableValue() {
451459
// Setting a value for the first time doesn't start a transition.
452460
setAnimationTime(0);
453461
property.applyStyle(StyleOrigin.USER, Background.fill(Color.RED));
462+
stage.show();
454463

455464
// Start the transition and sample the outputs.
456465
property.applyStyle(StyleOrigin.USER, null);

‎modules/javafx.graphics/src/test/java/test/javafx/scene/Node_transitionEvent_Test.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public void startup() {
6565
scene = new Scene(new Group(node));
6666
stage = new Stage();
6767
stage.setScene(scene);
68-
stage.show();
6968
}
7069

7170
@AfterEach
@@ -89,7 +88,7 @@ public void testRegularPlayback() {
8988
toolkit.setCurrentTime(0);
9089
scene.getStylesheets().add(url);
9190
node.getStyleClass().add("testClass");
92-
node.applyCss();
91+
stage.show();
9392

9493
List<TransitionEvent> trace = new ArrayList<>();
9594
node.addEventHandler(TransitionEvent.ANY, trace::add);
@@ -132,7 +131,7 @@ public void testPlaybackIsElidedWhenDurationIsZero() {
132131
toolkit.setCurrentTime(0);
133132
scene.getStylesheets().add(url);
134133
node.getStyleClass().add("testClass");
135-
node.applyCss();
134+
stage.show();
136135

137136
List<TransitionEvent> trace = new ArrayList<>();
138137
node.addEventHandler(TransitionEvent.ANY, trace::add);
@@ -161,7 +160,7 @@ public void testInterruptedPlayback() {
161160
toolkit.setCurrentTime(0);
162161
scene.getStylesheets().add(url);
163162
node.getStyleClass().add("testClass");
164-
node.applyCss();
163+
stage.show();
165164

166165
List<TransitionEvent> trace = new ArrayList<>();
167166
node.addEventHandler(TransitionEvent.ANY, trace::add);
@@ -199,7 +198,7 @@ public void testInterruptedPlaybackWithNegativeDelay() {
199198
toolkit.setCurrentTime(0);
200199
scene.getStylesheets().add(url);
201200
node.getStyleClass().add("testClass");
202-
node.applyCss();
201+
stage.show();
203202

204203
List<TransitionEvent> trace = new ArrayList<>();
205204
node.addEventHandler(TransitionEvent.ANY, trace::add);
@@ -237,7 +236,7 @@ public void testInterruptedPlaybackDuringDelayPhase() {
237236
toolkit.setCurrentTime(0);
238237
scene.getStylesheets().add(url);
239238
node.getStyleClass().add("testClass");
240-
node.applyCss();
239+
stage.show();
241240

242241
List<TransitionEvent> trace = new ArrayList<>();
243242
node.addEventHandler(TransitionEvent.ANY, trace::add);

0 commit comments

Comments
 (0)