Merge remote-tracking branch 'origin/GP-672-dragonmacher-pulldown-action-checkboxes-2'

Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java

@@ -379,7 +379,7 @@ public class DefaultGraphDisplay implements GraphDisplay {
 		new MultiStateActionBuilder<String>("Arrangement", ACTION_OWNER)
 				.description("Arrangement: " + layoutActionStates.get(0).getName())
 				.toolBarIcon(DefaultDisplayGraphIcons.LAYOUT_ALGORITHM_ICON)
-				.fireFirstAction(false)
+				.useCheckboxForIcons(true)
 				.onActionStateChanged((s, t) -> layoutChanged(s.getName()))
 				.addStates(layoutActionStates)
 				.buildAndInstallLocal(componentProvider);
@@ -701,11 +701,6 @@ public class DefaultGraphDisplay implements GraphDisplay {
 		}
 	}
 
-	/**
-	 * get a {@code List} of {@code ActionState} buttons for the
-	 * configured layout algorithms
-	 * @return a {@code List} of {@code ActionState} buttons
-	 */
 	private List<ActionState<String>> getLayoutActionStates() {
 		String[] names = layoutTransitionManager.getLayoutNames();
 		List<ActionState<String>> actionStates = new ArrayList<>();
@@ -718,20 +713,10 @@ public class DefaultGraphDisplay implements GraphDisplay {
 		return actionStates;
 	}
 
-	/**
-	 * respond to a change in the layout name
-	 * @param layoutName the name of the layout algorithm to apply
-	 */
 	private void layoutChanged(String layoutName) {
-		if (layoutTransitionManager != null) {
-			new TaskLauncher(new SetLayoutTask(viewer, layoutTransitionManager, layoutName), null,
-				1000);
-		}
+		TaskLauncher.launch(new SetLayoutTask(viewer, layoutTransitionManager, layoutName));
 	}
 
-	/**
-	 * show the dialog with generated filters
-	 */
 	private void showFilterDialog() {
 		if (filterDialog == null) {
 			if (vertexFilters == null) {
@@ -743,10 +728,6 @@ public class DefaultGraphDisplay implements GraphDisplay {
 		componentProvider.getTool().showDialog(filterDialog);
 	}
 
-	/**
-	 * add or remove the satellite viewer
-	 * @param context information about the event
-	 */
 	private void toggleSatellite(ActionContext context) {
 		boolean selected = ((AbstractButton) context.getSourceObject()).isSelected();
 		graphDisplayProvider.setDefaultSatelliteState(selected);

Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/SetLayoutTask.java

@@ -17,6 +17,7 @@ package ghidra.graph.visualization;
 
 import java.util.concurrent.CountDownLatch;
 
+import org.jgrapht.Graph;
 import org.jungrapht.visualization.VisualizationModel;
 import org.jungrapht.visualization.VisualizationViewer;
 import org.jungrapht.visualization.layout.event.LayoutStateChange.*;
@@ -24,6 +25,7 @@ import org.jungrapht.visualization.layout.model.LayoutModel;
 
 import ghidra.service.graph.AttributedEdge;
 import ghidra.service.graph.AttributedVertex;
+import ghidra.util.Msg;
 import ghidra.util.Swing;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.*;
@@ -48,8 +50,8 @@ public class SetLayoutTask extends Task {
 
 	@Override
 	public void run(TaskMonitor monitor) throws CancelledException {
-		// add a callback for when/if the user cancels the layout, use a variable cause
-		// monitor uses a weak listener list and it would othewise get garbage collected.
+		// add a callback for when/if the user cancels the layout, use a variable because
+		// monitor uses a weak listener list and it would otherwise get garbage collected.
 		CancelledListener cancelListener = this::taskCancelled;
 		monitor.addCancelledListener(cancelListener);
 
@@ -64,12 +66,28 @@ public class SetLayoutTask extends Task {
 		// paints - should be changed in the future to not require it to be on the swing thread.
 		Swing.runNow(() -> layoutTransitionManager.setLayout(layoutName));
 
+		waitForLayoutTransition(model);
+
+		support.removeLayoutStateChangeListener(listener);
+		monitor.removeCancelledListener(cancelListener);
+	}
+
+	private void waitForLayoutTransition(
+			VisualizationModel<AttributedVertex, AttributedEdge> model) {
+
+		Graph<AttributedVertex, AttributedEdge> graph = model.getGraph();
+		if (graph.vertexSet().isEmpty()) {
+			// note: the underlying graph API will not notify us of the layout state change if the
+			//       graph is empty, so do not wait.
+			return;
+		}
+
 		// some of the layouts are done on the calling thread and some aren't. If they are on
 		// the calling thread, then by now, we already got the "done" callback and the "taskDone"
 		// countdown latch has been triggered and won't wait.  If, however, the layout has been
 		// diverted to another thread, we want to wait until the layout is completed
 		// There are two ways the latch will be triggered, the layout is completed or the user
-		// cancles the layout.
+		// cancels the layout.
 		try {
 			taskDone.await();
 		}
@@ -77,17 +95,17 @@ public class SetLayoutTask extends Task {
 			model.getLayoutAlgorithm().cancel();
 		}
 
-		// clean up the listeners
-		support.removeLayoutStateChangeListener(listener);
-		monitor.removeCancelledListener(cancelListener);
 	}
 
 	/**
-	 * Notfication when the layout algorithm starts and stops.
-	 * @param e the event. If the event.active is true, then the
-	 * algorithm is starting, if false, the algorithm is done.
+	 * Notification when the layout algorithm starts and stops
+	 * @param e the event. If the event.active is true, then the algorithm is starting, if false,
+	 *        the algorithm is done.
 	 */
 	private void layoutStateChanged(Event e) {
+
+		Msg.debug(this, "layoutStatechanged(): " + e);
+
 		if (!e.active) {
 			// algorithm is done, release the latch
 			taskDone.countDown();
@@ -98,6 +116,9 @@ public class SetLayoutTask extends Task {
 	 * Callback if the user cancels the layout
 	 */
 	private void taskCancelled() {
+
+		Msg.debug(this, "taskCancelled()");
+
 		// release the latch and tell the layout algorithm to cancel.
 		taskDone.countDown();
 		viewer.getVisualizationModel().getLayoutAlgorithm().cancel();

Ghidra/Framework/Docking/src/main/java/docking/action/ToggleDockingAction.java

@@ -19,7 +19,6 @@ import javax.swing.JButton;
 import javax.swing.JMenuItem;
 
 import docking.*;
-import docking.menu.DockingCheckboxMenuItemUI;
 
 public abstract class ToggleDockingAction extends DockingAction implements ToggleDockingActionIf {
 	private boolean isSelected;
@@ -59,9 +58,7 @@ public abstract class ToggleDockingAction extends DockingAction implements Toggl
 
 	@Override
 	protected JMenuItem doCreateMenuItem() {
-		DockingCheckBoxMenuItem menuItem = new DockingCheckBoxMenuItem(isSelected);
-		menuItem.setUI((DockingCheckboxMenuItemUI) DockingCheckboxMenuItemUI.createUI(menuItem));
-		return menuItem;
+		return new DockingCheckBoxMenuItem(isSelected);
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/action/builder/MultiStateActionBuilder.java

@@ -34,8 +34,8 @@ public class MultiStateActionBuilder<T> extends
 		AbstractActionBuilder<MultiStateDockingAction<T>, ActionContext, MultiStateActionBuilder<T>> {
 
 	private BiConsumer<ActionState<T>, EventTrigger> actionStateChangedCallback;
+	private boolean useCheckboxForIcons;
 	private boolean performActionOnButtonClick = false;
-	private boolean fireFirstAction = true;
 
 	private List<ActionState<T>> states = new ArrayList<>();
 
@@ -80,6 +80,20 @@ public class MultiStateActionBuilder<T> extends
 		return self();
 	}
 
+	/**
+	 * Overrides the default icons for actions shown in popup menu of the multi-state action.  By
+	 * default, the popup menu items will use the icons as provided by the {@link ActionState}.
+	 * By passing true to this method, icons will not be used in the popup menu.  Instead, a 
+	 * checkbox icon will be used to show the active action state.
+	 * 
+	 * @param b true to use a checkbox
+	 * @return this MultiActionDockingActionBuilder (for chaining)
+	 */
+	public MultiStateActionBuilder<T> useCheckboxForIcons(boolean b) {
+		this.useCheckboxForIcons = b;
+		return self();
+	}
+
 	/**
 	 * Add an action state 
 	 * 
@@ -115,17 +129,6 @@ public class MultiStateActionBuilder<T> extends
 		return self();
 	}
 
-	/**
-	 * controls whether the first action added will automatically fire an event or not
-	 *
-	 * @param fireFirstAction do fire an action on the first action. Defaults to {@code true}
-	 * @return this MultiActionDockingActionBuilder (for chaining)
-	 */
-	public MultiStateActionBuilder<T> fireFirstAction(boolean fireFirstAction) {
-		this.fireFirstAction = fireFirstAction;
-		return self();
-	}
-
 	@Override
 	public MultiStateDockingAction<T> build() {
 		validate();
@@ -145,7 +148,6 @@ public class MultiStateActionBuilder<T> extends
 					}
 				}
 			};
-		action.setFireFirstEvent(fireFirstAction);
 
 		for (ActionState<T> actionState : states) {
 			action.addActionState(actionState);
@@ -153,6 +155,7 @@ public class MultiStateActionBuilder<T> extends
 
 		decorateAction(action);
 		action.setPerformActionOnPrimaryButtonClick(performActionOnButtonClick);
+		action.setUseCheckboxForIcons(useCheckboxForIcons);
 		return action;
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/menu/MultiStateDockingAction.java

@@ -53,9 +53,9 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 	private int currentStateIndex = 0;
 	private MultiActionDockingActionIf multiActionGenerator;
 	private MultipleActionDockingToolbarButton multipleButton;
-	private boolean fireFirstEvent = true;
 
 	private boolean performActionOnPrimaryButtonClick = true;
+	private boolean useCheckboxForIcons;
 
 	// A listener that will get called when the button (not the popup) is clicked.  Toolbar
 	// actions do not use this listener. 
@@ -127,6 +127,18 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 		}
 	}
 
+	/**
+	 * Overrides the default icons for actions shown in popup menu of the multi-state action.  By
+	 * default, the popup menu items will use the icons as provided by the {@link ActionState}.
+	 * By passing true to this method, icons will not be used in the popup menu.  Instead, a 
+	 * checkbox icon will be used to show the active action state.
+	 * 
+	 * @param useCheckboxForIcons true to use a checkbox
+	 */
+	public void setUseCheckboxForIcons(boolean useCheckboxForIcons) {
+		this.useCheckboxForIcons = useCheckboxForIcons;
+	}
+
 	@Override
 	public final void actionPerformed(ActionContext context) {
 		if (!performActionOnPrimaryButtonClick) {
@@ -137,21 +149,6 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 		doActionPerformed(context);
 	}
 
-	/**
-	 * @return {@code true} if the first action automatically fire its event
-	 */
-	public boolean isFireFirstEvent() {
-		return fireFirstEvent;
-	}
-
-	/**
-	 * set the flag to fire an event on the first action
-	 * @param fireFirstEvent whether to fire the event
-	 */
-	public void setFireFirstEvent(boolean fireFirstEvent) {
-		this.fireFirstEvent = fireFirstEvent;
-	}
-
 	/**
 	 * This is the callback to be overridden when the child wishes to respond to user button
 	 * presses that are on the button and not the drop-down.  This will only be called if
@@ -175,9 +172,17 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 	}
 
 	protected List<DockingActionIf> getStateActions() {
+		ActionState<T> selectedState = actionStates.get(currentStateIndex);
 		List<DockingActionIf> actions = new ArrayList<>(actionStates.size());
 		for (ActionState<T> actionState : actionStates) {
-			actions.add(new ActionStateAction(actionState));
+
+			//@formatter:off
+			boolean isSelected = actionState == selectedState;
+			DockingActionIf a = useCheckboxForIcons ? 
+				new ActionStateToggleAction(actionState, isSelected) :
+			    new ActionStateAction(actionState, isSelected);
+			actions.add(a);
+			//@formatter:on
 		}
 		return actions;
 	}
@@ -199,7 +204,7 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 	 */
 	public void addActionState(ActionState<T> actionState) {
 		actionStates.add(actionState);
-		if (actionStates.size() == 1 && fireFirstEvent) {
+		if (actionStates.size() == 1) {
 			setCurrentActionState(actionState);
 		}
 	}
@@ -209,10 +214,8 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 			throw new IllegalArgumentException("You must provide at least one ActionState");
 		}
 		actionStates = new ArrayList<>(newStates);
-		if (fireFirstEvent) {
 		setCurrentActionState(actionStates.get(0));
 	}
-	}
 
 	public T getCurrentUserData() {
 		return actionStates.get(currentStateIndex).getUserData();
@@ -235,7 +238,7 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 		}
 
 		throw new AssertException(
-			"Attempted to set an action state by a user type not " + "contained herein: " + t);
+			"Attempted to set an action state by a user type not contained herein: " + t);
 	}
 
 	public void setCurrentActionState(ActionState<T> actionState) {
@@ -318,13 +321,47 @@ public abstract class MultiStateDockingAction<T> extends DockingAction {
 //==================================================================================================
 // Inner Classes
 //==================================================================================================
+
+	private class ActionStateToggleAction extends ToggleDockingAction {
+
+		private final ActionState<T> actionState;
+
+		private ActionStateToggleAction(ActionState<T> actionState, boolean isSelected) {
+			super(actionState.getName(), "multiStateAction");
+
+			this.actionState = actionState;
+
+			setSelected(isSelected);
+
+			setMenuBarData(
+				new MenuData(new String[] { actionState.getName() }));
+			HelpLocation helpLocation = actionState.getHelpLocation();
+			if (helpLocation != null) {
+				setHelpLocation(helpLocation);
+			}
+		}
+
+		@Override
+		public String getInceptionInformation() {
+			// we want the debug info for these internal actions to be that of the outer class
+			return MultiStateDockingAction.this.getInceptionInformation();
+		}
+
+		@Override
+		public void actionPerformed(ActionContext context) {
+			setCurrentActionStateWithTrigger(actionState, EventTrigger.GUI_ACTION);
+		}
+
+	}
+
 	private class ActionStateAction extends DockingAction {
 
 		private final ActionState<T> actionState;
 
-		private ActionStateAction(ActionState<T> actionState) {
+		private ActionStateAction(ActionState<T> actionState, boolean isSelected) {
 			super(actionState.getName(), "multiStateAction");
 			this.actionState = actionState;
+
 			setMenuBarData(
 				new MenuData(new String[] { actionState.getName() }, actionState.getIcon()));
 			HelpLocation helpLocation = actionState.getHelpLocation();

Ghidra/Framework/Docking/src/main/java/docking/menu/MultipleActionDockingToolbarButton.java

@@ -30,7 +30,7 @@ import docking.*;
 import docking.action.*;
 import docking.widgets.EmptyBorderButton;
 import docking.widgets.label.GDHtmlLabel;
-import ghidra.util.SystemUtilities;
+import ghidra.util.Swing;
 import resources.ResourceManager;
 
 public class MultipleActionDockingToolbarButton extends EmptyBorderButton {
@@ -81,6 +81,7 @@ public class MultipleActionDockingToolbarButton extends EmptyBorderButton {
 	 * to show the popup menu.  During normal operation, the user can only show the popup by
 	 * clicking the drop-down arrow.
 	 * 
+	 * @param performActionOnButtonClick true to perform the action when the button is clicked
 	 */
 	public void setPerformActionOnButtonClick(boolean performActionOnButtonClick) {
 		entireButtonShowsPopupMenu = !performActionOnButtonClick;
@@ -160,7 +161,11 @@ public class MultipleActionDockingToolbarButton extends EmptyBorderButton {
 		return manager.getActiveComponentProvider();
 	}
 
-	/** Show a popup containing all the actions below the button */
+	/** 
+	 * Show a popup containing all the actions below the button
+	 * @param listener for the created popup menu
+	 * @return the popup menu that was shown
+	 */
 	JPopupMenu showPopup(PopupMenuListener listener) {
 		JPopupMenu menu = new JPopupMenu();
 		List<DockingActionIf> actionList = multipleAction.getActionList(getActionContext());
@@ -307,7 +312,7 @@ public class MultipleActionDockingToolbarButton extends EmptyBorderButton {
 				// will update the focused window after we click.  We need the focus to be
 				// correct before we show, since our menu is built with actions based upon the
 				// focused dude.
-				SystemUtilities.runSwingLater(() -> popupMenu = showPopup(PopupMouseListener.this));
+				Swing.runLater(() -> popupMenu = showPopup(PopupMouseListener.this));
 
 				e.consume();
 				model.setPressed(false);