Initialize nativeZoom from Shell via OS call #2196
Merged
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
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
fd4658a to
ba0696a
Compare
Contributor
|
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. |
Contributor
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
ba0696a to
057aa81
Compare
akoch-yatta
approved these changes
Jun 13, 2025
Contributor
akoch-yatta
left a comment
akoch-yatta
left a comment
There was a problem hiding this comment.
Changes look good to me. I tested it a bit (opening sub-shells or second workbench windows, moving around, re-opening) with the latest I-Build by replacing the Shell class in the jar and didn't find any issues with it.
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
057aa81 to
e56e184
Compare
Member
|
@ShahzaibIbrahim can you please document/link the failing tests and also bump the versions in the appropriate MANIFEST and POM files? You can use the Git patch that the bot created (#2196 (comment)) as a reference. |
HeikoKlare
reviewed
Jun 13, 2025
Contributor
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Is this just supposed to be a simplification or does it also improve the behavior in some situations, does it allow to remove some other tweaks or the like?
Contributor
Author
It also improves behavior in some situation as mentioned here in this ticket -> vi-eclipse/Eclipse-Platform#315 (comment), but this issue only happens with ibuilds not runtime workspace. That's why we have checked if this change doesn't break anything. |
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
e56e184 to
334c9af
Compare
Contributor
|
Okay, I will need to have a deeper look. This seems almost identical (except for the parent check) than #2165, which produced a severe regression when I tested it. The mentioned ticket also asks to reevaluate vi-eclipse/Eclipse-Platform#241, so whether the issue can be solved without this workaround: eclipse-platform/eclipse.platform.ui#2948 Did you already test that? Is that a scenario that we can use to valudate the improved behavior due to this PR? |
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
334c9af to
b2bd61a
Compare
Shell can be created on secondary monitor but inheriting the zoom of parent (which could be on primary monitor) means the DPI change will not be triggered.
ShahzaibIbrahim
force-pushed
the
master-315
branch
from
b2bd61a to
00c5b11
Compare
ShahzaibIbrahim
commented
Jun 23, 2025
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Shell.java
Show resolved
Hide resolved
Member
|
FTR the failing check is unrelated and the versions have been bumped in Check failure (check-versions)
@HannesWell WDYT? |
fedejeanne
approved these changes
Jun 23, 2025
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Shell.java
Show resolved
Hide resolved
Member
This is really strange. Sorry, I don't had the time yet to look into this, but I try to do it ASAP. |
Member
|
No worries, it seemed pretty clear that this PR wasn't to blame so I went ahead and merged it :-) . Yesterday's I-BUILD seems "OK" (for SWT at least --> eclipse-jdt/eclipse.jdt.ui#2287) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Shell can be created on secondary monitor but inheriting the zoom of parent (which could be on primary monitor) means the DPI change will not be triggered.