Skip to content

Commit

Permalink
start transitions only when not in initial CSS state
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstr2
mstr2 committed Oct 21, 2024
1 parent 6ac2dd3 commit 9acfc04
Show file tree
Hide file tree
Showing 13 changed files with 170 additions and 26 deletions.
5 changes: 5 additions & 0 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/scene/NodeHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,10 @@ public static void reapplyCSS(Node node) {
nodeAccessor.reapplyCSS(node);
}

public static boolean isInitialCssState(Node node) {
return nodeAccessor.isInitialCssState(node);
}

public static void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes) {
nodeAccessor.recalculateRelativeSizeProperties(node, fontForRelativeSizes);
}
Expand Down Expand Up @@ -391,6 +395,7 @@ boolean doComputeIntersects(Node node, PickRay pickRay,
void setLabeledBy(Node node, Node labeledBy);
Accessible getAccessible(Node node);
void reapplyCSS(Node node);
boolean isInitialCssState(Node node);
void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes);
boolean isTreeVisible(Node node);
BooleanExpression treeVisibleProperty(Node node);
Expand Down
3 changes: 2 additions & 1 deletion modules/javafx.graphics/src/main/java/javafx/css/StyleableBooleanProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public StyleableBooleanProperty(boolean initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, Boolean v) {
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

boolean newValue = v != null && v;
Expand Down
5 changes: 2 additions & 3 deletions modules/javafx.graphics/src/main/java/javafx/css/StyleableDoubleProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ public StyleableDoubleProperty(double initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, Number v) {
// 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 ?
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

double newValue = v != null ? v.doubleValue() : 0;
Expand Down
3 changes: 2 additions & 1 deletion modules/javafx.graphics/src/main/java/javafx/css/StyleableFloatProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public StyleableFloatProperty(float initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, Number v) {
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

float newValue = v != null ? v.floatValue() : 0;
Expand Down
3 changes: 2 additions & 1 deletion modules/javafx.graphics/src/main/java/javafx/css/StyleableIntegerProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public StyleableIntegerProperty(int initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, Number v) {
TransitionDefinition transition = this.origin != null && getBean() instanceof Node node ?
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

int newValue = v != null ? v.intValue() : 0;
Expand Down
5 changes: 2 additions & 3 deletions modules/javafx.graphics/src/main/java/javafx/css/StyleableLongProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ public StyleableLongProperty(long initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, Number v) {
// 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 ?
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition = getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

long newValue = v != null ? v.longValue() : 0;
Expand Down
10 changes: 4 additions & 6 deletions modules/javafx.graphics/src/main/java/javafx/css/StyleableObjectProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,9 @@ public void applyStyle(StyleOrigin origin, T newValue) {
* @param metadata the CSS metadata of the value
*/
private void applyValue(T oldValue, T newValue, CssMetaData<? extends Styleable, T> metadata) {
// If this.origin == null, we're setting the value for the first time.
// No transition should be started in this case.
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition =
this.origin != null && getBean() instanceof Node node ?
getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, metadata) : null;

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

List<CssMetaData<? extends Styleable, ?>> subMetadata = metadata.getSubProperties();
Expand Down
5 changes: 2 additions & 3 deletions modules/javafx.graphics/src/main/java/javafx/css/StyleableStringProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,9 @@ public StyleableStringProperty(String initialValue) {
/** {@inheritDoc} */
@Override
public void applyStyle(StyleOrigin origin, String newValue) {
// If this.origin == null, we're setting the value for the first time.
// No transition should be started in this case.
// If the value is applied for the first time, we don't start a transition.
TransitionDefinition transition =
this.origin != null && getBean() instanceof Node node ?
getBean() instanceof Node node && !NodeHelper.isInitialCssState(node) ?
NodeHelper.findTransitionDefinition(node, getCssMetaData()) : null;

if (transition == null) {
Expand Down
32 changes: 32 additions & 0 deletions modules/javafx.graphics/src/main/java/javafx/scene/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,11 @@ public void reapplyCSS(Node node) {
node.reapplyCSS();
}

@Override
public boolean isInitialCssState(Node node) {
return node.initialCssState;
}

@Override
public void recalculateRelativeSizeProperties(Node node, Font fontForRelativeSizes) {
node.recalculateRelativeSizeProperties(fontForRelativeSizes);
Expand Down Expand Up @@ -1010,6 +1015,8 @@ protected void invalidated() {
}
updateDisabled();
computeDerivedDepthTest();
resetInitialCssStateFlag();

final Parent newParent = get();

// Update the focus bits before calling reapplyCss(), as the focus bits can affect CSS styling.
Expand Down Expand Up @@ -1104,8 +1111,14 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
getClip().setScenes(newScene, newSubScene);
}
if (sceneChanged) {
if (oldScene != null) {
oldScene.unregisterClearInitialCssStageFlag(this);
}

if (newScene == null) {
completeTransitionTimers();
} else {
resetInitialCssStateFlag();
}
updateCanReceiveFocus();
if (isFocusTraversable()) {
Expand Down Expand Up @@ -10066,6 +10079,25 @@ private void doProcessCSS() {
}
}

/**
* A node is considered to be in its initial CSS state if it wasn't shown in a scene graph before.
* This flag is cleared after CSS processing was completed in a Scene pulse event. Note that manual
* calls to {@link #applyCss()} or similar methods will not clear this flag, since we consider all
* CSS processing before the Scene pulse to be part of the node's initial state.
*/
private boolean initialCssState = true;

private void resetInitialCssStateFlag() {
initialCssState = true;
Scene scene = getScene();
if (scene != null) {
scene.registerClearInitialCssStateFlag(this);
}
}

void clearInitialCssStateFlag() {
initialCssState = false;
}

/**
* A StyleHelper for this node.
Expand Down
23 changes: 23 additions & 0 deletions modules/javafx.graphics/src/main/java/javafx/scene/Scene.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,29 @@ private void doCSSPass() {
sceneRoot.clearDirty(com.sun.javafx.scene.DirtyBits.NODE_CSS);
sceneRoot.processCSS();
}

if (!clearInitialCssStateNodes.isEmpty()) {
for (var node : clearInitialCssStateNodes) {
node.clearInitialCssStateFlag();
}

clearInitialCssStateNodes.clear();
}
}

/**
* A list of nodes that have expressed their interest to be notified when the next CSS pass
* is completed. Nodes will use this event to determine whether they are in their initial
* CSS state (see {@link Node#initialCssState}.
*/
private final Set<Node> clearInitialCssStateNodes = Collections.newSetFromMap(new IdentityHashMap<>());

void registerClearInitialCssStateFlag(Node node) {
clearInitialCssStateNodes.add(node);
}

void unregisterClearInitialCssStageFlag(Node node) {
clearInitialCssStateNodes.remove(node);
}

void doLayoutPass() {
Expand Down
11 changes: 10 additions & 1 deletion modules/javafx.graphics/src/test/java/test/javafx/css/StyleableProperty_transition_Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ void setup() {
scene = new Scene(new Group());
stage = new Stage();
stage.setScene(scene);
stage.show();
}

@AfterEach
Expand All @@ -217,6 +216,7 @@ void testRedundantTransitionIsDiscarded(TestRun testRun) {
testRun.property.applyStyle(StyleOrigin.USER, testRun.defaultValue);
var mediator1 = getFieldValue(testRun.property, testRun.fieldName);
assertNull(mediator1);
stage.show();

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

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

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

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

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

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

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

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

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

// Start the transition and sample the outputs.
property.applyStyle(StyleOrigin.USER, null);
Expand Down
11 changes: 5 additions & 6 deletions modules/javafx.graphics/src/test/java/test/javafx/scene/Node_transitionEvent_Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public void startup() {
scene = new Scene(new Group(node));
stage = new Stage();
stage.setScene(scene);
stage.show();
}

@AfterEach
Expand All @@ -89,7 +88,7 @@ public void testRegularPlayback() {
toolkit.setCurrentTime(0);
scene.getStylesheets().add(url);
node.getStyleClass().add("testClass");
node.applyCss();
stage.show();

List<TransitionEvent> trace = new ArrayList<>();
node.addEventHandler(TransitionEvent.ANY, trace::add);
Expand Down Expand Up @@ -132,7 +131,7 @@ public void testPlaybackIsElidedWhenDurationIsZero() {
toolkit.setCurrentTime(0);
scene.getStylesheets().add(url);
node.getStyleClass().add("testClass");
node.applyCss();
stage.show();

List<TransitionEvent> trace = new ArrayList<>();
node.addEventHandler(TransitionEvent.ANY, trace::add);
Expand Down Expand Up @@ -161,7 +160,7 @@ public void testInterruptedPlayback() {
toolkit.setCurrentTime(0);
scene.getStylesheets().add(url);
node.getStyleClass().add("testClass");
node.applyCss();
stage.show();

List<TransitionEvent> trace = new ArrayList<>();
node.addEventHandler(TransitionEvent.ANY, trace::add);
Expand Down Expand Up @@ -199,7 +198,7 @@ public void testInterruptedPlaybackWithNegativeDelay() {
toolkit.setCurrentTime(0);
scene.getStylesheets().add(url);
node.getStyleClass().add("testClass");
node.applyCss();
stage.show();

List<TransitionEvent> trace = new ArrayList<>();
node.addEventHandler(TransitionEvent.ANY, trace::add);
Expand Down Expand Up @@ -237,7 +236,7 @@ public void testInterruptedPlaybackDuringDelayPhase() {
toolkit.setCurrentTime(0);
scene.getStylesheets().add(url);
node.getStyleClass().add("testClass");
node.applyCss();
stage.show();

List<TransitionEvent> trace = new ArrayList<>();
node.addEventHandler(TransitionEvent.ANY, trace::add);
Expand Down
Loading

0 comments on commit 9acfc04

Please sign in to comment.