From: Tuukka Lehtonen Date: Thu, 8 Nov 2018 09:51:17 +0000 (+0200) Subject: Usability fixes for GraphExplorerImpl -related WB selection propagation X-Git-Tag: v1.43.0~136^2~288 X-Git-Url: https://gerrit.simantics.org/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F33%2F2433%2F5;p=simantics%2Fplatform.git Usability fixes for GraphExplorerImpl -related WB selection propagation GraphExplorerImpl now uses JFace's OpenStrategy to implement the post selection provider mechanism which should work just like in JFace Viewers. SWTExplorer no longer listens to both selection and postSelection changes, which caused large amounts of unnecessary selection propagation to happen. It listens to postSelection if IPostSelectionProvider is available and selection if not. ModelledView did not implement IPostSelectionProvider at all and provided just an ISelectionProvider to the workbench. This partially caused other parts, like the property view to always react to model browser selection changes via immediate selection events instad of post selection events. The important lesson here is to just listen to either postSelection or selection, not both. gitlab #184 gitlab #185 Change-Id: I57fd7b6663bfa06f05105f6fa01e8d9a710c0ce0 --- diff --git a/bundles/org.simantics.browsing.ui.platform/src/org/simantics/browsing/ui/platform/PropertyPageView.java b/bundles/org.simantics.browsing.ui.platform/src/org/simantics/browsing/ui/platform/PropertyPageView.java index 0d4fc055b..05109c813 100644 --- a/bundles/org.simantics.browsing.ui.platform/src/org/simantics/browsing/ui/platform/PropertyPageView.java +++ b/bundles/org.simantics.browsing.ui.platform/src/org/simantics/browsing/ui/platform/PropertyPageView.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2010 Association for Decentralized Information Management + * Copyright (c) 2007, 2018 Association for Decentralized Information Management * in Industry THTH ry. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -11,17 +11,10 @@ *******************************************************************************/ package org.simantics.browsing.ui.platform; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.WeakHashMap; import java.util.function.Consumer; -import org.eclipse.core.runtime.IConfigurationElement; -import org.eclipse.core.runtime.IExtension; -import org.eclipse.core.runtime.IExtensionPoint; -import org.eclipse.core.runtime.IExtensionRegistry; -import org.eclipse.core.runtime.RegistryFactory; import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.jface.resource.JFaceResources; import org.eclipse.jface.resource.LocalResourceManager; @@ -38,7 +31,6 @@ import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.IWorkbenchPart; import org.eclipse.ui.IWorkbenchPart3; import org.eclipse.ui.PartInitException; -import org.eclipse.ui.PlatformUI; import org.eclipse.ui.contexts.IContextService; import org.eclipse.ui.part.IContributedContentsView; import org.eclipse.ui.part.IPage; @@ -49,8 +41,6 @@ import org.simantics.db.management.ISessionContextProvider; import org.simantics.selectionview.PropertyPage; import org.simantics.ui.workbench.IPropertyPage; import org.simantics.ui.workbench.ResourceInput; -import org.simantics.utils.threads.SWTThread; -import org.simantics.utils.threads.Throttler; import org.simantics.utils.ui.BundleUtils; import org.simantics.utils.ui.SWTUtils; @@ -91,19 +81,12 @@ import org.simantics.utils.ui.SWTUtils; * @see IPropertyPage * @see PropertyPage */ -public class PropertyPageView extends PageBookView implements ISelectionListener, IContributedContentsView { - - /** - * Extension point used to modify behavior of the view - */ - private static final String EXT_POINT = "org.eclipse.ui.propertiesView"; //$NON-NLS-1$ +public class PropertyPageView extends PageBookView implements IContributedContentsView { private static final String PROPERTY_VIEW_CONTEXT = "org.simantics.modeling.ui.properties"; private static final String PROP_PINNED = "pinned"; - protected static final long SELECTION_CHANGE_THRESHOLD = 500; - private ISessionContextProvider contextProvider; /** @@ -124,7 +107,7 @@ public class PropertyPageView extends PageBookView implements ISelectionListener private IWorkbenchPart lastPart; private ISelection lastSelection; - private final Map lastSelections = new WeakHashMap(); + private final Map lastSelections = new WeakHashMap<>(); private ResourceManager resourceManager; @@ -132,9 +115,10 @@ public class PropertyPageView extends PageBookView implements ISelectionListener private ImageDescriptor pinned; /** - * Set of workbench parts, which should not be used as a source for PropertySheet + * The workbench post-selection listener for this view that changes the + * input of the pagebook page for the part which changed its selection. */ - private Set ignoredViews; + private ISelectionListener selectionListener = this::doSelectionChanged; @Override public void createPartControl(Composite parent) { @@ -211,13 +195,12 @@ public class PropertyPageView extends PageBookView implements ISelectionListener // This prevents the Properties view from providing a selection to other // workbench parts, thus making them lose their selections which is not // desirable. -// site.setSelectionProvider(null); + site.setSelectionProvider(null); contextProvider = Simantics.getSessionContextProvider(); if (!bootstrapOnly) { - site.getPage().addSelectionListener(immediateSelectionListener); - site.getPage().addPostSelectionListener(this); + site.getPage().addPostSelectionListener(selectionListener); } } @@ -246,8 +229,7 @@ public class PropertyPageView extends PageBookView implements ISelectionListener // Remove ourselves as a workbench selection listener. if (!bootstrapOnly) { - getSite().getPage().removePostSelectionListener(this); - getSite().getPage().removeSelectionListener(immediateSelectionListener); + getSite().getPage().removePostSelectionListener(selectionListener); } if (resourceManager != null) { @@ -264,7 +246,7 @@ public class PropertyPageView extends PageBookView implements ISelectionListener page.createControl(book); page.setMessage(Messages.PropertyPageView_noPropertiesAvailable); return page; - */ + */ PropertyPage page = new PropertyPage(getSite()); initPage(page); @@ -276,7 +258,6 @@ public class PropertyPageView extends PageBookView implements ISelectionListener @Override protected PageRec doCreatePage(IWorkbenchPart part) { - // NOTE: If the default page should be shown, this method must return null. if (part == null) return null; @@ -334,37 +315,10 @@ public class PropertyPageView extends PageBookView implements ISelectionListener return this == part || ignore; } - private Set getIgnoredViews() { - if (ignoredViews == null) { - ignoredViews = new HashSet(); - IExtensionRegistry registry = RegistryFactory.getRegistry(); - IExtensionPoint ep = registry.getExtensionPoint(EXT_POINT); - if (ep != null) { - IExtension[] extensions = ep.getExtensions(); - for (int i = 0; i < extensions.length; i++) { - IConfigurationElement[] elements = extensions[i].getConfigurationElements(); - for (int j = 0; j < elements.length; j++) { - if ("excludeSources".equalsIgnoreCase(elements[j].getName())) { //$NON-NLS-1$ - String id = elements[j].getAttribute("id"); //$NON-NLS-1$ - if (id != null) - ignoredViews.add(id); - } - } - } - } - } - return ignoredViews; - } - - private boolean isViewIgnored(String partID) { - return getIgnoredViews().contains(partID); - } - @Override protected boolean isImportant(IWorkbenchPart part) { - String partID = part.getSite().getId(); - //System.out.println("isImportant(" + partID + ")"); - return !isWorkbenchSelectionPinned() && !isPropertyView(part) && !isViewIgnored(partID); + //System.out.println("isImportant(" + part.getSite().getId() + ")"); + return !isWorkbenchSelectionPinned() && !isPropertyView(part); } /** @@ -417,39 +371,10 @@ public class PropertyPageView extends PageBookView implements ISelectionListener // super.partHidden(part); } - ISelectionListener immediateSelectionListener = new ISelectionListener() { - - private Throttler throttler = new Throttler(SWTThread.getThreadAccess(PlatformUI.getWorkbench().getDisplay()), 500, 3); - - @Override - public void selectionChanged(final IWorkbenchPart part, final ISelection selection) { - - // Do not process selections from self - if(PropertyPageView.this == part) return; - - throttler.schedule(new Runnable() { - - @Override - public void run() { - PropertyPageView.this.doSelectionChanged(part, selection); - } - - }); - - } - }; - public ISelection getLastSelection() { return lastSelection; } - /* - * (non-Javadoc) - * - * @see org.eclipse.ui.ISelectionListener#selectionChanged(org.eclipse.ui.IWorkbenchPart, - * org.eclipse.jface.viewers.ISelection) - */ - @Override public void selectionChanged(IWorkbenchPart part, ISelection sel) { doSelectionChanged(part, sel); } @@ -461,7 +386,6 @@ public class PropertyPageView extends PageBookView implements ISelectionListener * false otherwise */ boolean doSelectionChanged(IWorkbenchPart part, ISelection sel) { - // we ignore our own selection or null selection if (isPropertyView(part) || sel == null) { return false; @@ -499,8 +423,8 @@ public class PropertyPageView extends PageBookView implements ISelectionListener lastPart.addPropertyListener(partPropertyListener); } - updatePartName(ppage, sel); if (!sameSelection) { + updatePartName(ppage, sel); ppage.selectionChanged(part, sel); return true; } @@ -516,14 +440,11 @@ public class PropertyPageView extends PageBookView implements ISelectionListener // This check is not safe - there might be a burst of changes incoming //if (getPartName().equals(parameter)) return; //System.out.println("partNameUpdateCallback : " + parameter); - SWTUtils.asyncExec(getPageBook(), new Runnable() { - @Override - public void run() { - if (!getPageBook().isDisposed()) { - if (getPartName().equals(parameter)) return; - //System.out.println("doSetParameterName : " + parameter); - doSetPartName(parameter); - } + SWTUtils.asyncExec(getPageBook(), () -> { + if (!getPageBook().isDisposed()) { + if (getPartName().equals(parameter)) return; + //System.out.println("doSetParameterName : " + parameter); + doSetPartName(parameter); } }); }; diff --git a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerImpl.java b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerImpl.java index df736c1e6..0168ad864 100644 --- a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerImpl.java +++ b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerImpl.java @@ -50,6 +50,7 @@ import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.jface.resource.JFaceResources; import org.eclipse.jface.resource.LocalResourceManager; import org.eclipse.jface.resource.ResourceManager; +import org.eclipse.jface.util.OpenStrategy; import org.eclipse.jface.viewers.IPostSelectionProvider; import org.eclipse.jface.viewers.ISelection; import org.eclipse.jface.viewers.ISelectionChangedListener; @@ -68,7 +69,6 @@ import org.eclipse.swt.events.KeyEvent; import org.eclipse.swt.events.KeyListener; import org.eclipse.swt.events.MouseEvent; import org.eclipse.swt.events.MouseListener; -import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.events.SelectionListener; import org.eclipse.swt.graphics.Color; import org.eclipse.swt.graphics.Font; @@ -289,24 +289,6 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph public static final int DEFAULT_MAX_CHILDREN = 1000; - private static final long POST_SELECTION_DELAY = 300; - - /** - * The time in milliseconds that must elapse between consecutive - * {@link Tree} {@link SelectionListener#widgetSelected(SelectionEvent)} - * invocations in order for this class to construct a new selection. - * - *

