Fix zoom issue during copy paste in clipboard example #2158
+6
−40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arunjose696
force-pushed
the
arunjose696/194
branch
2 times, most recently
from
56fcbd4 to
a80f142
Compare
arunjose696
force-pushed
the
arunjose696/194
branch
2 times, most recently
from
e501e52 to
c691984
Compare
HeikoKlare
approved these changes
May 30, 2025
There was a problem hiding this comment.
The change is sound. Always using the 100% version of an image via copy/paste makes sense. I was wondering about situations where an image is provided in at different resolution for different zooms (such as we do for icons with the @2x version or as would be possible for SVGs), but that does not seem to be a reasonable use case for copy/paste / DnD and in any case you would not know which zoom to actually use.
In addition, this aligns the implementation with the (also HiDPI-aware) implementation on MacOS (and Linux).
I have tested the change on Windows and MacOS (as the ClipboardExample is OS-agnostic). On Windows, I tested different primary monitor zooms (including values that are mapped to both device zooms of 100% and 200% with default int200-autoscaling).
It seems to work as expected.
I have also tested with monitor-specific scaling enabled (i.e., running the snippet with -Dswt.autoScale.updateOnRuntime=true) on a two-monitor setup with 200% and 100%. Behavior was completely broken before when switching between the monitors and now behaves properly fine and just as expected.
Actually, I think this is the proper solution for basically the same issue discussed 9 years ago: https://bugs.eclipse.org/bugs/show_bug.cgi?id=493047
There also a proposal for always using the image data at 100% was made, but was ignored in favor of using the adaptations to the ClipboardExample.
So I am all for merging this for 4.37 M1. But since this affects integral copy/paste behavior, I would like to have someone else have a look at the change and its behavior as well (@fedejeanne @akoch-yatta).
akoch-yatta
approved these changes
Jun 3, 2025
|
Just one minor request @arunjose696: could you please improve the commit message to have a short, comprehensive header and more information in a separated explaining body? You may just restructure the existing commit message or also take parts of your PR message for a more expressive commit body. See also the commit message commendations. |
arunjose696
force-pushed
the
arunjose696/194
branch
from
c691984 to
33f0ba6
Compare
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
arunjose696
force-pushed
the
arunjose696/194
branch
from
33f0ba6 to
a84a61b
Compare
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
Done, should I patch my branch as per the bots comment? |
Thank you, but please only link to issues in the official SWT repository, not in the one we use for our work planning. In this case, we don't have an issue in this repo to link to, so you can just leave out those links. Sorry if I confused here by linking to the commit message recommendations.
In general, yes, but please wait until the master branch is open again as the patch is currently not correct because we are in the transition to the new development stream. Once that's done, the bot should creat a proper patch proposal that you can add as a separate commit to the PR. It will contain required version bumps for the new development cycle. |
Yeah, I misunderstood I thought those were the commit message guidelines for eclipse-platform. Thanks for clarifying! 😅 Looks like I’ll need to go back and edit all the commit messages for other PRs I just updated. |
Previously, when copying an image in the clipboard example, the image at the current zoom level was stored to clipboard using getImageDataAtCurrentZoom(). However, in normal Windows behavior (e.g., Paint), images are always copied at their original size regardless of the current zoom. When pasting, the image is pasted at 100% zoom, and the application handles zoom internally when displaying the image. To match this behavior, copy and paste now always handle images at 100% scale, ignoring the current zoom and leaving zoom handling to the image class when it is fetched for display.
arunjose696
force-pushed
the
arunjose696/194
branch
from
a84a61b to
34111e8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Copy and paste no longer tries to autoscale the image. The image is always copied and pasted at 100% zoom.
The purpose of this change is to:
DPIUtil.get[Native]DeviceZoom()Previously, when copying an image in the clipboard example, the image at the current zoom level was stored to clipboard using
getImageDataAtCurrentZoom(). However, in normal Windows behavior (e.g., Paint), images are always copied at their original size regardless of the current zoom. When pasting, the image is pasted at 100% zoom, and the application handles zoom internally when displaying the image.To match this behavior, copy and paste now always handle images at 100% scale, ignoring the current zoom and leaving zoom handling to the image class when it is fetched for display.