]> gerrit.simantics Code Review - simantics/3d.git/blobdiff - org.simantics.g3d.vtk/src/org/simantics/g3d/vtk/common/AbstractVTKNodeMap.java
Remove static DEBUG flags and use slf4j.Logger.trace()
[simantics/3d.git] / org.simantics.g3d.vtk / src / org / simantics / g3d / vtk / common / AbstractVTKNodeMap.java
index 674a71f2987953f0cf1ace01014b1deb9a68103e..33a3154e798f96dca9a7dc993cd869cde3dac10e 100644 (file)
  *******************************************************************************/
 package org.simantics.g3d.vtk.common;
 
+import java.lang.reflect.InvocationTargetException;
+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;
@@ -21,14 +24,17 @@ import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
 
+import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.progress.IProgressService;
 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.UniqueRead;
 import org.simantics.db.common.request.WriteRequest;
 import org.simantics.db.exception.DatabaseException;
 import org.simantics.db.layer0.util.Layer0Utils;
+import org.simantics.db.procedure.SyncProcedure;
 import org.simantics.db.service.UndoRedoSupport;
 import org.simantics.g3d.ontology.G3D;
 import org.simantics.g3d.scenegraph.RenderListener;
@@ -38,24 +44,25 @@ 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;
 import org.simantics.utils.ui.ExceptionUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 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;
+       private static final Logger LOGGER = LoggerFactory.getLogger(AbstractVTKNodeMap.class);
        
        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;
        
@@ -64,7 +71,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        protected int redoOpCount = 0;
        protected boolean runUndo = false;
        protected boolean runRedo = false;
-       public AbstractVTKNodeMap(Session session, IMapping<Object,E> mapping, VtkView view, ParentNode<E> rootNode) {
+       public AbstractVTKNodeMap(Session session, IMapping<DBObject,INode> mapping, VtkView view, ParentNode<E> rootNode) {
                this.session = session;
                this.mapping = mapping;
                this.view = view;
@@ -80,12 +87,10 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        undoOpCount = undoContext.getAll().size();
                        redoOpCount = undoContext.getRedoList().size();
                } catch(DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("Error reading from undo context", e);
                }
        }
-       
-       
-       
+
        protected abstract void addActor(E node);
        protected abstract void removeActor(E node);
        protected abstract void updateActor(E node,Set<String> ids);
@@ -109,10 +114,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;
@@ -141,13 +168,18 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
 
        private boolean rangeModified = false;
        
+       public boolean isRangeModified() {
+        return rangeModified;
+    }
+       
        @Override
        public void onChanged() {
                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 (LOGGER.isTraceEnabled()) LOGGER.trace("Previous U:" + undoOpCount +" R:" + redoOpCount +" Current U:"+ucount+" R:"+rcount);
                        if (ucount < undoOpCount) {
                                runUndo = true;
                        } else {
@@ -160,7 +192,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        undoOpCount = ucount;
                        redoOpCount = rcount;
                        
-                       if (DEBUG) System.out.println("Undo " + runUndo + " Redo " + runRedo);
+                       if (LOGGER.isTraceEnabled()) LOGGER.trace("Undo " + runUndo + " Redo " + runRedo);
                } catch (DatabaseException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
@@ -169,12 +201,11 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                
        }
        
-       @SuppressWarnings("unchecked")
        @Override
        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();
@@ -183,12 +214,12 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                        actorToNode.remove(prop);
                }
                view.unlock();
-               nodeToActor.remove((E)node);
-               Collection<vtkProp> coll = getActors((E)node);
+               nodeToActor.remove(node);
+               Collection<vtkProp> coll = getActors(node);
                if (coll != null) {
                        for (vtkProp prop : coll) {
-                               nodeToActor.add((E)node,prop);
-                               actorToNode.put(prop, (E)node);
+                               nodeToActor.add(node,prop);
+                               actorToNode.put(prop, node);
                                toDelete.remove(prop);
                        }
                }