- * This is done because selection construction can be very expensive as the - * selected set grows larger when the user is pressing shift+arrow keys. - * GraphExplorerImpl will naturally listen to all changes in the tree - * selection, but as an optimization will not construct new - * StructuredSelection instances for every selection change event. A new - * selection will be constructed and set only if the selection hasn't - * changed for the amount of milliseconds specified by this constant. - */ - private static final long SELECTION_CHANGE_QUIET_TIME = 150; - private final IThreadWorkQueue thread; /** @@ -328,11 +310,11 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph Tree tree; @SuppressWarnings({ "rawtypes" }) - final HashMap, NodeQueryProcessor> processors = new HashMap, NodeQueryProcessor>(); + final HashMap, NodeQueryProcessor> processors = new HashMap<>(); @SuppressWarnings({ "rawtypes" }) - final HashMap primitiveProcessors = new HashMap(); + final HashMap primitiveProcessors = new HashMap<>(); @SuppressWarnings({ "rawtypes" }) - final HashMap dataSources = new HashMap(); + final HashMap dataSources = new HashMap<>(); class GraphExplorerContext extends AbstractDisposable implements IGraphExplorerContext { // This is for query debugging only. @@ -351,7 +333,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * null there's nothing scheduled yet in which case * scheduling can commence. Otherwise the update should be skipped. */ - AtomicReference currentQueryUpdater = new AtomicReference(); + AtomicReference currentQueryUpdater = new AtomicReference<>(); /** * Keeps track of nodes that have already been auto-expanded. After @@ -359,7 +341,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * expanded state after that. This makes it possible for the user to * close auto-expanded nodes. */ - Map autoExpanded = new WeakHashMap(); + Map autoExpanded = new WeakHashMap<>(); @Override @@ -482,7 +464,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph GraphExplorerContext explorerContext = new GraphExplorerContext(); - HashSet pendingItems = new HashSet(); + HashSet pendingItems = new HashSet<>(); boolean updating = false; boolean pendingRoot = false; @@ -503,14 +485,14 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * Access this map only in the SWT thread to keep it thread-safe. *

