RFR: JDK-8290010: IGV: Fix UndoRedo Action

Tobias Hartmann thartmann at openjdk.org
Mon Oct 24 10:44:54 UTC 2022


On Fri, 21 Oct 2022 11:53:31 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 stated 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`.

In general, it seems that when going backwards (undo) multiple steps, going forwards (redo) does not work anymore after one step forward.

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

Changes requested by thartmann (Reviewer).

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


More information about the hotspot-compiler-dev mailing list