JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window #9470

tobiasholenstein · 2022-07-12T13:48:16Z

A graph in IGV can be moved by dragging it with the left mouse button (called panning).

If the mouse left the visible window of the graph during dragging, the diagram started to move in the opposite direction. This was annoying. Now panning stops as soon as the mouse leaves the window.

In selection mode, the graph still moves when the mouse is dragged outside the window, as this is meant to make a larger selection.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to 942a34d5
Tobias Hartmann (@TobiHartmann - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9470/head:pull/9470
$ git checkout pull/9470

Update a local copy of the PR:
$ git checkout pull/9470
$ git pull https://git.openjdk.org/jdk pull/9470/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9470

View PR using the GUI difftool:
$ git pr show -t 9470

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9470.diff

bridgekeeper · 2022-07-12T13:49:04Z

👋 Welcome back tholenstein! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-07-12T13:51:16Z

@tobiasholenstein The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-07-12T14:14:28Z

Webrevs

01: Full - Incremental (e6647572)
00: Full (942a34d5)

vnkozlov

Good.

openjdk · 2022-07-12T16:26:23Z

@tobiasholenstein This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8290016: IGV: Fix graph panning when mouse dragged outside of window

Reviewed-by: kvn, thartmann

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 141 new commits pushed to the master branch:

799a2c8: 8276561: URL$DefaultFactory::PREFIX should be static final
52cc6cd: 8288723: Avoid redundant ConcurrentHashMap.get call in java.time
3582fd9: 8290359: Ensure that all directory streams are closed in jdk.link
53fc495: 8290316: Ensure that all directory streams are closed in java.base
db1e44c: 8290353: ModuleReader::list specification should suggest closing the returned stream
2c73a1f: 8290324: Move atomic operations outside of os_xxx.hpp
e8975be: 8290746: ProblemList compiler/vectorization/TestAutoVecIntMinMax.java
9c19d89: Merge
17e65bb: 8290625: Test jdk/javadoc/tool/CheckManPageOptions.java after manpage update
618f3a8: 8278274: Update nroff pages in JDK 19 before RC
... and 131 more: https://git.openjdk.org/jdk/compare/393dc7ade716485f4452d0185caf9e630e4c6139...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

TobiHartmann · 2022-07-18T11:09:59Z

...aphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/CustomizablePanAction.java

            scrollPane = null;
-        return state ? State.createLocked (widget, this) : State.REJECTED;
+        }
+        return state ? State.REJECTED : State.REJECTED;


That does not look correct.

Thanks for catching that! You are right, this is not correct.

I updated the PR to make the locking more clear:

CustomizablePanAction extends WidgetAction.LockedAdapter which is used for long-term actions like panning/moving. All methods return the locked state if isLocked() is true and rejected otherwise.
The locked state means the event is processed and the processing has to stopped immediately (no other action should processed it).
The rejected state the event is not processed by the action and has to be processed by other actions too.

In our case CustomizablePanAction should be locked when the user presses the mouse inside of the panning area (the graph). And CustomizablePanAction should be unlocked when the mouse button is released again. Leaving the panning area (mouseExited) and entering it again (mouseEntered) should not change the locking state as long as the user does not release the mouse.

TobiHartmann

Looks reasonable.

...aphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/CustomizablePanAction.java

tobiasholenstein · 2022-07-21T14:52:51Z

thanks @vnkozlov and @TobiHartmann for the reviews!

tobiasholenstein · 2022-07-21T14:53:39Z

/integrate

openjdk · 2022-07-21T14:54:39Z

Going to push as commit 604a115.
Since your change was applied there have been 142 commits pushed to the master branch:

59e495e: 8290704: x86: TemplateTable::_new should not call eden_allocate() without contiguous allocs enabled
799a2c8: 8276561: URL$DefaultFactory::PREFIX should be static final
52cc6cd: 8288723: Avoid redundant ConcurrentHashMap.get call in java.time
3582fd9: 8290359: Ensure that all directory streams are closed in jdk.link
53fc495: 8290316: Ensure that all directory streams are closed in java.base
db1e44c: 8290353: ModuleReader::list specification should suggest closing the returned stream
2c73a1f: 8290324: Move atomic operations outside of os_xxx.hpp
e8975be: 8290746: ProblemList compiler/vectorization/TestAutoVecIntMinMax.java
9c19d89: Merge
17e65bb: 8290625: Test jdk/javadoc/tool/CheckManPageOptions.java after manpage update
... and 132 more: https://git.openjdk.org/jdk/compare/393dc7ade716485f4452d0185caf9e630e4c6139...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-07-21T14:54:56Z

@tobiasholenstein Pushed as commit 604a115.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window

e6675a2

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Jul 12, 2022

tobiasholenstein marked this pull request as ready for review July 12, 2022 14:07

openjdk bot added the rfr Pull request is ready for review label Jul 12, 2022

remove imports

942a34d

vnkozlov approved these changes Jul 12, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 12, 2022

TobiHartmann suggested changes Jul 18, 2022

View reviewed changes

fixed locking in CustomizablePanAction

e664757

TobiHartmann approved these changes Jul 21, 2022

View reviewed changes

...aphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/CustomizablePanAction.java Show resolved Hide resolved

openjdk bot added the integrated Pull request has been integrated label Jul 21, 2022

openjdk bot closed this Jul 21, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window #9470

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window #9470

Uh oh!

tobiasholenstein commented Jul 12, 2022 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Jul 12, 2022

Uh oh!

openjdk bot commented Jul 12, 2022

Uh oh!

mlbridge bot commented Jul 12, 2022 •

edited

Loading

Uh oh!

vnkozlov left a comment

Uh oh!

openjdk bot commented Jul 12, 2022 •

edited

Loading

Uh oh!

TobiHartmann Jul 18, 2022

Uh oh!

tobiasholenstein Jul 20, 2022

Uh oh!

TobiHartmann left a comment

Uh oh!

Uh oh!

tobiasholenstein commented Jul 21, 2022

Uh oh!

tobiasholenstein commented Jul 21, 2022

Uh oh!

openjdk bot commented Jul 21, 2022

Uh oh!

openjdk bot commented Jul 21, 2022

Uh oh!

Uh oh!

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window #9470

JDK-8290016: IGV: Fix graph panning when mouse dragged outside of window #9470

Uh oh!

Conversation

tobiasholenstein commented Jul 12, 2022 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 12, 2022

Uh oh!

openjdk bot commented Jul 12, 2022

Uh oh!

mlbridge bot commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiHartmann Jul 18, 2022

Choose a reason for hiding this comment

Uh oh!

tobiasholenstein Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobiasholenstein commented Jul 21, 2022

Uh oh!

tobiasholenstein commented Jul 21, 2022

Uh oh!

openjdk bot commented Jul 21, 2022

Uh oh!

openjdk bot commented Jul 21, 2022

Uh oh!

Uh oh!

tobiasholenstein commented Jul 12, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 12, 2022 •

edited

Loading

openjdk bot commented Jul 12, 2022 •

edited

Loading