From f16950def6078e1408d1380aabb29185b0ac30bd Mon Sep 17 00:00:00 2001 From: Patrick Tasse Date: Mon, 20 Mar 2017 17:37:28 -0400 Subject: [PATCH] tmf: Fix Marker Set action handling When changing the active marker set, it didn't update the marker sources when the opened trace is an experiment. The action only updated the marker sources from the opened traces, but the marker sets were created on each trace in the experiment's trace set, for the default implementation of AbstractTimeGraphView.getTracesToBuild(). It wouldn't be good enough to update the traces using TmfTraceManager.getTraceSet(), since we don't know the implementation of getTracesToBuild() for each view. Also, calling TmfTraceAdapterManager.getAdapters() on each opened trace could actually create new adapter instances that weren't yet needed. Instead the ConfigurableMarkerEventSource instances should handle the TmfMarkerEventSourceUpdatedSignal, and reconfigure themselves. Also, since the Marker Set menu items are radio, selecting a new marker set first executes the action on the menu item that is being unchecked. The action now only executes for the menu item that is being checked. Change-Id: Ic8fd21b930ec29e8a619cbcedea712d4b7ee4d86 Signed-off-by: Patrick Tasse Reviewed-on: https://git.eclipse.org/r/93463 Reviewed-by: Hudson CI Reviewed-by: Bernd Hufmann Tested-by: Bernd Hufmann --- .../ConfigurableMarkerEventSource.java | 23 +++++++++++- .../ConfigurableMarkerEventSourceFactory.java | 13 +------ .../internal/tmf/ui/markers/MarkerUtils.java | 35 ++++++++++++++----- .../timegraph/AbstractTimeGraphView.java | 23 +++++------- 4 files changed, 59 insertions(+), 35 deletions(-) diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java index 8da331cd37..7497a53377 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java @@ -37,6 +37,10 @@ import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet; import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker; import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker.SplitMarker; import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker.WeightedMarker; +import org.eclipse.tracecompass.tmf.core.signal.TmfMarkerEventSourceUpdatedSignal; +import org.eclipse.tracecompass.tmf.core.signal.TmfSignalHandler; +import org.eclipse.tracecompass.tmf.core.signal.TmfSignalManager; +import org.eclipse.tracecompass.tmf.core.trace.AbstractTmfTraceAdapterFactory.IDisposableAdapter; import org.eclipse.tracecompass.tmf.core.trace.ICyclesConverter; import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace; import org.eclipse.tracecompass.tmf.ui.colors.X11Color; @@ -53,7 +57,7 @@ import com.google.common.collect.RangeSet; /** * Configurable marker event source. */ -public class ConfigurableMarkerEventSource implements IMarkerEventSource { +public class ConfigurableMarkerEventSource implements IMarkerEventSource, IDisposableAdapter { private static final long NANO_PER_MILLI = 1000000L; private static final long NANO_PER_MICRO = 1000L; @@ -74,6 +78,12 @@ public class ConfigurableMarkerEventSource implements IMarkerEventSource { public ConfigurableMarkerEventSource(ITmfTrace trace) { fMarkerEventSources = new ArrayList<>(); fTrace = trace; + TmfSignalManager.register(this); + } + + @Override + public void dispose() { + TmfSignalManager.deregister(this); } /** @@ -318,4 +328,15 @@ public class ConfigurableMarkerEventSource implements IMarkerEventSource { return fMarker.getSubMarkers(); } } + + /** + * A marker event source has been updated + * + * @param signal + * the signal + */ + @TmfSignalHandler + public void markerEventSourceUpdated(final TmfMarkerEventSourceUpdatedSignal signal) { + configure(MarkerUtils.getDefaultMarkerSet()); + } } diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java index 9484a764f9..179fbd414e 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java @@ -12,8 +12,6 @@ package org.eclipse.tracecompass.internal.tmf.ui.markers; -import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser; -import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet; import org.eclipse.tracecompass.tmf.core.trace.AbstractTmfTraceAdapterFactory; import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace; import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.model.IMarkerEventSource; @@ -27,7 +25,7 @@ public class ConfigurableMarkerEventSourceFactory extends AbstractTmfTraceAdapte protected T getTraceAdapter(ITmfTrace trace, Class adapterType) { if (IMarkerEventSource.class.equals(adapterType)) { ConfigurableMarkerEventSource adapter = new ConfigurableMarkerEventSource(trace); - configure(adapter); + adapter.configure(MarkerUtils.getDefaultMarkerSet()); return adapterType.cast(adapter); } return null; @@ -39,13 +37,4 @@ public class ConfigurableMarkerEventSourceFactory extends AbstractTmfTraceAdapte IMarkerEventSource.class }; } - - private static void configure(ConfigurableMarkerEventSource source) { - String defaultMarkerSetId = MarkerUtils.getDefaultMarkerSetId(); - for (MarkerSet markerSet : MarkerConfigXmlParser.getMarkerSets()) { - if (markerSet.getId().equals(defaultMarkerSetId)) { - source.configure(markerSet); - } - } - } } diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java index e6873e5f2a..616b95eb27 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java @@ -12,7 +12,10 @@ package org.eclipse.tracecompass.internal.tmf.ui.markers; +import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jface.dialogs.IDialogSettings; +import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser; +import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet; import org.eclipse.tracecompass.internal.tmf.ui.Activator; /** @@ -22,22 +25,38 @@ public class MarkerUtils { private static final String MARKER_SET_KEY = "marker.set"; //$NON-NLS-1$ + private static MarkerSet fDefaultMarkerSet = null; + /** - * Get the default marker set id + * Get the default marker set * - * @return the default marker set id, or null if none is set + * @return the default marker set, or null if none is set */ - public static String getDefaultMarkerSetId() { - return getDialogSettings().get(MARKER_SET_KEY); + public static synchronized @Nullable MarkerSet getDefaultMarkerSet() { + String id = getDialogSettings().get(MARKER_SET_KEY); + if (id == null) { + fDefaultMarkerSet = null; + } else { + if (fDefaultMarkerSet == null || !fDefaultMarkerSet.getId().equals(id)) { + for (MarkerSet markerSet : MarkerConfigXmlParser.getMarkerSets()) { + if (markerSet.getId().equals(id)) { + fDefaultMarkerSet = markerSet; + } + } + } + } + return fDefaultMarkerSet; } /** - * Set the default marker set id + * Set the default marker set * - * @param id - * the default marker set id, or null to set none + * @param markerSet + * the default marker set, or null to set none */ - public static void setDefaultMarkerSetId(String id) { + public static synchronized void setDefaultMarkerSet(@Nullable MarkerSet markerSet) { + fDefaultMarkerSet = markerSet; + String id = (markerSet == null) ? null : markerSet.getId(); getDialogSettings().put(MARKER_SET_KEY, id); } diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java index fcf7ef4064..50f40a5cfe 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java @@ -87,7 +87,6 @@ import org.eclipse.tracecompass.common.core.log.TraceCompassLog; import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser; import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet; import org.eclipse.tracecompass.internal.tmf.ui.Activator; -import org.eclipse.tracecompass.internal.tmf.ui.markers.ConfigurableMarkerEventSource; import org.eclipse.tracecompass.internal.tmf.ui.markers.MarkerUtils; import org.eclipse.tracecompass.tmf.core.resources.ITmfMarker; import org.eclipse.tracecompass.tmf.core.signal.TmfMarkerEventSourceUpdatedSignal; @@ -1707,15 +1706,10 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA @Override public void runWithEvent(Event event) { - MarkerUtils.setDefaultMarkerSetId(fMarkerSet == null ? null : fMarkerSet.getId()); - for (ITmfTrace trace : TmfTraceManager.getInstance().getOpenedTraces()) { - for (IMarkerEventSource source : TmfTraceAdapterManager.getAdapters(trace, IMarkerEventSource.class)) { - if (source instanceof ConfigurableMarkerEventSource) { - ((ConfigurableMarkerEventSource) source).configure(fMarkerSet); - } - } + if (isChecked()) { + MarkerUtils.setDefaultMarkerSet(fMarkerSet); + broadcast(new TmfMarkerEventSourceUpdatedSignal(AbstractTimeGraphView.this)); } - broadcast(new TmfMarkerEventSourceUpdatedSignal(AbstractTimeGraphView.this)); } } @@ -1734,13 +1728,14 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA fMarkerSetMenu.addMenuListener(new IMenuListener() { @Override public void menuAboutToShow(IMenuManager mgr) { - Action action = new MarkerSetAction(null); - String defaultMarkerSetId = MarkerUtils.getDefaultMarkerSetId(); - action.setChecked(defaultMarkerSetId == null || defaultMarkerSetId.isEmpty()); - mgr.add(action); + Action noneAction = new MarkerSetAction(null); + MarkerSet defaultMarkerSet = MarkerUtils.getDefaultMarkerSet(); + String defaultMarkerSetId = (defaultMarkerSet == null) ? null : defaultMarkerSet.getId(); + noneAction.setChecked(defaultMarkerSetId == null); + mgr.add(noneAction); List markerSets = MarkerConfigXmlParser.getMarkerSets(); for (MarkerSet markerSet : markerSets) { - action = new MarkerSetAction(markerSet); + Action action = new MarkerSetAction(markerSet); action.setChecked(markerSet.getId().equals(defaultMarkerSetId)); mgr.add(action); } -- 2.34.1