commit c204c69db2ef3787663ea13a08c92a52ba5049d6 Author: leszek Date: Tue Feb 14 00:26:20 2017 +0000 New, cleaner way to create/destroy DistortedSurfaces. Serious regression in StarWars (crashes!). Looks like the Node's internal FBO is being deleted and not re-created in time. diff --git a/src/main/java/org/distorted/library/Distorted.java b/src/main/java/org/distorted/library/Distorted.java index 071ff5e..53c4d94 100644 --- a/src/main/java/org/distorted/library/Distorted.java +++ b/src/main/java/org/distorted/library/Distorted.java @@ -138,6 +138,7 @@ public class Distorted DistortedNode.onDestroy(); EffectQueue.onDestroy(); DistortedEffects.onDestroy(); + DistortedAttachDaemon.onDestroy(); EffectMessageSender.stopSending(); mInitialized = false; diff --git a/src/main/java/org/distorted/library/DistortedAttachDaemon.java b/src/main/java/org/distorted/library/DistortedAttachDaemon.java new file mode 100644 index 0000000..dce57fc --- /dev/null +++ b/src/main/java/org/distorted/library/DistortedAttachDaemon.java @@ -0,0 +1,113 @@ +/////////////////////////////////////////////////////////////////////////////////////////////////// +// Copyright 2016 Leszek Koltunski // +// // +// This file is part of Distorted. // +// // +// Distorted is free software: you can redistribute it and/or modify // +// it under the terms of the GNU General Public License as published by // +// the Free Software Foundation, either version 2 of the License, or // +// (at your option) any later version. // +// // +// Distorted is distributed in the hope that it will be useful, // +// but WITHOUT ANY WARRANTY; without even the implied warranty of // +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // +// GNU General Public License for more details. // +// // +// You should have received a copy of the GNU General Public License // +// along with Distorted. If not, see . // +/////////////////////////////////////////////////////////////////////////////////////////////////// + +package org.distorted.library; + +import java.util.ArrayList; + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This static class handles changing topology of a Tree in a safe way. I.e. if one wants to + * attach() a new Node somewhere in the Tree, we cannot simply attach it mid-render (detach() is + * even worse!). What we do instead is mark that this job will have to be done and actually do it + * just before the next render. That's what this Daemon does. + */ +class DistortedAttachDaemon + { + private static final int ATTACH = 0; // + private static final int DETACH = 1; // types of jobs that the Daemon can do + private static final int DETACHALL = 2; // + + private static class Job + { + int type; + DistortedAttacheable attacheable; + DistortedNode object; + + Job(int t, DistortedAttacheable a, DistortedNode o) + { + type = t; + attacheable = a; + object = o; + } + } + + private static ArrayList mJobs = new ArrayList<>(); + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + private DistortedAttachDaemon() + { + + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + static boolean toDo() + { + int num = mJobs.size(); + Job job; + + for(int i=0; i0 ); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + static void attach(DistortedAttacheable a, DistortedNode n) + { + mJobs.add(new Job(ATTACH,a,n)); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + static void detach(DistortedAttacheable a, DistortedNode n) + { + mJobs.add(new Job(DETACH,a,n)); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + static void detachAll(DistortedAttacheable a) + { + mJobs.add(new Job(DETACHALL,a,null)); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + static void onDestroy() + { + mJobs.clear(); + } + } diff --git a/src/main/java/org/distorted/library/DistortedAttacheable.java b/src/main/java/org/distorted/library/DistortedAttacheable.java new file mode 100644 index 0000000..8d3a6ba --- /dev/null +++ b/src/main/java/org/distorted/library/DistortedAttacheable.java @@ -0,0 +1,36 @@ +/////////////////////////////////////////////////////////////////////////////////////////////////// +// Copyright 2016 Leszek Koltunski // +// // +// This file is part of Distorted. // +// // +// Distorted is free software: you can redistribute it and/or modify // +// it under the terms of the GNU General Public License as published by // +// the Free Software Foundation, either version 2 of the License, or // +// (at your option) any later version. // +// // +// Distorted is distributed in the hope that it will be useful, // +// but WITHOUT ANY WARRANTY; without even the implied warranty of // +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // +// GNU General Public License for more details. // +// // +// You should have received a copy of the GNU General Public License // +// along with Distorted. If not, see . // +/////////////////////////////////////////////////////////////////////////////////////////////////// + +package org.distorted.library; + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * Package-private interface implemented by all objects to which one can attach a Tree-like structure + * of DistortedNodes. (Currently: DistortedNode itself and DistortedOutputSurface). + *

+ * All the methods below are really meant to be package-local; and this interface should really be a + * package-local class which other classes would extend (but that's impossible because OutputSurface + * already extends other class). + */ +interface DistortedAttacheable + { + void attachNow(DistortedNode n); + void detachNow(DistortedNode n); + void detachAllNow(); + } diff --git a/src/main/java/org/distorted/library/DistortedNode.java b/src/main/java/org/distorted/library/DistortedNode.java index 97fccc6..638dcc0 100644 --- a/src/main/java/org/distorted/library/DistortedNode.java +++ b/src/main/java/org/distorted/library/DistortedNode.java @@ -27,11 +27,15 @@ import android.opengl.GLES30; /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Class which represents a Node in a Tree of (InputSurface,Mesh,Effects) triplets. - * + *

* Having organized such sets into a Tree, we can then render any Node to any OutputSurface. * That recursively renders the set held in the Node and all its children. + *

+ * The class takes special care to only render identical sub-trees once. Each Node holds a reference + * to sub-class 'NodeData'. Two identical sub-trees attached at different points of the main tree + * will point to the same NodeData; only the first of this is rendered (when mData.numRendered==0). */ -public class DistortedNode +public class DistortedNode implements DistortedAttacheable { private static HashMap,NodeData> mMapNodeID = new HashMap<>(); private static long mNextNodeID =0; @@ -41,7 +45,6 @@ public class DistortedNode private DistortedInputSurface mSurface; private NodeData mData; - private DistortedNode mParent; private ArrayList mChildren; private int[] mNumChildren; // ==mChildren.length(), but we only create mChildren if the first one gets added @@ -58,8 +61,6 @@ public class DistortedNode numPointingNodes= 1; numRendered = 0; mFBO = null; - - android.util.Log.e("DistortedNode", "new NodeData"); } } @@ -71,25 +72,6 @@ public class DistortedNode mMapNodeID.clear(); } -/////////////////////////////////////////////////////////////////////////////////////////////////// -// tree isomorphism - - private void RecomputeNodeID(ArrayList prev) - { - ArrayList curr = generateIDList(); - - if( mParent==null ) - { - adjustNodeData(prev,curr); - } - else - { - ArrayList parentPrev = mParent.generateIDList(); - adjustNodeData(prev,curr); - mParent.RecomputeNodeID(parentPrev); - } - } - /////////////////////////////////////////////////////////////////////////////////////////////////// private ArrayList generateIDList() @@ -108,55 +90,6 @@ public class DistortedNode return ret; } -/////////////////////////////////////////////////////////////////////////////////////////////////// - - private void adjustNodeData(ArrayList oldList, ArrayList newList) - { - boolean otherNodesPoint = (mData.numPointingNodes>1); - - if( otherNodesPoint ) mData.numPointingNodes--; - else mMapNodeID.remove(oldList); - - NodeData newData = mMapNodeID.get(newList); - - if( newData==null ) - { - if( otherNodesPoint ) mData = new NodeData(++mNextNodeID); - else mData.ID = ++mNextNodeID; // numPointingNodes must be 1 already - - if( newList.size()>1 ) - { - if( mData.mFBO ==null ) - { - android.util.Log.e("DistortedNode", "creating a new FBO"); - - mData.mFBO = new DistortedFramebuffer(mSurface.getWidth(), mSurface.getHeight()); - } - } - else - { - if( mData.mFBO !=null ) - { - android.util.Log.e("DistortedNode", "marking FBO for deletion and setting it to NULL"); - - mData.mFBO.markForDeletion(); - mData.mFBO = null; - } - else - { - android.util.Log.e("DistortedNode", "adjustNodeData: impossible situation??"); - } - } - - mMapNodeID.put(newList, mData); - } - else - { - newData.numPointingNodes++; - mData = newData; - } - } - /////////////////////////////////////////////////////////////////////////////////////////////////// // this will be called on startup and every time OpenGL context has been lost @@ -174,14 +107,14 @@ public class DistortedNode /////////////////////////////////////////////////////////////////////////////////////////////////// // Debug - print all the Node IDs -/* + void debug(int depth) { String tmp=""; int i; for(i=0; i key: mMapNodeID.keySet()) { tmp = mMapNodeID.get(key); + android.util.Log.e("NODE", "NodeID: "+tmp.ID+" <-- "+key); + } + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + + void treeIsomorphism() + { + android.util.Log.e("NODE", "begin treeIso for Node Surface ID="+mSurface.getID()); + debug(0); + debugMap(); + android.util.Log.e("NODE", "begin treeIso for Node Surface ID="+mSurface.getID()); + + ArrayList oldList = generateIDList(); + + for(int i=0; i newList = generateIDList(); + NodeData newData = mMapNodeID.get(newList); + + if( newData==null ) + { + android.util.Log.d("NODE", " inserted newData to map, newList="+newList); + + + newData = new NodeData(++mNextNodeID); + mMapNodeID.put(newList,newData); + debugMap(); + } + else if( newData.ID != mData.ID ) + { + newData.numPointingNodes++; + } + + if( newData.ID != mData.ID ) + { + boolean fboUsed = false; + + if( mNumChildren[0]>0 && newData.mFBO==null ) + { + if( mData.mFBO!=null ) + { + newData.mFBO = mData.mFBO; + fboUsed = true; + } + else + { + newData.mFBO = new DistortedFramebuffer(mSurface.getWidth(),mSurface.getHeight()); + } + } + if( mNumChildren[0]==0 && newData.mFBO!=null ) + { + newData.mFBO.markForDeletion(); + newData.mFBO = null; + } + + if( --mData.numPointingNodes==0 ) + { + android.util.Log.d("NODE", " removed oldData to map, oldList="+oldList); + - android.util.Log.e("NODE", "key="+key+" NodeID: "+tmp.ID); + mMapNodeID.remove(oldList); + debugMap(); + + if( !fboUsed && mData.mFBO!=null ) + { + mData.mFBO.markForDeletion(); + mData.mFBO = null; + } + } + + mData = newData; } + + android.util.Log.e("NODE", "end treeIso for Node SurfaceID="+mSurface.getID()+" newList="+newList); + debug(0); + debugMap(); + android.util.Log.e("NODE", "end treeIso for Node SurfaceID="+mSurface.getID()+" newList="+newList); } -*/ + /////////////////////////////////////////////////////////////////////////////////////////////////// void drawRecursive(long currTime, DistortedOutputSurface surface) @@ -256,7 +267,6 @@ public class DistortedNode mSurface = surface; mEffects = effects; mMesh = mesh; - mParent = null; mChildren = null; mNumChildren = new int[1]; mNumChildren[0]= 0; @@ -289,7 +299,6 @@ public class DistortedNode */ public DistortedNode(DistortedNode node, int flags) { - mParent = null; mEffects= new DistortedEffects(node.mEffects,flags); mMesh = node.mMesh; @@ -314,6 +323,11 @@ public class DistortedNode } if( (flags & Distorted.CLONE_CHILDREN) != 0 ) { + if( node.mChildren==null ) // do NOT copy over the NULL! + { + node.mChildren = new ArrayList<>(2); + } + mChildren = node.mChildren; mNumChildren = node.mNumChildren; } @@ -338,82 +352,68 @@ public class DistortedNode mMapNodeID.put(list, mData); } } - + /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Adds a new child to the last position in the list of our Node's children. - * + *

+ * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling attachNow()) + * * @param node The new Node to add. */ - public synchronized void attach(DistortedNode node) + public void attach(DistortedNode node) { - ArrayList prev = generateIDList(); - - if( mChildren==null ) mChildren = new ArrayList<>(2); - - android.util.Log.e("DistortedNode", "ATTACH1"); - - - node.mParent = this; - mChildren.add(node); - mNumChildren[0]++; - - RecomputeNodeID(prev); + DistortedAttachDaemon.attach(this,node); } - + /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Adds a new child to the last position in the list of our Node's children. - * + *

+ * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling attachNow()) + * * @param surface InputSurface to initialize our child Node with. * @param effects DistortedEffects to initialize our child Node with. * @param mesh MeshObject to initialize our child Node with. * @return the newly constructed child Node, or null if we couldn't allocate resources. */ - public synchronized DistortedNode attach(DistortedInputSurface surface, DistortedEffects effects, MeshObject mesh) + public DistortedNode attach(DistortedInputSurface surface, DistortedEffects effects, MeshObject mesh) { - ArrayList prev = generateIDList(); - - android.util.Log.e("DistortedNode", "ATTACH2"); + DistortedNode node = new DistortedNode(surface,effects,mesh); + DistortedAttachDaemon.attach(this,node); + return node; + } +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. + * + * @param node The new Node to add. + */ + public void attachNow(DistortedNode node) + { if( mChildren==null ) mChildren = new ArrayList<>(2); - DistortedNode node = new DistortedNode(surface,effects,mesh); - node.mParent = this; + mChildren.add(node); mNumChildren[0]++; - - RecomputeNodeID(prev); - - return node; } - + /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Removes the first occurrence of a specified child from the list of children of our Node. - * + *

+ * We cannot do this mid-render - actual detachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling detachNow()) + * * @param node The Node to remove. - * @return true if the child was successfully removed. */ - public synchronized boolean detach(DistortedNode node) + public void detach(DistortedNode node) { - if( mNumChildren[0]>0 ) - { - ArrayList prev = generateIDList(); - - if( mChildren.remove(node) ) - { -android.util.Log.e("DistortedNode", "DETACH1"); - - node.mParent = null; - mNumChildren[0]--; - - RecomputeNodeID(prev); - - return true; - } - } - - return false; + DistortedAttachDaemon.detach(this,node); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -422,11 +422,13 @@ android.util.Log.e("DistortedNode", "DETACH1"); *

* A bit questionable method as there can be many different Nodes attached as children, some * of them having the same Effects but - for instance - different Mesh. Use with care. + *

+ * We cannot do this mid-render - actual detachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling detachNow()) * * @param effects DistortedEffects to remove. - * @return true if the child was successfully removed. */ - public synchronized boolean detach(DistortedEffects effects) + public void detach(DistortedEffects effects) { long id = effects.getID(); DistortedNode node; @@ -437,42 +439,52 @@ android.util.Log.e("DistortedNode", "DETACH1"); if( node.mEffects.getID()==id ) { -android.util.Log.e("DistortedNode", "DETACH2"); - - - ArrayList prev = generateIDList(); - - node.mParent = null; - mChildren.remove(i); - mNumChildren[0]--; - - RecomputeNodeID(prev); - - return true; + DistortedAttachDaemon.detach(this,node); + break; } } - - return false; } /////////////////////////////////////////////////////////////////////////////////////////////////// /** - * Removes all children Nodes. + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. + * + * @param node The Node to remove. */ - public synchronized void detachAll() + public void detachNow(DistortedNode node) { - for(int i=0; i0 && mChildren.remove(node) ) { - mChildren.get(i).mParent = null; + mNumChildren[0]--; } - + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * Removes all children Nodes. + *