*/ - BijectionMap contextToItem = new BijectionMap(); + BijectionMap contextToItem = new BijectionMap<>(); /** * Columns of the UI viewer. Use {@link #setColumns(Column[])} to * initialize. */ Column[] columns = new Column[0]; - Map columnKeyToIndex = new HashMap(); + Map columnKeyToIndex = new HashMap<>(); boolean refreshingColumnSizes = false; boolean columnsAreVisible = true; @@ -537,12 +519,12 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph /** Set to true when the Tree widget is disposed. */ private boolean disposed = false; - private final CopyOnWriteArrayList focusListeners = new CopyOnWriteArrayList(); - private final CopyOnWriteArrayList mouseListeners = new CopyOnWriteArrayList(); - private final CopyOnWriteArrayList keyListeners = new CopyOnWriteArrayList(); + private final CopyOnWriteArrayList focusListeners = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList mouseListeners = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList keyListeners = new CopyOnWriteArrayList<>(); /** Selection provider */ - private GraphExplorerPostSelectionProvider postSelectionProvider = new GraphExplorerPostSelectionProvider(this); + private GraphExplorerPostSelectionProvider postSelectionProvider = new GraphExplorerPostSelectionProvider(this); protected BasePostSelectionProvider selectionProvider = new BasePostSelectionProvider(); protected SelectionDataResolver selectionDataResolver; protected SelectionFilter selectionFilter; @@ -570,12 +552,12 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * item was a part of the current selection in which case the selection must * be updated. */ - private final Map selectedItems = new HashMap(); + private final Map selectedItems = new HashMap<>(); /** * TODO: specify what this is for */ - private final Set selectionRefreshContexts = new HashSet(); + private final Set selectionRefreshContexts = new HashSet<>(); /** * If this field is non-null, it means that if {@link #setData(Event)} @@ -649,7 +631,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * * @see #setPendingImages(IProgressMonitor) */ - Map imageTasks = new THashMap(); + Map imageTasks = new THashMap<>(); /** * A state flag indicating whether the vertical scroll bar was visible for @@ -713,7 +695,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph updateCounter = 0; uiUpdateScheduler.schedule(this, 50, TimeUnit.MILLISECONDS); } else { - tree.getDisplay().asyncExec(new UpdateRunner(GraphExplorerImpl.this, GraphExplorerImpl.this.explorerContext)); + tree.getDisplay().asyncExec(new UpdateRunner(GraphExplorerImpl.this)); } } @@ -788,7 +770,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph * modified. These are used internally to prevent duplicate edits from being * initiated which should always be a sensible thing to do. */ - private Set currentlyModifiedNodes = new THashSet(); + private Set currentlyModifiedNodes = new THashSet<>(); private final TreeEditor editor; private Color invalidModificationColor = null; @@ -1381,17 +1363,14 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph // Keep track of the previous single selection to help // decide whether to start editing a tree node on mouse // downs or not. - tree.addListener(SWT.Selection, new Listener() { - @Override - public void handleEvent(Event event) { - TreeItem[] selection = tree.getSelection(); - if (selection.length == 1) { - //for (TreeItem item : selection) - // System.out.println("selection: " + item); - previousSingleSelection = selection[0]; - } else { - previousSingleSelection = null; - } + tree.addListener(SWT.Selection, event -> { + TreeItem[] selection = tree.getSelection(); + if (selection.length == 1) { + //for (TreeItem item : selection) + // System.out.println("selection: " + item); + previousSingleSelection = selection[0]; + } else { + previousSingleSelection = null; } }); @@ -1493,39 +1472,33 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph }; tree.addListener(SWT.MouseDown, mouseEditListener); tree.addListener(SWT.DragDetect, mouseEditListener); - tree.addListener(SWT.DragDetect, new Listener() { - @Override - public void handleEvent(Event event) { - Point test = new Point(event.x, event.y); - TreeItem item = tree.getItem(test); - if(item != null) { - for(int i=0;i { + Point test = new Point(event.x, event.y); + TreeItem item = tree.getItem(test); + if(item != null) { + for(int i=0;i { + Point test = new Point(event.x, event.y); + TreeItem item = tree.getItem(test); + if(item != null) { + for(int i=0;i { + //System.out.println("OPENSTRATEGY: post selection changed: " + e); + resetSelection(); + selectionProvider.firePostSelection(selectionProvider.getSelection()); + })); // This listener takes care of updating the set of currently selected // TreeItem instances. This set is needed because we need to know in @@ -1611,108 +1578,14 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph }); } - /** - * Mod count for delaying post selection changed events. - */ - int postSelectionModCount = 0; - - /** - * Last tree selection modification time for implementing a quiet - * time for selection changes. - */ - long lastSelectionModTime = System.currentTimeMillis() - 10000; - - /** - * Current target time for the selection to be set. Calculated - * according to the set quiet time and last selection modification - * time. - */ - long selectionSetTargetTime = 0; - - /** - * true if delayed selection runnable is current scheduled or - * running. - */ - boolean delayedSelectionScheduled = false; - - Runnable SELECTION_DELAY = new Runnable() { - @Override - public void run() { - if (tree.isDisposed()) - return; - long now = System.currentTimeMillis(); - long waitTimeLeft = selectionSetTargetTime - now; - if (waitTimeLeft > 0) { - // Not enough quiet time, reschedule. - delayedSelectionScheduled = true; - tree.getDisplay().timerExec((int) waitTimeLeft, this); - } else { - // Time to perform selection, stop rescheduling. - delayedSelectionScheduled = false; - resetSelection(); - } - } - }; - - private void widgetSelectionChanged(boolean forceSelectionChange) { - long modTime = System.currentTimeMillis(); - long delta = modTime - lastSelectionModTime; - lastSelectionModTime = modTime; - if (!forceSelectionChange && delta < SELECTION_CHANGE_QUIET_TIME) { - long msToWait = SELECTION_CHANGE_QUIET_TIME - delta; - selectionSetTargetTime = modTime + msToWait; - if (!delayedSelectionScheduled) { - delayedSelectionScheduled = true; - tree.getDisplay().timerExec((int) msToWait, SELECTION_DELAY); - } - // Make sure that post selection change events do not fire. - ++postSelectionModCount; - return; - } - - // Immediate selection reconstruction. - resetSelection(); - } - private void resetSelection() { final ISelection selection = getWidgetSelection(); - - //System.out.println("resetSelection(" + postSelectionModCount + ")"); - //System.out.println(" provider selection: " + selectionProvider.getSelection()); - //System.out.println(" widget selection: " + selection); - +// System.out.println("resetSelection()"); +// System.out.println(" provider selection: " + selectionProvider.getSelection()); +// System.out.println(" widget selection: " + selection); selectionProvider.setAndFireNonEqualSelection(selection); - - // Implement deferred firing of post selection events - final int count = ++postSelectionModCount; - //System.out.println("[" + System.currentTimeMillis() + "] scheduling postSelectionChanged " + count + ": " + selection); - ThreadUtils.getNonBlockingWorkExecutor().schedule(new Runnable() { - @Override - public void run() { - int newCount = postSelectionModCount; - // Don't publish selection yet, there's another change incoming. - //System.out.println("[" + System.currentTimeMillis() + "] checking post selection publish: " + count + " vs. " + newCount + ": " + selection); - if (newCount != count) - return; - //System.out.println("[" + System.currentTimeMillis() + "] " + count + " count equals, firing post selection listeners: " + selection); - - if (tree.isDisposed()) - return; - - //System.out.println("scheduling fire post selection changed: " + selection); - tree.getDisplay().asyncExec(new Runnable() { - @Override - public void run() { - if (tree.isDisposed() || selectionProvider == null) - return; - //System.out.println("firing post selection changed: " + selection); - selectionProvider.firePostSelection(selection); - } - }); - } - }, POST_SELECTION_DELAY, TimeUnit.MILLISECONDS); } - + protected void setDefaultProcessors() { // Add a simple IMAGER query processor that always returns null. // With this processor no images will ever be shown. @@ -2427,7 +2300,7 @@ class GraphExplorerImpl extends GraphExplorerImplBase implements Listener, Graph // System.out.println("MODCOUNT: " + modCount + " vs. " + count); if (modCount != count) return; - widgetSelectionChanged(true); + resetSelection(); } }); } diff --git a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/UpdateRunner.java b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/UpdateRunner.java index 72c17ed74..80ef2bf39 100644 --- a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/UpdateRunner.java +++ b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/UpdateRunner.java @@ -21,8 +21,9 @@ import org.simantics.browsing.ui.CheckedState; import org.simantics.browsing.ui.NodeContext; import org.simantics.browsing.ui.NodeQueryManager; import org.simantics.browsing.ui.common.internal.GENodeQueryManager; -import org.simantics.browsing.ui.common.internal.IGraphExplorerContext; import org.simantics.browsing.ui.content.PrunedChildrenResult; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class is responsible for updating the {@link TreeItem}s contained by the @@ -36,21 +37,20 @@ import org.simantics.browsing.ui.content.PrunedChildrenResult; */ class UpdateRunner implements Runnable { + private static final Logger LOGGER = LoggerFactory.getLogger(UpdateRunner.class); + final GraphExplorerImpl ge; - //final IGraphExplorerContext geContext; - UpdateRunner(GraphExplorerImpl ge, IGraphExplorerContext geContext) { + UpdateRunner(GraphExplorerImpl ge) { this.ge = ge; - // this.geContext = geContext; } public void run() { try { doRun(); } catch (Throwable t) { - t.printStackTrace(); + LOGGER.error("", t); } - } public void doRun() { diff --git a/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BasePostSelectionProvider.java b/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BasePostSelectionProvider.java index c5ca0e754..b06143bf6 100644 --- a/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BasePostSelectionProvider.java +++ b/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BasePostSelectionProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2010 Association for Decentralized Information Management + * Copyright (c) 2007, 2018 Association for Decentralized Information Management * in Industry THTH ry. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -21,8 +21,6 @@ import org.eclipse.jface.viewers.IPostSelectionProvider; import org.eclipse.jface.viewers.ISelection; import org.eclipse.jface.viewers.ISelectionChangedListener; import org.eclipse.jface.viewers.SelectionChangedEvent; -import org.eclipse.jface.viewers.StructuredSelection; -import org.simantics.utils.ObjectUtils; /** * BaseSelectionProvider is a base implementation ISelectionProvider -interface. @@ -36,40 +34,21 @@ import org.simantics.utils.ObjectUtils; *

