Adapt GetSystemMetrics call to be DPI dependent

[ShahzaibIbrahim]

Writing GetSystemMetrics method in Widget class so that the child controls can use it to get System Metrics to be DPI dependent. There are some visible affected areas (before and after zooming to 200 from 100) for which screenshots are attached.

The OS.getSystemMetrics call had a slight issue in some of the controls i.e. Combo, List and Table. The values returned for scroll i.e. OS.SM_CXVSCROLL were independent of DPI. So moving the shell from smaller zoom level to higher zoom level we can see the visible difference in the scroll bar which is not visible in the latter part.

HOW TO TEST

Compare the values returned for new and old method when sending the OS.SM_CXVSCROLL call to get system metrics and check the difference or you can do the manual testing (before and after the new call) for which screenshots are also attached.

EXPECTED BEHAVIOUR

None of the widgets should look out of order when changing the zoom level.

Contributes to #62 and #127:

[github-actions[bot]]

Test Results

   450 files  ±0     450 suites  ±0   7m 44s :stopwatch: -16s  4 127 tests ±0   4 119 :white_check_mark: ±0   8 :zzz: ±0  0 :x: ±0  16 319 runs  ±0  16 227 :white_check_mark: ±0  92 :zzz: ±0  0 :x: ±0 

Results for commit 95051036. ± Comparison against base commit a3cf373e.

:recycle: This comment has been updated with latest results.

[fedejeanne]

@ShahzaibIbrahim please also improve the commit message:

• Remove the links from the first line (they are in the body anyway)
• Add the text "Fixes" or "Contributes to" appropriately for the linked issues

[ShahzaibIbrahim]

I see that this PR changes (all?) calls to OS.GetSystemMetrics for a call to a new intermediate method in Widget: why not simply change OS.GetSystemMetrics and put the new logic in there?

Not all calls, just the child of all widgets, also these are OS related calls, we can't really change the implementation of it. That's why created one intermediate call for all of them.

[ShahzaibIbrahim]

2. What would happen if one removes the if (OS.WIN32_BUILD >= OS.WIN32_BUILD_WIN10_1607) in the new method and always calls OS.GetSystemMetricsForDpi(...)? Would the application crash in older Windows versions?

I am not sure if the applications crashes in older version, and also can't test it but looking at the usage for dpi dependent OS calls, this check is always used which makes me think that it may have some affects on older windows versions.

[fedejeanne]

@ShahzaibIbrahim thank you for adding a How to test. I tested in Windows 10 and I see no difference before and after this PR.

Before (Table, size 100x100)

Zoom = 100

Zoom = 200

After

Zoom = 100

Zoom = 200

Maybe I'm testing it wrong?

[ShahzaibIbrahim]

Maybe I'm testing it wrong?

No you have tested it correctly. I just forgot to mention that you need to add these vm arguments while running the example.

-Dswt.autoScale=quarter -Dswt.autoScale.updateOnRuntime=true

[fedejeanne]

I tested it again and I still see no difference between before and after this PR. Moreover, the status before this PR looks OK (I see both scroll bars when moving to the monitor with zoom = 200. Can you please confirm?

Before (Table size 100x100)

Zoom = 100

Zoom = 200 According to the initial description of the bug, this image should show a missing horizontal scrollbar, right?

After (Table size 100x100)

Zoom = 100

Zoom = 200

[ShahzaibIbrahim]

I tested it again and I still see no difference between before and after this PR. Moreover, the status before this PR looks OK (I see both scroll bars when moving to the monitor with zoom = 200. Can you please confirm?

Please check the scroll with 50x50 size and you will see the difference visibly. I have also added this in How to test section.

[akoch-yatta]

I tested it again and I still see no difference between before and after this PR. Moreover, the status before this PR looks OK (I see both scroll bars when moving to the monitor with zoom = 200. Can you please confirm?

If a scrollbar is shown or not could perhaps be a side effect in some scenarios, but the main reason for that change is that the correct value for scroolbar sizes are used when the size for the table is calculated. You'll see the effect in the following screenshot:

Both are started on 100% and moved to 200% (That is when the screenshots were made). The left one uses OS.GetSystemMetrics, the right one uses OS.GetSystemMetricsForDpi. You'll see the left table is smaller, because it still uses the scrollbar sizes for the primary monitor (=OS.GetSystemMetrics)

[iloveeclipse]

This change caused severe regression in the compare editor, see https://github.com/eclipse-platform/eclipse.platform/issues/1489.