@@ -200,7 +231,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        
        @SuppressWarnings("unchecked")
        private void receiveAdd(E node, String id, boolean db) {
-               if (DEBUG) System.out.println("receiveAdd " + debugString(node)  + " " + id + " " + db);
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("receiveAdd " + debugString(node)  + " " + id + " " + db);
                synchronized (syncMutex) {
                        for (Pair<E, String> n : added) {
                                if (n.first.equals(node))
@@ -208,32 +239,34 @@ 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")
        private void receiveRemove(E node, String id, boolean db) {
-               if (DEBUG) System.out.println("receiveRemove " + debugString(node)  + " " + id + " " + db);
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("receiveRemove " + debugString(node)  + " " + id + " " + db);
                synchronized (syncMutex) {
                        for (Pair<E, String> n : removed) {
                                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;
                }
                repaint();
        }
        
-       @SuppressWarnings("unchecked")
        private void receiveUpdate(E node, String id, boolean db) {
-               if (DEBUG) System.out.println("receiveUpdate " + debugString(node)  + " " + id + " " + db);
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("receiveUpdate " + debugString(node)  + " " + id + " " + db);
                synchronized (syncMutex) {
 //          for (Pair<E, String> n : updated) {
 //              if (n.first.equals(node))
@@ -261,37 +294,42 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        }
        
        protected void doCommit() {
-               session.asyncRequest(new WriteRequest() {
-                       
-                       @Override
-                       public void perform(WriteGraph graph) throws DatabaseException {
-                               if (DEBUG) System.out.println("Commit " + commitMessage);
-                               if (commitMessage != null) {
-                                       Layer0Utils.addCommentMetadata(graph, commitMessage);
-                                       graph.markUndoPoint();
-                                       commitMessage = null;
+               IProgressService service = PlatformUI.getWorkbench().getProgressService();
+               try {
+                       service.busyCursorWhile(monitor -> {
+                               try {
+                                       session.syncRequest(new WriteRequest() {
+                                               @Override
+                                               public void perform(WriteGraph graph) throws DatabaseException {
+                                                       if (LOGGER.isTraceEnabled()) LOGGER.trace("Commit " + commitMessage);
+                                                       if (commitMessage != null) {
+                                                               Layer0Utils.addCommentMetadata(graph, commitMessage);
+                                                               graph.markUndoPoint();
+                                                               commitMessage = null;
+                                                       }
+                                                       commit(graph);
+                                               }
+                                       });
+                               } catch (DatabaseException e) {
+                                       ExceptionUtils.logAndShowError("Cannot commit editor changes", e);
                                }
-                               commit(graph);
-                       }
-                       
-               }, new Callback<DatabaseException>() {
-                       
-                       @Override
-                       public void run(DatabaseException parameter) {
-                               if (parameter != null)
-                                       ExceptionUtils.logAndShowError("Cannot commit editor changes", parameter);
-                       }
-               });
+                       });
+                       postCommit();
+               } catch (InvocationTargetException | InterruptedException e) {
+                       LOGGER.error("Unexpected exception", e);
+               }
        }
-       
+
+       protected void postCommit() {}
+
        protected void commit(WriteGraph graph) throws DatabaseException {
                synchronized(syncMutex) {
-                       if (DEBUG) System.out.println("Commit");
+                       if (LOGGER.isTraceEnabled()) LOGGER.trace("Commit");
                        graphUpdates = true;
                        mapping.updateDomain(graph);
                        graphUpdates = false;
                        clearDeletes();
-                       if (DEBUG) System.out.println("Commit done");
+                       if (LOGGER.isTraceEnabled()) LOGGER.trace("Commit done");
                }
        }
        
@@ -301,61 +339,79 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        public void domainModified() {
                if (graphUpdates)
                        return;
-               if (DEBUG)System.out.println("domainModified");
-               session.asyncRequest(new ReadRequest() {
-                       
-                       @SuppressWarnings("unchecked")
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("domainModified");
+               session.asyncRequest(new UniqueRead<Object>() {
+                       @Override
+                       public Object perform(ReadGraph graph) throws DatabaseException {
+                               return new Object();
+                       }
+               }, new SyncProcedure<Object>() {
                        @Override
-                       public void run(ReadGraph graph) throws DatabaseException {
+                       public void execute(ReadGraph graph, Object result) throws DatabaseException {
+                               // Perform all updates to the model in a single query thread
                                update(graph);
+                       };
+                       
+                       @Override
+                       public void exception(ReadGraph graph, Throwable throwable) throws DatabaseException {
+                               LOGGER.error("Failed to update pipeline changes" + throwable);
                        }
                });
-               
        }
        
        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 (LOGGER.isTraceEnabled()) LOGGER.trace("Reset");
+               
+               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 (LOGGER.isTraceEnabled()) LOGGER.trace("Graph update start");
+               
                if (runUndo && useFullSyncWithUndo) {
-                       reset(graph);
+                       synchronized (syncMutex) {
+                               reset(graph);
+                       }
                } else {
                        synchronized (syncMutex) {
                                graphUpdates = true;
-                               for (Object domainObject : mapping.getDomainModified()) {
-                                       E rangeObject = mapping.get(domainObject);
+                               for (DBObject domainObject : mapping.getDomainModified()) {
+                                       @SuppressWarnings("unchecked")
+                                       E rangeObject = (E) mapping.get(domainObject);
                                        if (rangeObject != null)
                                                graphModified.add(rangeObject);
                                }
-                               mapping.updateRange(graph);
+                       
+                       }
+                       
+                       mapping.updateRange(graph);
+                       
+                       synchronized (syncMutex) {
                                graphModified.clear();
                                syncDeletes();
-                               clearDeletes();
-                               graphUpdates = false;
                        }
+                       
+                       clearDeletes();
+                       graphUpdates = false;
                }
                
                if (mapping.isRangeModified() && !runUndo && !runRedo)
                        commit((String)null);
-               if (DEBUG) System.out.println("Graph update done");
+               
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("Graph update done");
        }
        
        @Override
        public void rangeModified() {
-               //System.out.println("rangeModified");
+               //LOGGER.trace("rangeModified");
 
        }
        
@@ -387,7 +443,7 @@ 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).  
         * 
@@ -400,10 +456,10 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                for (Pair<E, String> n : added) {
                        deleteUC.remove(n.first);   
                } 
-               if (DEBUG && deleteUC.size() > 0) {
-                       System.out.println("Delete sync");
+               if (LOGGER.isTraceEnabled() && deleteUC.size() > 0) {
+                       LOGGER.trace("Delete sync");
                        for (E n : delete) {
-                               System.out.println(debugString(n));
+                               LOGGER.trace(debugString(n));
                        }
                }
                delete.addAll(deleteUC);
@@ -414,10 +470,11 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
         * Clears deletable objects from mapping cache.
         */
        protected void clearDeletes() {
-               if (DEBUG && delete.size() > 0) System.out.println("Delete");
+               if (LOGGER.isTraceEnabled() && delete.size() > 0) LOGGER.trace("Delete");
                for (E n : delete) {
-                       if (DEBUG) System.out.println(debugString(n));
+                       if (LOGGER.isTraceEnabled()) LOGGER.trace(debugString(n));
                        mapping.getRange().remove(n);
+                       stopListening(n);
                }
                delete.clear();
        }
@@ -426,6 +483,16 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                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")
        protected void updateCycle() {
                rem.clear();
@@ -435,6 +502,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 (LOGGER.isTraceEnabled()) LOGGER.trace("Prevent removing " + n);
+                       //filterChange(added, n)
+                       if (filterChange(added, n))
+                          if (LOGGER.isTraceEnabled()) LOGGER.trace("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()) {
@@ -448,15 +540,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);
@@ -490,26 +584,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);
@@ -566,23 +645,23 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
        @Override
        public <T extends INode> void nodeAdded(ParentNode<T> node, INode child,
                        String rel) {
-               if (DEBUG) System.out.println("Node added " + child + " parent " + node);
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("Node added " + child + " parent " + node);
                //receiveAdd((E)child, rel ,graphUpdates);
                receiveAdd((E)child, rel ,graphModified.contains(node));
-               
        }
        
        @SuppressWarnings("unchecked")
        @Override
        public <T extends INode> void nodeRemoved(ParentNode<T> node, INode child,
                        String rel) {
-               if (DEBUG) System.out.println("Node removed " + child + " parent " + node);
+               if (LOGGER.isTraceEnabled()) LOGGER.trace("Node removed " + child + " parent " + node);
                //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
@@ -624,7 +703,7 @@ public abstract class AbstractVTKNodeMap<E extends INode> implements VTKNodeMap<
                
        }
        
-       public IMapping<Object,E> getMapping() {
+       public IMapping<DBObject,INode> getMapping() {
                return mapping;
        }