Skip to content

Conversation

@akoch-yatta
Copy link
Contributor

@akoch-yatta akoch-yatta commented Mar 31, 2025

This PR addresses issues, when DnD across multiple monitors with monitors specific scaling is used. If the monitors have different zoom dragging into a table or tree on the second monitor will detect the wrong destination items depending on the zoom. To address this the zoom should always be inherited from the underlying control if available.

How to test

Test setup

Monitor setup with two different monitors:

  • Primary should be the smaller zoom (e.g. 100)
  • Secondary should be the bigger zoom (e.g. 150)
  • Start the runtime IDE with monitor specific scaling active

Test scenario

  1. Open and position the IDE on the secondary monitor and the File explorer on the primary monitor
  2. Open the Project explorer with at least one project to drag into
  3. Drag a file from the File explorer over to the project
  4. You will notice the hover overlay does not highlight the correct entry

@github-actions
Copy link
Contributor

github-actions bot commented Mar 31, 2025

Test Results

   545 files  +  140     545 suites  +140   28m 52s ⏱️ + 3m 25s
 4 373 tests +   37   4 361 ✅ +   35   12 💤 + 3  0 ❌  - 1 
16 634 runs  +4 123  16 520 ✅ +4 101  114 💤 +23  0 ❌  - 1 

Results for commit 17c4d08. ± Comparison against base commit 00c6aef.

♻️ This comment has been updated with latest results.

@akoch-yatta akoch-yatta force-pushed the fix-dnd-across-monitors branch from af218d2 to 8fd4c01 Compare March 31, 2025 10:39
@akoch-yatta akoch-yatta marked this pull request as draft March 31, 2025 11:48
@akoch-yatta akoch-yatta force-pushed the fix-dnd-across-monitors branch 2 times, most recently from 14d5d9e to 9fd4c4a Compare April 4, 2025 08:30
@akoch-yatta akoch-yatta marked this pull request as ready for review April 4, 2025 08:46
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks sound to me. The conversion via the control makes sense and I do not see any issues with that.

I have tested the change with different configuration and with both monitor-specific scaling enabled and disabled. I was not able to find any regressions but found the behavior for monitor-specific scaling improved in at least two kinds of cases:

  1. The scenario described in the PR, i.e., dragging from primary monitor (e.g., using Windows explorer) and dropping into an Eclipse application on secondary monitor
  2. Drag-and-drop inside an Eclipse application on the primary monitor. The video below shows the existing and improved behavior for that case.

drag_drop_fix

I only have a really nitpick comment/question regarding naming, but despite that I am in favor of merging this.

@akoch-yatta akoch-yatta force-pushed the fix-dnd-across-monitors branch 2 times, most recently from 7166cfe to da73a60 Compare April 17, 2025 10:11
@HeikoKlare HeikoKlare force-pushed the fix-dnd-across-monitors branch 3 times, most recently from 4b62fd6 to 69e450f Compare April 18, 2025 16:23
This commit addresses issues, when DnD across multiple monitors with
monitors specific scaling is used. If the monitors have different zoom
dragging into a table or tree on the second monitor will detect the wrong
destination items depending on the zoom. To address this the zoom should
always be inherited from the underlying control if available.
@HeikoKlare HeikoKlare force-pushed the fix-dnd-across-monitors branch from 69e450f to 17c4d08 Compare April 22, 2025 11:48
@HeikoKlare
Copy link
Contributor

Merging this despite Jenkins failure caused by https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/5892

Since master builds seem to still work on Jenkins, the subsequent master build should hopefully work, thus I will check results of that build to retrospectively validate this PR.

@HeikoKlare HeikoKlare merged commit cc248ab into eclipse-platform:master Apr 22, 2025
10 of 12 checks passed
@HeikoKlare HeikoKlare deleted the fix-dnd-across-monitors branch April 22, 2025 12:01
@HeikoKlare
Copy link
Contributor

No new issues on subsequent master build for 6517c9e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong drop position on non-primary monitor

2 participants