RFR: 8343535: IGV: Colorize nodes on demand [v2]

[Roberto Castañeda Lozano]

On Wed, 6 Nov 2024 20:30:26 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> <img width="292" alt="color" src="https://github.com/user-attachments/assets/ea050a39-5f1e-456b-adaa-2102d78b0f60">
>> 
>> <img width="697" alt="pick" src="https://github.com/user-attachments/assets/67569b5d-1d74-4a93-8a97-bb37c5bc9f93">
>> 
>> <img width="428" alt="nodes" src="https://github.com/user-attachments/assets/c8a994e3-9946-42ae-8701-34223a000a0c">
>> 
>> Adds new option to IGV to color selected nodes:
>> 1) select some nodes
>> 2) `Ctrl + C` or `View` -> `Color action`
>> 3) pick a color and apply
>
> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/DiagramScene.java
>   
>   Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>

In my opinion, the IGV toolbar is already pretty crowded (this hurts most when opening two graphs side-by-side) and I would prefer not adding the color icon there. On the other hand, we could add the action to the pop-up menu that's opened when right-clicking into a node or set of nodes. Here's my suggestion:


diff --git a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java
index e68abd3297e..c4f2ac670e7 100644
--- a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java
+++ b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java
@@ -100,6 +100,7 @@ public EditorTopComponent(DiagramViewModel diagramViewModel) {
         };
 
         Action[] actionsWithSelection = new Action[]{
+                ColorAction.get(ColorAction.class),
                 ExtractAction.get(ExtractAction.class),
                 HideAction.get(HideAction.class),
                 null,
@@ -168,8 +169,6 @@ public void mouseMoved(MouseEvent e) {}
         toolBar.add(ReduceDiffAction.get(ReduceDiffAction.class));
         toolBar.add(ExpandDiffAction.get(ExpandDiffAction.class));
         toolBar.addSeparator();
-        toolBar.add(ColorAction.get(ColorAction.class));
-        toolBar.addSeparator();
         toolBar.add(ExtractAction.get(ExtractAction.class));
         toolBar.add(HideAction.get(HideAction.class));
         toolBar.add(ShowAllAction.get(ShowAllAction.class));
diff --git a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ColorAction.java b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ColorAction.java
index a51934a4322..92921c81512 100644
--- a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ColorAction.java
+++ b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ColorAction.java
@@ -43,7 +43,7 @@
         @ActionReference(path = "Shortcuts", name = "D-C")
 })
 @Messages({
-        "CTL_ColorAction=Color action",
+        "CTL_ColorAction=Color",
         "HINT_ColorAction=Color current set of selected nodes"
 })
 public final class ColorAction extends ModelAwareAction {
diff --git a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExtractAction.java b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExtractAction.java
index 24815527a0e..c2329cbb26f 100644
--- a/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExtractAction.java
+++ b/src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExtractAction.java
@@ -42,7 +42,7 @@
         @ActionReference(path = "Shortcuts", name = "D-X")
 })
 @Messages({
-        "CTL_ExtractAction=Extract action",
+        "CTL_ExtractAction=Extract",
         "HINT_ExtractAction=Extract current set of selected nodes"
 })
 public final class ExtractAction extends ModelAwareAction {


@tobiasholenstein @chhagedorn what do you think? If you agree, feel free to merge the patch into this PR.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21925#issuecomment-2464175649