]> gerrit.simantics Code Review - simantics/3d.git/blobdiff - org.simantics.g3d.vtk/src/org/simantics/g3d/vtk/common/AbstractVTKNodeMap.java
Fix removed nodes leaving visible graphical elements.
[simantics/3d.git] / org.simantics.g3d.vtk / src / org / simantics / g3d / vtk / common / AbstractVTKNodeMap.java
index 6194ec822a0d8ef51fa7a202be5bbd0e1eff3f62..4bcb5df271b53f5c3b8f8651d5146b1096e54540 100644 (file)
  *******************************************************************************/
 package org.simantics.g3d.vtk.common;
 
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
+import java.util.function.Consumer;
 
 import org.simantics.db.ReadGraph;
 import org.simantics.db.Session;
+import org.simantics.db.UndoContext;
 import org.simantics.db.WriteGraph;
 import org.simantics.db.common.request.ReadRequest;
 import org.simantics.db.common.request.WriteRequest;
@@ -37,7 +41,6 @@ import org.simantics.g3d.scenegraph.base.ParentNode;
 import org.simantics.objmap.exceptions.MappingException;
 import org.simantics.objmap.graph.IMapping;
 import org.simantics.objmap.graph.IMappingListener;
-import org.simantics.utils.datastructures.Callback;
 import org.simantics.utils.datastructures.MapList;
 import org.simantics.utils.datastructures.MapSet;
 import org.simantics.utils.datastructures.Pair;
@@ -45,23 +48,25 @@ import org.simantics.utils.ui.ExceptionUtils;
 
 import vtk.vtkProp;
 
