diff --git a/jme3-core/src/main/java/com/jme3/scene/Geometry.java b/jme3-core/src/main/java/com/jme3/scene/Geometry.java
index 50ca01912..f56e71a57 100644
--- a/jme3-core/src/main/java/com/jme3/scene/Geometry.java
+++ b/jme3-core/src/main/java/com/jme3/scene/Geometry.java
@@ -77,15 +77,18 @@ public class Geometry extends Spatial {
* is managed by.
*/
protected GeometryGroupNode groupNode;
+
/**
* The start index of this Geometry's inside
* the {@link GeometryGroupNode}.
*/
- protected int startIndex = -1;
+ protected int startIndex = -1;
+
/**
* Serialization only. Do not use.
*/
public Geometry() {
+ this(null);
}
/**
@@ -97,6 +100,12 @@ public class Geometry extends Spatial {
*/
public Geometry(String name) {
super(name);
+
+ // For backwards compatibility, only clear the "requires
+ // update" flag if we are not a subclass of Geometry.
+ // This prevents subclass from silently failing to receive
+ // updates when they upgrade.
+ setRequiresUpdates(Geometry.class != getClass());
}
/**
diff --git a/jme3-core/src/main/java/com/jme3/scene/Node.java b/jme3-core/src/main/java/com/jme3/scene/Node.java
index 3934b3e13..f085251bd 100644
--- a/jme3-core/src/main/java/com/jme3/scene/Node.java
+++ b/jme3-core/src/main/java/com/jme3/scene/Node.java
@@ -66,10 +66,27 @@ public class Node extends Spatial implements Savable {
*/
protected SafeArrayList children = new SafeArrayList(Spatial.class);
+ /**
+ * If this node is a root, this list will contain the current
+ * set of children (and children of children) that require
+ * updateLogicalState() to be called as indicated by their
+ * requiresUpdate() method.
+ */
+ private SafeArrayList updateList = null;
+
+ /**
+ * False if the update list requires rebuilding. This is Node.class
+ * specific and therefore not included as part of the Spatial update flags.
+ * A flag is used instead of nulling the updateList to avoid reallocating
+ * a whole list every time the scene graph changes.
+ */
+ private boolean updateListValid = false;
+
/**
* Serialization only. Do not use.
*/
public Node() {
+ this(null);
}
/**
@@ -81,6 +98,12 @@ public class Node extends Spatial implements Savable {
*/
public Node(String name) {
super(name);
+
+ // For backwards compatibility, only clear the "requires
+ // update" flag if we are not a subclass of Node.
+ // This prevents subclass from silently failing to receive
+ // updates when they upgrade.
+ setRequiresUpdates(Node.class != getClass());
}
/**
@@ -152,16 +175,72 @@ public class Node extends Spatial implements Savable {
this.worldBound = resultBound;
}
+ @Override
+ protected void setParent(Node parent) {
+ if( this.parent == null && parent != null ) {
+ // We were a root before and now we aren't... make sure if
+ // we had an updateList then we clear it completely to
+ // avoid holding the dead array.
+ updateList = null;
+ updateListValid = false;
+ }
+ super.setParent(parent);
+ }
+
+ private void addUpdateChildren( SafeArrayList results ) {
+ for( Spatial child : children.getArray() ) {
+ if( child.requiresUpdates() ) {
+ results.add(child);
+ }
+ if( child instanceof Node ) {
+ ((Node)child).addUpdateChildren(results);
+ }
+ }
+ }
+
+ /**
+ * Called to invalide the root node's update list. This is
+ * called whenever a spatial is attached/detached as well as
+ * when a control is added/removed from a Spatial in a way
+ * that would change state.
+ */
+ void invalidateUpdateList() {
+ updateListValid = false;
+ for( Node n = parent; n != null; n = n.getParent() ) {
+ n.invalidateUpdateList();
+ }
+ }
+
+ private SafeArrayList getUpdateList() {
+ if( updateListValid ) {
+ return updateList;
+ }
+ if( updateList == null ) {
+ updateList = new SafeArrayList(Spatial.class);
+ } else {
+ updateList.clear();
+ }
+
+ // Build the list
+ addUpdateChildren(updateList);
+ updateListValid = true;
+ return updateList;
+ }
+
@Override
public void updateLogicalState(float tpf){
super.updateLogicalState(tpf);
- if (children.isEmpty()) {
+ // Only perform updates on children if we are the
+ // root and then only peform updates on children we
+ // know to require updates.
+ // So if this isn't the root, abort.
+ if( parent != null ) {
return;
}
-
- for (Spatial child : children.getArray()) {
- child.updateLogicalState(tpf);
+
+ for( Spatial s : getUpdateList().getArray() ) {
+ s.updateLogicalState(tpf);
}
}
@@ -251,28 +330,7 @@ public class Node extends Spatial implements Savable {
* @throws IllegalArgumentException if child is null.
*/
public int attachChild(Spatial child) {
- if (child == null)
- throw new IllegalArgumentException("child cannot be null");
-
- if (child.getParent() != this && child != this) {
- if (child.getParent() != null) {
- child.getParent().detachChild(child);
- }
- child.setParent(this);
- children.add(child);
-
- // XXX: Not entirely correct? Forces bound update up the
- // tree stemming from the attached child. Also forces
- // transform update down the tree-
- child.setTransformRefresh();
- child.setLightListRefresh();
- if (logger.isLoggable(Level.FINE)) {
- logger.log(Level.FINE,"Child ({0}) attached to this node ({1})",
- new Object[]{child.getName(), getName()});
- }
- }
-
- return children.size();
+ return attachChildAt(child, children.size());
}
/**
@@ -298,12 +356,18 @@ public class Node extends Spatial implements Savable {
}
child.setParent(this);
children.add(index, child);
+
+ // XXX: Not entirely correct? Forces bound update up the
+ // tree stemming from the attached child. Also forces
+ // transform update down the tree-
child.setTransformRefresh();
child.setLightListRefresh();
if (logger.isLoggable(Level.FINE)) {
logger.log(Level.FINE,"Child ({0}) attached to this node ({1})",
new Object[]{child.getName(), getName()});
}
+
+ invalidateUpdateList();
}
return children.size();
@@ -380,6 +444,8 @@ public class Node extends Spatial implements Savable {
child.setTransformRefresh();
// lights are also inherited from parent
child.setLightListRefresh();
+
+ invalidateUpdateList();
}
return child;
}
@@ -390,6 +456,9 @@ public class Node extends Spatial implements Savable {
* node.
*/
public void detachAllChildren() {
+ // Note: this could be a bit more efficient if it delegated
+ // to a private method that avoided setBoundRefresh(), etc.
+ // for every child and instead did one in here at the end.
for ( int i = children.size() - 1; i >= 0; i-- ) {
detachChildAt(i);
}
diff --git a/jme3-core/src/main/java/com/jme3/scene/Spatial.java b/jme3-core/src/main/java/com/jme3/scene/Spatial.java
index 62add98ce..242aaf288 100644
--- a/jme3-core/src/main/java/com/jme3/scene/Spatial.java
+++ b/jme3-core/src/main/java/com/jme3/scene/Spatial.java
@@ -162,16 +162,24 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
protected transient int refreshFlags = 0;
/**
- * Serialization only. Do not use.
+ * Set to true if a subclass requires updateLogicalState() even
+ * if it doesn't have any controls. Defaults to true thus implementing
+ * the legacy behavior for any subclasses not specifically turning it
+ * off.
+ * This flag should be set during construction and never changed
+ * as it's supposed to be class-specific and not runtime state.
*/
- public Spatial() {
- localTransform = new Transform();
- worldTransform = new Transform();
-
- localLights = new LightList(this);
- worldLights = new LightList(this);
+ private boolean requiresUpdates = true;
- refreshFlags |= RF_BOUND;
+ /**
+ * Serialization only. Do not use.
+ * Not really. This class is never instantiated directly but the
+ * subclasses like to use the no-arg constructor for their own
+ * no-arg constructor... which is technically weaker than
+ * forward supplying defaults.
+ */
+ protected Spatial() {
+ this(null);
}
/**
@@ -182,9 +190,16 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
* the name of the scene element. This is required for
* identification and comparison purposes.
*/
- public Spatial(String name) {
- this();
+ protected Spatial(String name) {
this.name = name;
+
+ localTransform = new Transform();
+ worldTransform = new Transform();
+
+ localLights = new LightList(this);
+ worldLights = new LightList(this);
+
+ refreshFlags |= RF_BOUND;
}
public void setKey(AssetKey key) {
@@ -195,6 +210,54 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
return key;
}
+ /**
+ * Returns true if this spatial requires updateLogicalState() to
+ * be called, either because setRequiresUpdate(true) has been called
+ * or because the spatial has controls. This is package private to
+ * avoid exposing it to the public API since it is only used by Node.
+ */
+ boolean requiresUpdates() {
+ return requiresUpdates | !controls.isEmpty();
+ }
+
+ /**
+ * Subclasses can call this with true to denote that they require
+ * updateLogicalState() to be called even if they contain no controls.
+ * Setting this to false reverts to the default behavior of only
+ * updating if the spatial has controls. This is not meant to
+ * indicate dynamic state in any way and must be called while
+ * unattached or an IllegalStateException is thrown. It is designed
+ * to be called during object construction and then never changed, ie:
+ * it's meant to be subclass specific state and not runtime state.
+ * Subclasses of Node or Geometry that do not set this will get the
+ * old default behavior as if this was set to true. Subclasses should
+ * call setRequiresUpdate(false) in their constructors to receive
+ * optimal behavior if they don't require updateLogicalState() to be
+ * called even if there are no controls.
+ */
+ protected void setRequiresUpdates( boolean f ) {
+ // Note to explorers, the reason this was done as a protected setter
+ // instead of passed on construction is because it frees all subclasses
+ // from having to make sure to always pass the value up in case they
+ // are subclassed.
+ // The reason that requiresUpdates() isn't just a protected method to
+ // override (which would be more correct) is because the flag provides
+ // some flexibility in how we break subclasses. A protected method
+ // would require that all subclasses that required updates need implement
+ // this method or they would silently stop processing updates. A flag
+ // lets us set a default when a subclass is detected that is different
+ // than the internal "more efficient" default.
+ // Spatial's default is 'true' for this flag requiring subclasses to
+ // override it for more optimal behavior. Node and Geometry will override
+ // it to false if the class is Node.class or Geometry.class.
+ // This means that all subclasses will default to the old behavior
+ // unless they opt in.
+ if( parent != null ) {
+ throw new IllegalStateException("setRequiresUpdates() cannot be called once attached.");
+ }
+ this.requiresUpdates = f;
+ }
+
/**
* Indicate that the transform of this spatial has changed and that
* a refresh is required.
@@ -599,8 +662,17 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
* @see Spatial#removeControl(java.lang.Class)
*/
public void addControl(Control control) {
+ boolean before = requiresUpdates();
controls.add(control);
control.setSpatial(this);
+ boolean after = requiresUpdates();
+
+ // If the requirement to be updated has changed
+ // then we need to let the parent node know so it
+ // can rebuild its update list.
+ if( parent != null && before != after ) {
+ parent.invalidateUpdateList();
+ }
}
/**
@@ -609,12 +681,22 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
* @see Spatial#addControl(com.jme3.scene.control.Control)
*/
public void removeControl(Class controlType) {
+ boolean before = requiresUpdates();
for (int i = 0; i < controls.size(); i++) {
if (controlType.isAssignableFrom(controls.get(i).getClass())) {
Control control = controls.remove(i);
control.setSpatial(null);
+ break; // added to match the javadoc -pspeed
}
}
+ boolean after = requiresUpdates();
+
+ // If the requirement to be updated has changed
+ // then we need to let the parent node know so it
+ // can rebuild its update list.
+ if( parent != null && before != after ) {
+ parent.invalidateUpdateList();
+ }
}
/**
@@ -627,11 +709,21 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab
* @see Spatial#addControl(com.jme3.scene.control.Control)
*/
public boolean removeControl(Control control) {
+ boolean before = requiresUpdates();
boolean result = controls.remove(control);
if (result) {
control.setSpatial(null);
}
+ boolean after = requiresUpdates();
+
+ // If the requirement to be updated has changed
+ // then we need to let the parent node know so it
+ // can rebuild its update list.
+ if( parent != null && before != after ) {
+ parent.invalidateUpdateList();
+ }
+
return result;
}