Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[win32] workaround for ImageDataProvider implementation #1569

Conversation

akoch-yatta
Copy link
Contributor

@akoch-yatta akoch-yatta commented Nov 1, 2024

This commit adds a workaround for ImageDataProvider implementations that instantiate new Images inside the getImageData(zoom) implementation. This image can be instantiated with the wrong zoom in a multi zoom setting as Images still relay on the static zoom value in DPIUtil. This workaround should be replaced by a proper solution.

How to reproduce

You can reproduce the behavior as follows:

  1. Set up at least two monitors with different zoom values (e.g., 100% and 125%)
  2. Start an Eclipse application with rescaling on runtime activated (and auto scale mode quarter).
  3. Place the workbench window on the 100% monitor. Note: if you move it between both monitors now, the minimize/maximize icons will be rescaled correctly.
  4. Move the workbench window from the 100% to the 125% monitor.
  5. Change focus to another application on the 100% monitor.
  6. Move the workbench window from the 125% to the 100% monitor.

Now the icons are scaled incorrectly for the active part stack. As soon as you switch focus to some other application again, the icons will be rescaled correctly.

Important note: This is not a permanent solution for this issue, but (due to being quite late in the current development cycle) meant as workaround for the 2024-12 release. A proper solution will require an API change, which is not possible now. I tried to implement the workaround as least invasive as possible

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Test Results

   483 files  ±0     483 suites  ±0   9m 36s ⏱️ + 1m 45s
 4 095 tests ±0   4 085 ✅ ±0   7 💤 ±0  3 ❌ ±0 
16 173 runs  ±0  16 080 ✅ ±0  90 💤 ±0  3 ❌ ±0 

For more details on these failures, see this check.

Results for commit bb4f693. ± Comparison against base commit ce2795a.

♻️ This comment has been updated with latest results.

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.

Workaround looks acceptable to me. We target the proper solution as a replacement for M1.

Can you please re-check the description for how to reproduce? I was not able to reproduce the behavior and I am not sure what point 6 is supposed to say. Should it be moving back to the 100% monitor?

Move the workbench window from the 125% to the 125% monitor.

Still, if I do that, everything looks fine even on master.

This commit adds a workaround for ImageDataProvider implementations that instantiate new Images inside the getImageData(zoom) implementation. This image can be instantiated with the wrong zoom in a multi zoom setting as Images still relay on the static zoom value in DPIUtil. This workaround should be replaced by a proper solution.

Contributes to eclipse-platform#62 and eclipse-platform#131
@akoch-yatta akoch-yatta force-pushed the workaround-imagedataprovider-in-multizoom branch from 1237105 to bb4f693 Compare November 4, 2024 14:00
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.

Thank you for the latest changes.

I was finally able to reproduce the issue (only in dark mode though).

This is how it looks when following the described scenario on master:
image

This is how it looks when following the described scenario on this change:
image

@HeikoKlare
Copy link
Contributor

Failing tests are documented and unrelated: #1564

@HeikoKlare HeikoKlare merged commit 5aeb924 into eclipse-platform:master Nov 5, 2024
12 of 14 checks passed
@HeikoKlare HeikoKlare deleted the workaround-imagedataprovider-in-multizoom branch November 5, 2024 09:43
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.

Minimize/Maximize Icons Scaled Incorrectly
2 participants