+ * We cannot do this mid-render - actual detachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling detachAllNow()) + */ + public void detachAll() + { + DistortedAttachDaemon.detachAll(this); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. + */ + public void detachAllNow() + { if( mNumChildren[0]>0 ) { - ArrayList prev = generateIDList(); - mNumChildren[0] = 0; mChildren.clear(); - RecomputeNodeID(prev); } } diff --git a/src/main/java/org/distorted/library/DistortedOutputSurface.java b/src/main/java/org/distorted/library/DistortedOutputSurface.java index c26c8d0..75703a0 100644 --- a/src/main/java/org/distorted/library/DistortedOutputSurface.java +++ b/src/main/java/org/distorted/library/DistortedOutputSurface.java @@ -24,7 +24,7 @@ import java.util.ArrayList; /////////////////////////////////////////////////////////////////////////////////////////////////// -abstract class DistortedOutputSurface extends DistortedSurface +abstract class DistortedOutputSurface extends DistortedSurface implements DistortedAttacheable { private ArrayList mChildren; private int mNumChildren; // ==mChildren.length(), but we only create mChildren if the first one gets added @@ -99,6 +99,21 @@ abstract class DistortedOutputSurface extends DistortedSurface */ public void render(long time) { + // change tree topology (attach and detach children) + // create and delete all underlying OpenGL resources + // Watch out: FIRST change topology, only then deal + // with OpenGL resources. That's because changing Tree + // can result in additional Framebuffers that would need + // to be created immediately, before the calls to drawRecursive() + + if( DistortedAttachDaemon.toDo() ) + { + for(int i=0; i + * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling attachNow()) * * @param node The new Node to add. */ - public synchronized void attach(DistortedNode node) + public void attach(DistortedNode node) { - if( mChildren==null ) mChildren = new ArrayList<>(2); - mChildren.add(node); - mNumChildren++; + DistortedAttachDaemon.attach(this,node); } /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Adds a new child to the last position in the list of our Surface's children. + *

+ * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling attachNow()) * * @param surface InputSurface to initialize our child Node with. * @param effects DistortedEffects to initialize our child Node with. * @param mesh MeshObject to initialize our child Node with. * @return the newly constructed child Node, or null if we couldn't allocate resources. */ - public synchronized DistortedNode attach(DistortedInputSurface surface, DistortedEffects effects, MeshObject mesh) + public DistortedNode attach(DistortedInputSurface surface, DistortedEffects effects, MeshObject mesh) { - if( mChildren==null ) mChildren = new ArrayList<>(2); DistortedNode node = new DistortedNode(surface,effects,mesh); + DistortedAttachDaemon.attach(this,node); + return node; + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. + * + * @param node new Node to add. + */ + public void attachNow(DistortedNode node) + { + if( mChildren==null ) mChildren = new ArrayList<>(2); mChildren.add(node); mNumChildren++; - - return node; } /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Removes the first occurrence of a specified child from the list of children of our Surface. + *

+ * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling detachNow()) + * + * @param node The Node to remove. + */ + public void detach(DistortedNode node) + { + DistortedAttachDaemon.detach(this,node); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. * * @param node The Node to remove. - * @return true if the child was successfully removed. */ - public synchronized boolean detach(DistortedNode node) + public void detachNow(DistortedNode node) { if( mNumChildren>0 && mChildren.remove(node) ) { mNumChildren--; - return true; } - - return false; } /////////////////////////////////////////////////////////////////////////////////////////////////// /** * Removes all children Nodes. + *

+ * We cannot do this mid-render - actual attachment will be done just before the next render, by the + * DistortedAttachDeamon (by calling detachAllNow()) + */ + public void detachAll() + { + DistortedAttachDaemon.detachAll(this); + } + +/////////////////////////////////////////////////////////////////////////////////////////////////// +/** + * This is not really part of the public API. Has to be public only because it is a part of the + * DistortedAttacheable interface, which should really be a class that we extend here instead but + * Java has no multiple inheritance. */ - public synchronized void detachAll() + public void detachAllNow() { if( mNumChildren>0 ) { diff --git a/src/main/java/org/distorted/library/DistortedSurface.java b/src/main/java/org/distorted/library/DistortedSurface.java index 35fd901..d76e3e9 100644 --- a/src/main/java/org/distorted/library/DistortedSurface.java +++ b/src/main/java/org/distorted/library/DistortedSurface.java @@ -38,7 +38,9 @@ abstract class DistortedSurface private static boolean mToDo = false; private static LinkedList mDoneList = new LinkedList<>(); private static LinkedList mToDoList = new LinkedList<>(); + private static long mNextID = 0; + private long mID; private boolean mMarked; int[] mColorH = new int[1]; int mSizeX, mSizeY; // in screen space @@ -100,6 +102,7 @@ abstract class DistortedSurface } mToDo = true; + mNextID = 0; } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -121,6 +124,7 @@ abstract class DistortedSurface mSizeY = height; mColorH[0]= color; mMarked = false; + mID = mNextID++; if( color!=DONT_CREATE ) { @@ -152,7 +156,7 @@ abstract class DistortedSurface */ public long getID() { - return mColorH[0]; + return mID; } ///////////////////////////////////////////////////////////////////////////////////////////////////