RFR: JDK-8290010: IGV: Fix UndoRedo Action [v2]

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Mon Oct 24 14:35:56 UTC 2022


On Mon, 24 Oct 2022 11:58:51 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> # Problem
>> - The history of the Undo Action did not start at zero when opening a new graph.
>> - In the "Show sea of nodes" view you could press Undo once before doing any changes. 
>> - In the "Show control view graph" you could even go back many times when opening a new graph, ending up in wired state of the graph. 
>> - Selecting a node should not be added to the history. You could select X different nodes in a row and then go back X times in history without that anything changed. 
>> 
>> # Overview UndoRedo
>> Different `ChangedEvent<DiagramViewModel>` events are fired in `DiagramViewModel` that trigger the `DiagramScene` to update its state. After those updates `DiagramScene` calls  `addUndo()` to save the new state of the `DiagramViewModel` as well as the scrolling position of the `DiagramScene` in a `DiagramUndoRedo` object. `undo()` / `redo()` can then be used to restore the saved state.
>> 
>> One problem was that selecting nodes triggered a `ChangedEvent<DiagramViewModel>` event that caused a recording to the history but the selection itself was not part of the saved state. Another problem was that switching between different views (CFG view, sea-of-nodes view) as well as applying filters also triggered an `addUndo()` - The recording of filters and view switches is however not implemented. 
>> 
>> # Solution
>> We now only record when the graph itself changes (not the view) - like when opening a new graph or creating a difference graph. And we also record changes in the selection of the visible nodes. 
>> 
>> We refactored the `ChangedEvent<DiagramViewModel>` events in `DiagramViewModel`: 
>> - `graphChangedEvent` now is fired when the graph changes and triggers an `addUndo()`
>> - `hiddenNodesChangedEvent` is now fired when the selection of visible nodes changes and also triggers an `addUndo()`
>> 
>> `DiagramUndoRedo` previously stored a deep copy of the `DiagramViewModel` for every datapoint in the history. Besides using a lot of memory, most of the stored objects were redundant or not used. Now, we introduced a new `ModelState` object that stores the id of the visible nodes in `Set<Integer> hiddenNodes` as well as the opened graph. For the graph we only need `int firstPos` and `int secondPos` which indicate the position of the difference graph within the group. If we don't use a difference graph `firstPos` == `secondPos`.
>
> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   disable undoRedo during execution of an undo/redo

Thanks for making the undo/redo functionality usable!

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

Marked as reviewed by rcastanedalo (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10813


More information about the hotspot-compiler-dev mailing list