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 Pattern to enable multi zoom level support for win32 #1265

Merged

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Jun 6, 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)

Description

This PR introduces multi zoom level support of pattern using a map which maintains patterns at different zoom level. The map is used by a package protected method which is used in GC in the place where the handle of patterns are needed. Since, testing patterns automatically for how they look visually is a bit difficult, so there's a snippet created for manual testing of the same pattern at different zoom levels.

@amartya4256 amartya4256 requested a review from niraj-modi as a code owner June 6, 2024 12:39
@amartya4256 amartya4256 force-pushed the multi_zoom_level_pattern branch 2 times, most recently from 4ba3d27 to 37053e2 Compare June 6, 2024 12:50
@amartya4256 amartya4256 changed the title Implementation of FontMetrics to enable multi zoom level support for win32 Implementation of Pattern to enable multi zoom level support for win32 Jun 6, 2024
Copy link
Contributor

github-actions bot commented Jun 6, 2024

Test Results

   450 files  ±0     450 suites  ±0   7m 34s ⏱️ -26s
 4 127 tests ±0   4 119 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 319 runs  ±0  16 227 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit 075f477. ± Comparison against base commit a3cf373.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the multi_zoom_level_pattern branch 6 times, most recently from 61b0a41 to 76e5391 Compare June 10, 2024 10:00
@amartya4256 amartya4256 force-pushed the multi_zoom_level_pattern branch 2 times, most recently from 24473af to bc79513 Compare June 12, 2024 14:14
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.

#1236 is merged. Rebase on master and resolve the conflicts.

@amartya4256 amartya4256 force-pushed the multi_zoom_level_pattern branch from bc79513 to 7748ad3 Compare June 14, 2024 13:12
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 contributing this. I added my initial evaluation. Once these have been resolved and the basics (checklist) have been checked I can move to the testing phase.

@amartya4256 amartya4256 force-pushed the multi_zoom_level_pattern branch 2 times, most recently from c789fa1 to ab6e47c Compare June 17, 2024 12:06
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.

Sorry, I forgot about one: rename the private method in the test

This commit contributes to multi zoom level support of pattern using a map of zoom levels with relevant patterns.

Contributes to eclipse-platform#62 and eclipse-platform#131
@fedejeanne fedejeanne force-pushed the multi_zoom_level_pattern branch from 0f3c523 to 075f477 Compare June 18, 2024 08: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.

I tested the Snippet219 and when I move from the 100% monitor to the 200% monitor I see that the region (content) is resized but the shell isn't

100%
image

200% (ignore the background, it's my wallpaper)
image

Can you please look into that @amartya4256 ?

EDIT: wrong PR --> moved to #1278 (review)

@amartya4256
Copy link
Contributor Author

I tested the Snippet219 and when I move from the 100% monitor to the 200% monitor I see that the region (content) is resized but the shell isn't

100% image

200% (ignore the background, it's my wallpaper) image

Can you please look into that @amartya4256 ?

Do you mean for this PR: #1278?

@fedejeanne fedejeanne dismissed their stale review June 19, 2024 09:10

I mistkingly requested changes that were for the PR #1278

@fedejeanne fedejeanne merged commit f708ac3 into eclipse-platform:master Jun 19, 2024
9 of 11 checks passed
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