* * @author Toni Kalajainen + * @author Tuukka Lehtonen */ -public class BasePostSelectionProvider implements IPostSelectionProvider { +public class BasePostSelectionProvider extends BaseSelectionProvider implements IPostSelectionProvider { - protected ListenerList selectionListeners = new ListenerList(); - - protected ListenerList postSelectionListeners = new ListenerList(); - - protected ISelection selection = StructuredSelection.EMPTY; - - public void addSelectionChangedListener(ISelectionChangedListener listener) { - selectionListeners.add(listener); - } - - public ISelection getSelection() { - return selection; - } + protected ListenerList postSelectionListeners = new ListenerList<>(); public void clearListeners() { - selectionListeners.clear(); - postSelectionListeners.clear(); - } - - public void clearSelectionChangedListeners() { - postSelectionListeners.clear(); + super.clearListeners(); + clearPostSelectionChangedListeners(); } public void clearPostSelectionChangedListeners() { postSelectionListeners.clear(); } - public void removeSelectionChangedListener(ISelectionChangedListener listener) { - selectionListeners.remove(listener); - } - @Override public void addPostSelectionChangedListener(ISelectionChangedListener listener) { postSelectionListeners.add(listener); @@ -80,30 +59,10 @@ public class BasePostSelectionProvider implements IPostSelectionProvider { postSelectionListeners.remove(listener); } - public void setSelection(ISelection selection) { - setSelectionWithoutFiring(selection); - } - - protected Object[] getListeners() { - return selectionListeners.getListeners(); - } - protected Object[] getPostListeners() { return postSelectionListeners.getListeners(); } - /** - * Notify other UIs that selection has changed - * @param selection new selection - */ - public void fireSelection(ISelection selection) { - if (selection == null) - return; - SelectionChangedEvent e = new SelectionChangedEvent(this, selection); - for (Object l : getListeners()) - ((ISelectionChangedListener) l).selectionChanged(e); - } - /** * Notify other UIs that selection has changed * @param selection new selection @@ -123,46 +82,4 @@ public class BasePostSelectionProvider implements IPostSelectionProvider { } } - public void setSelectionWithoutFiring(ISelection selection) { - this.selection = selection; - } - - - /** - * Sets a new selection and always fires a SelectionChangedEvent about it. - * - * @param selection the new selection - */ - public void setAndFireSelection(ISelection selection) { - setSelection(selection); - fireSelection(selection); - } - - /** - * Sets the new selection for this provider and fires all selection change - * listeners if the specified selection differs from the current selection. - * If the selection is either the same object or considered equal to the - * current selection, the listeners are not fired. - * - * @param selection the new selection - */ - public void setAndFireNonEqualSelection(ISelection selection) { - ISelection old = getSelection(); - if (ObjectUtils.objectEquals(old, selection)) - return; - - this.selection = selection; - if (selection != null && !selection.equals(old)) - fireSelection(selection); - } - - public boolean selectionEquals(ISelection s) { - if (s == selection) - return true; - if (s == null) - // Old selection had to be non-null - return true; - return s.equals(selection); - } - } diff --git a/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BaseSelectionProvider.java b/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BaseSelectionProvider.java index b2e6d9215..55ec0241b 100644 --- a/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BaseSelectionProvider.java +++ b/bundles/org.simantics.utils.ui/src/org/simantics/utils/ui/jface/BaseSelectionProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2010 Association for Decentralized Information Management + * Copyright (c) 2007, 2018 Association for Decentralized Information Management * in Industry THTH ry. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -20,6 +20,7 @@ import org.eclipse.jface.viewers.ISelectionChangedListener; import org.eclipse.jface.viewers.ISelectionProvider; import org.eclipse.jface.viewers.SelectionChangedEvent; import org.eclipse.jface.viewers.StructuredSelection; +import org.simantics.utils.ObjectUtils; /** * BaseSelectionProvider is a base implementation ISelectionProvider -interface. @@ -39,7 +40,7 @@ import org.eclipse.jface.viewers.StructuredSelection; */ public class BaseSelectionProvider implements ISelectionProvider { - protected ListenerList selectionListeners = new ListenerList(); + protected ListenerList selectionListeners = new ListenerList<>(); protected ISelection selection = StructuredSelection.EMPTY; @@ -59,6 +60,14 @@ public class BaseSelectionProvider implements ISelectionProvider { return selection; } + public void clearListeners() { + clearSelectionChangedListeners(); + } + + public void clearSelectionChangedListeners() { + selectionListeners.clear(); + } + public void removeSelectionChangedListener(ISelectionChangedListener listener) { selectionListeners.remove(listener); } @@ -106,14 +115,15 @@ public class BaseSelectionProvider implements ISelectionProvider { * * @param selection the new selection */ - public void setAndFireNonEqualSelection(ISelection selection) { + public boolean setAndFireNonEqualSelection(ISelection selection) { ISelection old = getSelection(); - if (selection == old) - return; + if (ObjectUtils.objectEquals(old, selection)) + return false; this.selection = selection; - if (selection != null && !selection.equals(old)) + if (selection != null) fireSelection(selection); + return true; } public boolean selectionEquals(ISelection s) { diff --git a/bundles/org.simantics.views.swt.client/src/org/simantics/views/swt/client/impl/SWTExplorer.java b/bundles/org.simantics.views.swt.client/src/org/simantics/views/swt/client/impl/SWTExplorer.java index 1666413e9..e196e6732 100644 --- a/bundles/org.simantics.views.swt.client/src/org/simantics/views/swt/client/impl/SWTExplorer.java +++ b/bundles/org.simantics.views.swt.client/src/org/simantics/views/swt/client/impl/SWTExplorer.java @@ -10,11 +10,8 @@ import org.eclipse.jface.viewers.IPostSelectionProvider; import org.eclipse.jface.viewers.ISelection; import org.eclipse.jface.viewers.ISelectionChangedListener; import org.eclipse.jface.viewers.ISelectionProvider; -import org.eclipse.jface.viewers.SelectionChangedEvent; import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Event; -import org.eclipse.swt.widgets.Listener; import org.eclipse.ui.IWorkbenchSite; import org.simantics.browsing.ui.Column; import org.simantics.browsing.ui.StatePersistor; @@ -45,9 +42,19 @@ public class SWTExplorer extends SingleSWTViewNode { public Boolean useNodeBrowseContexts; public Boolean useNodeActionContexts; public StatePersistor persistor; - + public ISelection lastSelection; - + + private ISelectionChangedListener internalToExternalSelectionPropagator = event -> { + IWorkbenchSite site = getSite(); + if (site != null) { + ISelectionProvider sp = site.getSelectionProvider(); + if (sp != null) { + sp.setSelection(event.getSelection()); + } + } + }; + @Override public void createControls(Composite parent) { @@ -70,80 +77,30 @@ public class SWTExplorer extends SingleSWTViewNode { if (uiContext != null) { control.setUiContexts(Collections.singleton(uiContext)); } - + control.setStatePersistor(persistor); - + control.finish(); setProperties(); - + control.setInputSource(PassThruInputSource.INSTANCE); - ISelectionProvider sp = (ISelectionProvider)control.getExplorer().getAdapter(ISelectionProvider.class); - sp.addSelectionChangedListener(new ISelectionChangedListener() { - - @Override - public void selectionChanged(SelectionChangedEvent event) { - if (site != null) { - ISelectionProvider sp = site.getSelectionProvider(); - if (sp != null) { - sp.setSelection(event.getSelection()); - } - } - } - - }); - - IPostSelectionProvider psp = (IPostSelectionProvider)control.getExplorer().getAdapter(IPostSelectionProvider.class); - psp.addPostSelectionChangedListener(new ISelectionChangedListener() { - - @Override - public void selectionChanged(SelectionChangedEvent event) { - -// ISelection selection = (ISelection)control.getExplorer().getWidgetSelection(); - if (site != null) { - ISelectionProvider sp = site.getSelectionProvider(); - if (sp != null) { - sp.setSelection(event.getSelection()); - } - } - - } - - }); - - control.addListenerToControl(SWT.Selection, new Listener() { - - @Override - public void handleEvent(Event event) { - - if(selectionListener != null) - selectionListener.apply(event); - - ISelection selection = (ISelection)control.getExplorer().getWidgetSelection(); - // [Tuukka@2012-04-08] Disabled this because it was causing - // horrible selection feedback effects in the Model Browser - // view that is using it. It causes the browser to react to - // external selections which were initially published as its own - // with a delay. - //System.out.println("selection: " + selection); -// if(publishSelection != null && publishSelection) { -// if (site != null) { -// ISelectionProvider sp = site.getSelectionProvider(); -// if (sp != null) { -// sp.setSelection(selection); -// } -// } -// } - - propertyCallback.apply("selection", selection); - - lastSelection = selection; - - } - + ISelectionProvider isp = (ISelectionProvider)control.getExplorer().getAdapter(ISelectionProvider.class); + if (isp instanceof IPostSelectionProvider) { + ((IPostSelectionProvider) isp).addPostSelectionChangedListener(internalToExternalSelectionPropagator); + } else { + isp.addSelectionChangedListener(internalToExternalSelectionPropagator); + } + + control.addListenerToControl(SWT.Selection, event -> { + if(selectionListener != null) + selectionListener.apply(event); + ISelection selection = (ISelection)control.getExplorer().getWidgetSelection(); + propertyCallback.apply("selection", selection); + lastSelection = selection; }); - + } public void synchronizeColumnsVisible(Boolean columnsVisible) { @@ -191,5 +148,16 @@ public class SWTExplorer extends SingleSWTViewNode { final public void synchronizeLayout(LayoutBean layout) { } - + + @Override + public void dispose() { + ISelectionProvider isp = (ISelectionProvider)control.getExplorer().getAdapter(ISelectionProvider.class); + if (isp instanceof IPostSelectionProvider) { + ((IPostSelectionProvider) isp).removePostSelectionChangedListener(internalToExternalSelectionPropagator); + } else { + isp.removeSelectionChangedListener(internalToExternalSelectionPropagator); + } + super.dispose(); + } + } diff --git a/bundles/org.simantics.views.swt/META-INF/MANIFEST.MF b/bundles/org.simantics.views.swt/META-INF/MANIFEST.MF index 18048f38c..cd17a1872 100644 --- a/bundles/org.simantics.views.swt/META-INF/MANIFEST.MF +++ b/bundles/org.simantics.views.swt/META-INF/MANIFEST.MF @@ -16,7 +16,8 @@ Require-Bundle: org.simantics.browsing.ui.swt;bundle-version="1.1.0";visibility: org.simantics.project;bundle-version="1.0.1", org.simantics.scenegraph.loader;bundle-version="1.0.0";visibility:=reexport, org.simantics.scenegraph.ontology;bundle-version="1.0.0", - org.simantics.utils.thread.swt;bundle-version="1.1.0" + org.simantics.utils.thread.swt;bundle-version="1.1.0", + org.slf4j.api Export-Package: org.simantics.views.swt Bundle-Activator: org.simantics.views.swt.Activator Bundle-Vendor: Semantum Oy diff --git a/bundles/org.simantics.views.swt/src/org/simantics/views/swt/ModelledView.java b/bundles/org.simantics.views.swt/src/org/simantics/views/swt/ModelledView.java index f50954857..92396f421 100644 --- a/bundles/org.simantics.views.swt/src/org/simantics/views/swt/ModelledView.java +++ b/bundles/org.simantics.views.swt/src/org/simantics/views/swt/ModelledView.java @@ -27,7 +27,6 @@ import org.eclipse.ui.IWorkbenchPart; import org.eclipse.ui.IWorkbenchPartReference; import org.eclipse.ui.IWorkbenchSite; import org.simantics.Simantics; -import org.simantics.browsing.ui.common.ErrorLogger; import org.simantics.browsing.ui.swt.widgets.impl.WidgetSupport; import org.simantics.browsing.ui.swt.widgets.impl.WidgetSupportImpl; import org.simantics.db.ReadGraph; @@ -36,7 +35,6 @@ import org.simantics.db.VirtualGraph; import org.simantics.db.WriteGraph; import org.simantics.db.common.request.WriteRequest; import org.simantics.db.common.request.WriteResultRequest; -import org.simantics.db.common.utils.Logger; import org.simantics.db.exception.DatabaseException; import org.simantics.db.exception.ServiceNotFoundException; import org.simantics.db.layer0.util.RemoverUtil; @@ -47,8 +45,11 @@ import org.simantics.layer0.Layer0; import org.simantics.scenegraph.ontology.ScenegraphResources; import org.simantics.scl.runtime.function.Function1; import org.simantics.ui.workbench.IPropertyPage; -import org.simantics.utils.ui.jface.ActiveSelectionProvider; +import org.simantics.utils.ui.ErrorLogger; +import org.simantics.utils.ui.jface.BasePostSelectionProvider; import org.simantics.views.swt.client.base.SWTRoot; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * To use this class, first model your view contents in .pgraph files according @@ -67,6 +68,8 @@ import org.simantics.views.swt.client.base.SWTRoot; */ public class ModelledView extends SimanticsView implements IPartListener2 { + private static final Logger LOGGER = LoggerFactory.getLogger(ModelledView.class); + public static final int TIME_BEFORE_DISPOSE_WHEN_HIDDEN = 30000; // ms private static final boolean DEBUG = false; @@ -88,10 +91,11 @@ public class ModelledView extends SimanticsView implements IPartListener2 { protected ModelledSupport support; - ActiveSelectionProvider selectionProvider = new ActiveSelectionProvider() { + private BasePostSelectionProvider selectionProvider = new BasePostSelectionProvider() { @Override public void setSelection(ISelection selection) { - super.setSelection(selection); + super.setAndFireNonEqualSelection(selection); + super.firePostSelection(selection); } }; @@ -101,7 +105,6 @@ public class ModelledView extends SimanticsView implements IPartListener2 { @Override protected WidgetSupportImpl createSupport() { - try { runtime = Simantics.getSession().sync( new WriteResultRequest(Simantics.getSession().getService(VirtualGraph.class)) { @@ -114,16 +117,13 @@ public class ModelledView extends SimanticsView implements IPartListener2 { return runtime; } }); - } catch (ServiceNotFoundException e) { - Logger.defaultLogError(e); - } catch (DatabaseException e) { - Logger.defaultLogError(e); + } catch (ServiceNotFoundException | DatabaseException e) { + LOGGER.error("Failed to initialize modelled view database runtime support structures", e); } support = new ModelledSupport(this); return support; - } public void fireInput() { @@ -166,9 +166,7 @@ public class ModelledView extends SimanticsView implements IPartListener2 { } if (load) { - try { - loader = new SWTViewLoaderProcess(this, getSite(), getClass().getSimpleName()); viewVariable = loader.getVariable(Simantics.getSession(), configurationURI(), runtime); @@ -200,17 +198,10 @@ public class ModelledView extends SimanticsView implements IPartListener2 { body.layout(true); - getSite().setSelectionProvider(selectionProvider); - } catch (DatabaseException e) { - - e.printStackTrace(); - Logger.defaultLogError(e); - + LOGGER.error("Failed to create modelled SWT view controls", e); } - } - } @Override @@ -220,14 +211,12 @@ public class ModelledView extends SimanticsView implements IPartListener2 { // Only create controls if the part is TRULY visible. // Fast view parts seem to cause calls to createPartControl even // when the part is hidden in reality - boolean visible = site.getPage().isPartVisible(this); if (DEBUG) System.out.println(this + ": createControls( visible=" + site.getPage().isPartVisible(this) + " )"); doCreateControls(true); getSite().setSelectionProvider(selectionProvider); getSite().getPage().addPartListener(this); - } protected void inputChanged(IWorkbenchPart provider, Object input) { @@ -291,10 +280,8 @@ public class ModelledView extends SimanticsView implements IPartListener2 { RemoverUtil.remove(graph, rt); } }); - } catch (ServiceNotFoundException e) { - Logger.defaultLogError(e); - } catch (DatabaseException e) { - Logger.defaultLogError(e); + } catch (ServiceNotFoundException | DatabaseException e) { + LOGGER.error("Failed to dispose of modelled view database runtime support structures", e); } } @@ -394,10 +381,10 @@ public class ModelledView extends SimanticsView implements IPartListener2 { } } } - + // Can be used to cancel already scheduled dispose volatile boolean reallyClearExisting = false; - + Runnable clearExisting = new Runnable() { @Override @@ -445,5 +432,5 @@ public class ModelledView extends SimanticsView implements IPartListener2 { protected IPropertyPage getPropertyPage() { return null; } - + }