-public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<E>, IMappingListener, RenderListener, NodeListener, UndoRedoSupport.ChangeListener{
+public abstract class AbstractVTKNodeMap<DBObject,E extends INode> implements VTKNodeMap<DBObject,E>, IMappingListener, RenderListener, NodeListener, UndoRedoSupport.ChangeListener{
 
        private static final boolean DEBUG = false;
        
        protected Session session;
-       protected IMapping<Object,E> mapping;
+       protected IMapping<DBObject, INode> mapping;
        protected VtkView view;
        
-       protected MapList<E, vtkProp> nodeToActor = new MapList<E, vtkProp>();
-       protected Map<vtkProp,E> actorToNode = new HashMap<vtkProp, E>();
+       private MapList<E, vtkProp> nodeToActor = new MapList<E, vtkProp>();
+       private Map<vtkProp,E> actorToNode = new HashMap<vtkProp, E>();
 
        protected ParentNode<E> rootNode;
        
        protected UndoRedoSupport undoRedoSupport;
        protected int undoOpCount = 0;
+       protected int redoOpCount = 0;
        protected boolean runUndo = false;
-       public AbstractVTKNodeMap(Session session, IMapping<Object,E> mapping, VtkView view, ParentNode<E> rootNode) {
+       protected boolean runRedo = false;
+       public AbstractVTKNodeMap(Session session, IMapping<DBObject,INode> mapping, VtkView view, ParentNode<E> rootNode) {
                this.session = session;
                this.mapping = mapping;
                this.view = view;
@@ -73,14 +78,14 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                undoRedoSupport = session.getService(UndoRedoSupport.class);
                undoRedoSupport.subscribe(this);
                try {
-                   undoOpCount = undoRedoSupport.getUndoContext(session).getAll().size();
+                       UndoContext undoContext = undoRedoSupport.getUndoContext(session); 
+                       undoOpCount = undoContext.getAll().size();
+                       redoOpCount = undoContext.getRedoList().size();
                } catch(DatabaseException e) {
-                   e.printStackTrace();
+                       e.printStackTrace();
                }
        }
-       
-       
-       
+
        protected abstract void addActor(E node);
        protected abstract void removeActor(E node);
        protected abstract void updateActor(E node,Set<String> ids);
@@ -104,10 +109,32 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        @SuppressWarnings("unchecked")
        @Override
        public Collection<vtkProp> getRenderObjects(INode node) {
-               return nodeToActor.getValues((E)node);
+               return nodeToActor.getValues((E) node);
+       }
+       
+       protected <T extends vtkProp> void map(E node, Collection<T> props) {
+           for (vtkProp p : props) {
+               nodeToActor.add(node, p);
+               actorToNode.put(p, node);
+           }
+       }
+       
+       protected void removeMap(E node) {
+           Collection<vtkProp> coll = nodeToActor.getValuesUnsafe(node);
+           if (coll.size() > 0) {
+               view.lock();
+           for (vtkProp p : coll) {
+               actorToNode.remove(p);
+               if (p.GetVTKId() != 0) {
+                    view.getRenderer().RemoveActor(p);
+                    p.Delete();
+                }
+           }
+           view.unlock();
+           }
+           nodeToActor.remove(node);
        }
        
-       @SuppressWarnings("unchecked")
        @Override
        public ParentNode<E> getRootNode() {
                return (ParentNode<E>)rootNode;
@@ -132,36 +159,47 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
 
        private List<Pair<E,String>> added = new ArrayList<Pair<E,String>>();
        private List<Pair<E,String>> removed = new ArrayList<Pair<E,String>>();
-       //private List<Pair<E,String>> updated = new ArrayList<Pair<E,String>>();
        private MapSet<E, String> updated = new MapSet.Hash<E, String>();
 
        private boolean rangeModified = false;
        
+       public boolean isRangeModified() {
+        return rangeModified;
+    }
+       
        @Override
        public void onChanged() {
-           try {
-               int count = undoRedoSupport.getUndoContext(session).getAll().size();
-            if (count < undoOpCount) {
-                runUndo = true;
-            } else {
-                runUndo = false;
-            }
-            undoOpCount = count;
-            if (DEBUG) System.out.println("Undo " + runUndo);
-        } catch (DatabaseException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        }
-           
-           
+               try {
+                       UndoContext undoContext = undoRedoSupport.getUndoContext(session);
+                       int ucount = undoContext.getAll().size();
+                       int rcount = undoContext.getRedoList().size();
+                       if (DEBUG) System.out.println("Previous U:" + undoOpCount +" R:" + redoOpCount +" Current U:"+ucount+" R:"+rcount);
+                       if (ucount < undoOpCount) {
+                               runUndo = true;
+                       } else {
+                               runUndo = false;
+                       }
+                       if (!runUndo && rcount > 0)
+                               runRedo = true;
+                       else
+                               runRedo = false;
+                       undoOpCount = ucount;
+                       redoOpCount = rcount;
+                       
+                       if (DEBUG) System.out.println("Undo " + runUndo + " Redo " + runRedo);
+               } catch (DatabaseException e) {
+                       // TODO Auto-generated catch block
+                       e.printStackTrace();
+               }
+               
+               
        }
        
-       @SuppressWarnings("unchecked")
        @Override
-       public void updateRenderObjectsFor(INode node) {
+       public void updateRenderObjectsFor(E node) {
                List<vtkProp> toDelete = new ArrayList<vtkProp>();
                view.lock();
-               for (vtkProp prop : nodeToActor.getValues((E)node)) {
+               for (vtkProp prop : nodeToActor.getValues(node)) {
                        if (prop.GetVTKId() != 0) {
                                view.getRenderer().RemoveActor(prop);
                                //prop.Delete();
@@ -170,14 +208,14 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        actorToNode.remove(prop);
                }
                view.unlock();
-               nodeToActor.remove((E)node);
-               Collection<vtkProp> coll = getActors((E)node);
-               if (coll == null)
-                       return;
-               for (vtkProp prop : coll) {
-                       nodeToActor.add((E)node,prop);
-                       actorToNode.put(prop, (E)node);
-                       toDelete.remove(prop);
+               nodeToActor.remove(node);
+               Collection<vtkProp> coll = getActors(node);
+               if (coll != null) {
+                       for (vtkProp prop : coll) {
+                               nodeToActor.add(node,prop);
+                               actorToNode.put(prop, node);
+                               toDelete.remove(prop);
+                       }
                }
                for (vtkProp p : toDelete)
                        p.Delete();
@@ -195,11 +233,12 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        }
                        if (changeTracking) {
                                mapping.rangeModified((E)node.getParent());
+                               mapping.rangeModified((E)node);
                        }
                        added.add(new Pair<E, String>(node, id));
                        rangeModified = true;
                }
-               view.refresh();
+               repaint();
        }
        
        @SuppressWarnings("unchecked")
@@ -210,29 +249,30 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                                if (n.first.equals(node))
                                        return;
                        }
-                       if (changeTracking && !db)
+                       if (changeTracking && !db) {
+                           mapping.rangeModified((E)node);
                                mapping.rangeModified((E)node.getParent());
+                       }
                        removed.add(new Pair<E, String>(node, id));
                        rangeModified = true;
                }
-               view.refresh();
+               repaint();
        }
        
-       @SuppressWarnings("unchecked")
        private void receiveUpdate(E node, String id, boolean db) {
                if (DEBUG) System.out.println("receiveUpdate " + debugString(node)  + " " + id + " " + db);
                synchronized (syncMutex) {
-//                     for (Pair<E, String> n : updated) {
-//                             if (n.first.equals(node))
-//                                     return;
-//                     }
+//          for (Pair<E, String> n : updated) {
+//              if (n.first.equals(node))
+//                  return;
+//          }
                        if (changeTracking && !db)
                                mapping.rangeModified(node);
                        //updated.add(new Pair<E, String>(node, id));
                        updated.add(node, id);
                        rangeModified = true;
                }
-               view.refresh();
+               repaint();
        }
        
        private boolean graphUpdates = false;
@@ -252,19 +292,19 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        
                        @Override
                        public void perform(WriteGraph graph) throws DatabaseException {
-                           if (DEBUG) System.out.println("Commit " + commitMessage);
+                               if (DEBUG) System.out.println("Commit " + commitMessage);
                                if (commitMessage != null) {
-                                   Layer0Utils.addCommentMetadata(graph, commitMessage);
-                                   graph.markUndoPoint();
-                                   commitMessage = null;
+                                       Layer0Utils.addCommentMetadata(graph, commitMessage);
+                                       graph.markUndoPoint();
+                                       commitMessage = null;
                                }
-                       commit(graph);
+                               commit(graph);
                        }
                        
-               }, new Callback<DatabaseException>() {
+               }, new Consumer<DatabaseException>() {
                        
                        @Override
-                       public void run(DatabaseException parameter) {
+                       public void accept(DatabaseException parameter) {
                                if (parameter != null)
                                        ExceptionUtils.logAndShowError("Cannot commit editor changes", parameter);
                        }
@@ -278,7 +318,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        mapping.updateDomain(graph);
                        graphUpdates = false;
                        clearDeletes();
-               if (DEBUG) System.out.println("Commit done");
+                       if (DEBUG) System.out.println("Commit done");
                }
        }
        
@@ -291,7 +331,6 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                if (DEBUG)System.out.println("domainModified");
                session.asyncRequest(new ReadRequest() {
                        
-                       @SuppressWarnings("unchecked")
                        @Override
                        public void run(ReadGraph graph) throws DatabaseException {
                                update(graph);
@@ -301,42 +340,43 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        }
        
        protected void reset(ReadGraph graph) throws MappingException {
-           if (DEBUG) System.out.println("Reset");
-        synchronized (syncMutex) {
-            graphUpdates = true;
-            mapping.getRangeModified().clear();
-            for (Object o : mapping.getDomain())
-                mapping.domainModified(o);
-            mapping.updateRange(graph);
-            graphModified.clear();
-            graphUpdates = false;
-        }
+               if (DEBUG) System.out.println("Reset");
+               
+               synchronized (syncMutex) {
+                       graphUpdates = true;
+                       mapping.getRangeModified().clear();
+                       for (DBObject o : mapping.getDomain())
+                               mapping.domainModified(o);
+                       mapping.updateRange(graph);
+                       graphModified.clear();
+                       graphUpdates = false;
+               }
        }
        
        private boolean useFullSyncWithUndo = false;
        
        protected void update(ReadGraph graph) throws DatabaseException {
-           if (DEBUG) System.out.println("Graph update start");
-           if (runUndo && useFullSyncWithUndo) {
-               reset(graph);
-           } else {
-               synchronized (syncMutex) {
-                       graphUpdates = true;
-                       for (Object domainObject : mapping.getDomainModified()) {
-                               E rangeObject = mapping.get(domainObject);
-                               if (rangeObject != null)
-                                       graphModified.add(rangeObject);
-                       }
-                       mapping.updateRange(graph);
-                       graphModified.clear();
-                       syncDeletes();
-                       clearDeletes();
-                       graphUpdates = false;
+               if (DEBUG) System.out.println("Graph update start");
+               if (runUndo && useFullSyncWithUndo) {
+                       reset(graph);
+               } else {
+                   synchronized (syncMutex) {
+                               graphUpdates = true;
+                               for (DBObject domainObject : mapping.getDomainModified()) {
+                                       @SuppressWarnings("unchecked")
+                                       E rangeObject = (E) mapping.get(domainObject);
+                                       if (rangeObject != null)
+                                               graphModified.add(rangeObject);
+                               }
+                               mapping.updateRange(graph);
+                               graphModified.clear();
+                               syncDeletes();
+                               clearDeletes();
+                               graphUpdates = false;
                }
-           }
+               }
                
-               //if (mapping.isRangeModified() && !runUndo) // FIXME : redo?
-           if (mapping.isRangeModified())
+               if (mapping.isRangeModified() && !runUndo && !runRedo)
                        commit((String)null);
                if (DEBUG) System.out.println("Graph update done");
        }
@@ -375,43 +415,56 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        
        /**
         * When objects are removed (either from Java or Graph), after remove processing the Java objects remain in mapping cache.
-        * This causes problems with Undo and Redo, whcih the end up re-using the removed objects from mapping cache.
+        * This causes problems with Undo and Redo, which cause re-using the removed objects from mapping cache.
         * 
         * This code here synchronizes removed and added objects to collect deletable objects. (a deletable object is one which is removed but not added).  
         * 
         */
+       @SuppressWarnings("unused")
        protected void syncDeletes() {
-           deleteUC.clear();
-           for (Pair<E, String> n : removed) {
-            deleteUC.add(n.first);   
-         }
-         for (Pair<E, String> n : added) {
-             deleteUC.remove(n.first);   
-         } 
-         if (DEBUG && deleteUC.size() > 0) {
-             System.out.println("Delete sync");
-             for (E n : delete) {
-                 System.out.println(debugString(n));
-             }
-         }
-         delete.addAll(deleteUC);
-         deleteUC.clear();
+               deleteUC.clear();
+               for (Pair<E, String> n : removed) {
+                       deleteUC.add(n.first);   
+               }
+               for (Pair<E, String> n : added) {
+                       deleteUC.remove(n.first);   
+               
+               if (DEBUG && deleteUC.size() > 0) {
+                       System.out.println("Delete sync");
+                       for (E n : delete) {
+                               System.out.println(debugString(n));
+                       }
+               }
+               delete.addAll(deleteUC);
+               deleteUC.clear();
        }
-       
+
        /**
         * Clears deletable objects from mapping cache.
         */
+       @SuppressWarnings("unused")
        protected void clearDeletes() {
-        if (DEBUG && delete.size() > 0) System.out.println("Delete");
-        for (E n : delete) {
-            if (DEBUG) System.out.println(debugString(n));
-            mapping.getRange().remove(n);
-        }
-        delete.clear();
-    }
+               if (DEBUG && delete.size() > 0) System.out.println("Delete");
+               for (E n : delete) {
+                       if (DEBUG) System.out.println(debugString(n));
+                       mapping.getRange().remove(n);
+                       stopListening(n);
+               }
+               delete.clear();
+       }
        
        protected String debugString(E n) {
-           return n + "@" + Integer.toHexString(n.hashCode());
+               return n + "@" + Integer.toHexString(n.hashCode());
+       }
+       
+       protected boolean filterChange(List<Pair<E,String>> list,E n) {
+           for (int i = list.size()-1; i >= 0; i--) {
+            if (list.get(i).first == n) {
+                list.remove(i);
+                return true;
+            }
+        }
+           return false;
        }
        
        @SuppressWarnings("unchecked")
@@ -423,6 +476,31 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                
                
                synchronized (syncMutex) {
+                   // Check for overlapping additions and deletions, prevent deleting objects that are also added and vice versa.
+                   Deque<E> stack = new ArrayDeque<E>();
+               for (Pair<E, String> n : added) {
+                   stack.add(n.first);
+               }
+               while (!stack.isEmpty()) {
+                   E n = stack.pop();
+                   boolean conflict = filterChange(removed, n);
+                   if (conflict) {
+                       if (DEBUG) System.out.println("Prevent removing " + n);
+                       //filterChange(added, n)
+                       if (filterChange(added, n))
+                          if (DEBUG) System.out.println("Prevent adding " + n);
+                   }
+                   if (n instanceof ParentNode) {
+                       ParentNode<INode> pn = (ParentNode<INode>)n;
+                       for (INode cn : pn.getNodes()) {
+                           stack.push((E)cn);
+                       }
+                   }
+               }
+               // Do not process updates for removed nodes.
+               for (Pair<E, String> r : removed) {
+                   updated.removeValues(r.first);
+               }
                        rem.addAll(removed);
                        add.addAll(added);
                        for (E e : updated.getKeys()) {
@@ -436,15 +514,17 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        updated.clear();
                }
                
+               
                for (Pair<E, String> n : rem) {
                        stopListening(n.first);
                        removeActor(n.first);
+                       n.first.remove();
                }
                
-               for (Pair<E, String> n : add) {
-                       addActor(n.first);
-                       listen(n.first);
-               }
+           for (Pair<E, String> n : add) {
+               addActor(n.first);
+               listen(n.first);
+           }
                
                for (E e : mod.getKeys()) {
                        Set<String> ids = mod.getValues(e);
@@ -478,26 +558,11 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        }
                }
                
-//             synchronized (syncMutex) {
-//                     rem.addAll(removed);
-//                     add.addAll(added);
-//                     //mod.addAll(updated);
-//                     for (E e : updated.getKeys()) {
-//                             for (String s : updated.getValues(e))
-//                                     mod.add(e, s);
-//                     }
-//                     
-//                     removed.clear();
-//                     added.clear();
-//                     updated.clear();
-//             }
-               
                for (E e : mod.getKeys()) {
                        Set<String> ids = mod.getValues(e);
                        updateActor(e,ids);
                }
-               
-               
+
                for (Pair<E, String> n : rem) {
                        for (NodeListener l : nodeListeners)
                                l.nodeRemoved(null, n.first, n.second);
@@ -506,10 +571,10 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        for (NodeListener l : nodeListeners)
                                l.nodeAdded(n.first.getParent(), n.first, n.second);
                }
-//             for (Pair<E, String> n : mod) {
-//                     for (NodeListener l : nodeListeners)
-//                             l.propertyChanged(n.first, n.second);
-//             }
+//      for (Pair<E, String> n : mod) {
+//          for (NodeListener l : nodeListeners)
+//              l.propertyChanged(n.first, n.second);
+//      }
                for (E e : mod.getKeys()) {
                        for (NodeListener l : nodeListeners)
                                for (String s : mod.getValues(e))
@@ -517,7 +582,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                }
                
                synchronized (syncMutex) {
-                   if (added.isEmpty() && removed.isEmpty() && updated.getKeys().size() == 0)
+                       if (added.isEmpty() && removed.isEmpty() && updated.getKeys().size() == 0)
                                rangeModified = false;
                }
        }
@@ -557,7 +622,6 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                if (DEBUG) System.out.println("Node added " + child + " parent " + node);
                //receiveAdd((E)child, rel ,graphUpdates);
                receiveAdd((E)child, rel ,graphModified.contains(node));
-               
        }
        
        @SuppressWarnings("unchecked")
@@ -568,16 +632,17 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                //receiveRemove((E)child, rel, graphUpdates);
                receiveRemove((E)child, rel, graphModified.contains(node));
                
-               //FIXME : sometimes removed structural models cause ObjMap to add their children again.
-               //        removing the listener here prevents corruption of visual model, but better fix is needed.
-               stopListening(child);
+               //FIXME : 1. sometimes removed structural models cause ObjMap to add their children again.
+               //           removing the listener here prevents corruption of visual model, but better fix is needed.
+               //        2. detach causes nodeRemoved event, which then causes other critical events to be missed. Took out call: 
+               //stopListening(child);
        }
        
        @Override
        public void delete() {
-           if (undoRedoSupport != null)
-               undoRedoSupport.cancel(this);
-        
+               if (undoRedoSupport != null)
+                       undoRedoSupport.cancel(this);
+               
                changeTracking = false;
                view.removeListener(this);
                mapping.removeMappingListener(this);
@@ -612,7 +677,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                
        }
        
-       public IMapping<Object,E> getMapping() {
+       public IMapping<DBObject,INode> getMapping() {
                return mapping;
        }