Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8290765: Remove parent disabled/treeVisible listeners
Reviewed-by: arapte, jhendrikx, nlisker
  • Loading branch information
Michael Strauß committed Feb 7, 2023
1 parent ef9f066 commit 44b8c62
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 25 deletions.
33 changes: 12 additions & 21 deletions modules/javafx.graphics/src/main/java/javafx/scene/Node.java
Expand Up @@ -958,8 +958,6 @@ private ReadOnlyObjectWrapper<Parent> parentPropertyImpl() {
protected void invalidated() {
if (oldParent != null) {
clearParentsFocusWithin(oldParent);
oldParent.disabledProperty().removeListener(parentDisabledChangedListener);
oldParent.treeVisibleProperty().removeListener(parentTreeVisibleChangedListener);
if (nodeTransformation != null && nodeTransformation.listenerReasons > 0) {
((Node) oldParent).localToSceneTransformProperty().removeListener(
nodeTransformation.getLocalToSceneInvalidationListener());
Expand All @@ -969,8 +967,6 @@ protected void invalidated() {
computeDerivedDepthTest();
final Parent newParent = get();
if (newParent != null) {
newParent.disabledProperty().addListener(parentDisabledChangedListener);
newParent.treeVisibleProperty().addListener(parentTreeVisibleChangedListener);
if (nodeTransformation != null && nodeTransformation.listenerReasons > 0) {
((Node) newParent).localToSceneTransformProperty().addListener(
nodeTransformation.getLocalToSceneInvalidationListener());
Expand Down Expand Up @@ -1009,10 +1005,6 @@ public String getName() {
return parent;
}

private final InvalidationListener parentDisabledChangedListener = valueModel -> updateDisabled();

private final InvalidationListener parentTreeVisibleChangedListener = valueModel -> updateTreeVisible(true);

private SubScene subScene = null;

/**
Expand Down Expand Up @@ -1918,6 +1910,10 @@ protected void invalidated() {
pseudoClassStateChanged(DISABLED_PSEUDOCLASS_STATE, get());
updateCanReceiveFocus();
focusSetDirty(getScene());

if (Node.this instanceof Parent parent) {
parent.getChildren().forEach(Node::updateDisabled);
}
}

@Override
Expand Down Expand Up @@ -2597,16 +2593,7 @@ <P extends NGNode> P getPeer() {
/**
* Creates a new instance of Node.
*/
protected Node() {
//if (PerformanceTracker.isLoggingEnabled()) {
// PerformanceTracker.logEvent("Node.init for [{this}, id=\"{id}\"]");
//}
updateTreeVisible(false);
//if (PerformanceTracker.isLoggingEnabled()) {
// PerformanceTracker.logEvent("Node.postinit " +
// "for [{this}, id=\"{id}\"] finished");
//}
}
protected Node() {}

/* *************************************************************************
* *
Expand Down Expand Up @@ -8563,7 +8550,7 @@ && isDirty(DirtyBits.NODE_VISIBLE)) {
setTreeVisible(isTreeVisible);
}

private boolean treeVisible;
private boolean treeVisible = true;
private TreeVisiblePropertyReadOnly treeVisibleRO;

final void setTreeVisible(boolean value) {
Expand All @@ -8578,8 +8565,12 @@ final void setTreeVisible(boolean value) {
addToSceneDirtyList();
}
((TreeVisiblePropertyReadOnly) treeVisibleProperty()).invalidate();
if (Node.this instanceof SubScene) {
Node subSceneRoot = ((SubScene)Node.this).getRoot();
if (Node.this instanceof Parent parent) {
for (Node child : parent.getChildren()) {
child.updateTreeVisible(true);
}
} else if (Node.this instanceof SubScene subScene) {
Node subSceneRoot = subScene.getRoot();
if (subSceneRoot != null) {
// SubScene.getRoot() is only null if it's constructor
// has not finished.
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -81,4 +81,8 @@ public static <P extends NGNode> P getPeer(Node n) {
public static ObservableSet<PseudoClass> pseudoClassStates(Node n) {
return n.pseudoClassStates;
}

public static void setTreeVisible(Node n, boolean visible) {
n.setTreeVisible(visible);
}
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -124,8 +124,6 @@ public class NodeTest {
// Test setting/clearing the effect affects the bounds
// Test changing state on Effect updates bounds of Node

// Test that a disabled Group affects the disabled property of child nodes

// Test contains, intersects methods
// Test parentToLocal/localToStage/etc

Expand Down Expand Up @@ -2023,4 +2021,46 @@ public interface StubRectAccessor {
}

}

@Test
public void disabledFlagUpdatesChildrenDisabledFlag() {
var g = new Group();
var n1 = new Rectangle();
var n2 = new Rectangle();
g.getChildren().addAll(n1, n2);
assertFalse(g.isDisabled());
assertFalse(n2.isDisabled());
assertFalse(n2.isDisabled());

g.setDisable(true);
assertTrue(g.isDisabled());
assertTrue(n2.isDisabled());
assertTrue(n2.isDisabled());

g.setDisable(false);
assertFalse(g.isDisabled());
assertFalse(n2.isDisabled());
assertFalse(n2.isDisabled());
}

@Test
public void treeVisibleFlagUpdatesChildrenTreeVisibleFlag() {
var g = new Group();
var n1 = new Rectangle();
var n2 = new Rectangle();
g.getChildren().addAll(n1, n2);
assertTrue(NodeHelper.isTreeVisible(g));
assertTrue(NodeHelper.isTreeVisible(n1));
assertTrue(NodeHelper.isTreeVisible(n2));

NodeShim.setTreeVisible(g, false);
assertFalse(NodeHelper.isTreeVisible(g));
assertFalse(NodeHelper.isTreeVisible(n1));
assertFalse(NodeHelper.isTreeVisible(n2));

NodeShim.setTreeVisible(g, true);
assertTrue(NodeHelper.isTreeVisible(g));
assertTrue(NodeHelper.isTreeVisible(n1));
assertTrue(NodeHelper.isTreeVisible(n2));
}
}

1 comment on commit 44b8c62

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.