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

Implementation of Image to enable Multi Zoom Level Support for Win32 #1236

Merged

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented May 17, 2024

Addressed issues

Requires

Note: Only the last commit in this PR is to be reviewed. Previous commit(s) belong to the prerequisite PR(s)

Unlocks

Description

This pull request is based on the implementations of PR #1214. It extends to the native zoom which is provided within widget and propagated to GC. In this PR, we provide a map of different handles to Image for different device zoom levels as required by different monitors. The method win32_getHandle provides the appropriate handle for the scaled image as per requested by the client. The clients avail the zoom level information from the GCData and Widget (based on PR #1214 ).

@amartya4256 amartya4256 requested a review from niraj-modi as a code owner May 17, 2024 12:40
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from e096d61 to 1df54f4 Compare May 17, 2024 12:42
@amartya4256
Copy link
Contributor Author

@fedejeanne @HeikoKlare @akoch-yatta I have created the PR for Image. You can review it now.

@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 1df54f4 to 1fadba5 Compare May 17, 2024 12:58
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 2 times, most recently from daa481c to 3283909 Compare May 17, 2024 14:09
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 2 times, most recently from 8f77101 to 084b8f8 Compare May 21, 2024 15:56
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 084b8f8 to 7722383 Compare May 22, 2024 11:00
@amartya4256 amartya4256 requested a review from akoch-yatta May 23, 2024 11:34
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 4 times, most recently from 4953f00 to 295c87c Compare May 27, 2024 12:31
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 4 times, most recently from 21d6cad to 481d8b1 Compare June 4, 2024 11:12
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 481d8b1 to 694c853 Compare June 6, 2024 12:40
Copy link
Contributor

github-actions bot commented Jun 6, 2024

Test Results

   442 files  ±0     442 suites  ±0   11m 27s ⏱️ + 2m 44s
 4 126 tests ±0   4 118 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 318 runs  ±0  16 226 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit a01f91e. ± Comparison against base commit 78ef5a2.

This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.
org.eclipse.swt.graphics.ImageWin32Tests ‑ imageMustBeRescaledOnZoomChange
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 694c853 to fa8e857 Compare June 10, 2024 08:14
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 4 times, most recently from 898716d to 92aecaa Compare June 11, 2024 08:22
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.

Thank you for this contribution! I have some questions (for my own understanding) and some minor improvement proposals.

@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch 3 times, most recently from 163b584 to 5e68596 Compare June 13, 2024 09:21
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.

@amartya4256 thank you for addressing all my comments.

I tested the changes and all I could find was one missing image at the top of the window: the Eclipse icon

image

Can you please look into that?

@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 5e68596 to 3b9227e Compare June 14, 2024 10:22
This commit implements the mean to provide different handles for an Image at different zoom levels which can be consumed by the clients. All the clients are updated to use the new method to obtain the image as per their zoom level either obtained from the widget or GC.

Contributes to eclipse-platform#62 and eclipse-platform#127
@amartya4256 amartya4256 force-pushed the mutli_zoom_level_image branch from 3b9227e to a01f91e Compare June 14, 2024 10:44
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.

Better, it works now. Thanks!

@fedejeanne fedejeanne merged commit e0356e9 into eclipse-platform:master Jun 14, 2024
14 checks passed
@amartya4256
Copy link
Contributor Author

Fixed the issue with the icon of the shell. But I have also created a follow up issue which addresses the problem in the depth but it doesn't have to be the part of this PR because it addresses the usability of a method outside its scope.
https://github.com/orgs/vi-eclipse/projects/1/views/1?filterQuery=assignee%3Aamartya4256+47&pane=issue&itemId=67443386
FYI @akoch-yatta @fedejeanne @HeikoKlare

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.

3 participants