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] Refactor toolbar dpi callback #1364

Merged

Conversation

akoch-yatta
Copy link
Contributor

This PR mainly is targeted on refactoring the existing callback for ToolBar and ToolItem after a DPI change in the win32 implementation.
To achieve that a method to scale an ImageList was added. Scaled variants of an image list are saved and reused similar to the other Resources.
The refactoring uses the scalable ImageLists to reuse the existing toolbar items and only resets the ImageList with a correctly scaled one. Due to the limitations of the win32 implementation all items must be removed and re-added via the win32-API to make shrinking of toolbars possible. Additionally, the commit removes the now unused callback in ToolItem.

All calls to the ImageList handle are replaced to use the added method.

@akoch-yatta akoch-yatta changed the title Refactor toolbar dpi callback [win32] Refactor toolbar dpi callback Jul 25, 2024
@akoch-yatta akoch-yatta force-pushed the refactor-toolbar-dpi-callback branch from 4e266f6 to 5104b44 Compare July 25, 2024 12:57
Copy link
Contributor

github-actions bot commented Jul 25, 2024

Test Results

   478 files  ±0     478 suites  ±0   7m 6s ⏱️ - 1m 32s
 4 147 tests ±0   4 139 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 354 runs  ±0  16 262 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit ee0b8d7. ± Comparison against base commit 29a1795.

♻️ This comment has been updated with latest results.

@akoch-yatta akoch-yatta force-pushed the refactor-toolbar-dpi-callback branch from 5104b44 to 530dc79 Compare July 25, 2024 13:09
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.

I have not tested the proposal yet, but I have taken a first look at the code. So far, I only have minor comments.

@akoch-yatta akoch-yatta force-pushed the refactor-toolbar-dpi-callback branch 5 times, most recently from c3ea35b to 09f2eb6 Compare July 26, 2024 12:18
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. I've also tested it (in combination with akoch-yatta/eclipse.platform.ui@384e390 to properly see effects on the main toolbar) and found it working fine.

@fedejeanne maybe you could also have a look since parts of the change slightly affect the behavior also when using existing HiDPI support, so it would be good to have an additional review.

This commit adds a method to receive handles for scaled variants of an ImageList for the specified zoom. All calls to the handle of an ImageList is replaced by the added method.

Contributes to eclipse-platform#62 and eclipse-platform#131.
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

LGTM, just 1 nit

This commit refactors the callback use for ToolBar and ToolItem after a DPI change in the win32 implementation. It makes use of the scalable ImageList to reuse the existing items in the toolbar and only resets the ImageList with a correctly scaled one. Due to the limitations of the win32 implementation all items must be removed and re-added via the win32-API to make shrinking of toolbars possible. Additionally, the commit removes the now unused callback in ToolItem

Contributes to eclipse-platform#62 and eclipse-platform#131.
@akoch-yatta akoch-yatta force-pushed the refactor-toolbar-dpi-callback branch from 577b77c to ee0b8d7 Compare July 29, 2024 12:59
@fedejeanne fedejeanne merged commit 663d109 into eclipse-platform:master Jul 29, 2024
14 checks passed
@akoch-yatta akoch-yatta deleted the refactor-toolbar-dpi-callback branch September 13, 2024 12:42
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.

4 participants