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 transform to enable Multi Zoom Level Support for Win32 #1276

Merged

Conversation

amartya4256
Copy link
Contributor

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 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 Transform for different zoom levels as required by different monitors. The method win32_getHandle provides the appropriate handle for the scaled transform as per requested by the client. So far, Transform is only used by GC in the project, so all the calls from GC has been adapted to use the new method to obtasin the right handle.

@amartya4256 amartya4256 requested a review from niraj-modi as a code owner June 12, 2024 11:48
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch 4 times, most recently from 5d597a6 to a2fd51a Compare June 12, 2024 11:59
Copy link
Contributor

github-actions bot commented Jun 12, 2024

Test Results

   458 files  +8     458 suites  +8   6m 32s ⏱️ -14s
 4 129 tests +2   4 121 ✅ +2   8 💤 ±0  0 ❌ ±0 
16 321 runs  +2  16 229 ✅ +2  92 💤 ±0  0 ❌ ±0 

Results for commit 778da20. ± Comparison against base commit f708ac3.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch from a2fd51a to c35ce40 Compare June 12, 2024 13:26
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch 2 times, most recently from 4ecc631 to 934b2c5 Compare June 17, 2024 13:32
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch from 43f31d9 to 6ae5413 Compare June 19, 2024 15:45
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.

Looks better thank you!

Some remarks though:

1 leaks found, the first is:
java.lang.AssertionError: 1 leaks found, the first is:
	at org.eclipse.swt.tests.junit.AllNonBrowserTests.afterClass(AllNonBrowserTests.java:92)
	...
Caused by: java.lang.Error: SWT Resource was not properly disposed
	at org.eclipse.swt.graphics.Resource.initNonDisposeTracking(Resource.java:172)
	at org.eclipse.swt.graphics.Resource.<init>(Resource.java:120)
	at org.eclipse.swt.graphics.Transform.<init>(Transform.java:140)
	at org.eclipse.swt.graphics.Transform.<init>(Transform.java:72)
	at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_graphics_GC.test_drawLine_noSingularitiesIn45DregreeRotation(Test_org_eclipse_swt_graphics_GC.java:750)
	...

Could you please look into that?

@fedejeanne fedejeanne mentioned this pull request Jun 20, 2024
1 task
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch 2 times, most recently from 0b75ddb to 5884fdc Compare June 20, 2024 11:24
@fedejeanne fedejeanne dismissed their stale review June 20, 2024 11:41

All fixed

@fedejeanne fedejeanne force-pushed the multi_zoom_level_transform branch from 5884fdc to 3fc1f56 Compare June 20, 2024 11:42
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch from 3fc1f56 to bc1b223 Compare June 20, 2024 11:44
This commit implements the mean to provide different handles of a transform object at different zoom levels which can be consumed by GC depending on the zoom level available within its GCData.

Contributes to eclipse-platform#62 and eclipse-platform#127
@amartya4256 amartya4256 force-pushed the multi_zoom_level_transform branch from bc1b223 to 778da20 Compare June 20, 2024 11:46
@fedejeanne fedejeanne merged commit f634e24 into eclipse-platform:master Jun 21, 2024
14 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