Adapt GetSystemMetrics call to be DPI dependent

[ShahzaibIbrahim]

NOTE: these changes affect only Windows

What it does

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 (Windows only!)

• Run the ControlExample
• Add these lines in your vm arguments
  -Dswt.autoScale=quarter
-Dswt.autoScale.updateOnRuntime=true
• Open the SWT Controls view
• Switch to the tab "Combo"/"List"/"Table"
• Change the size to 50x50 from Size section
• Move the window from 100 to 200 zoom level monitor
• See if the scroll bar is still visible

EXPECTED BEHAVIOUR

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

Contributes to #62 and #127:

[github-actions]

Test Results

   418 files  ±0     418 suites  ±0   8m 41s ⏱️ -43s
 4 121 tests ±0   4 113 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 313 runs  ±0  16 221 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit 20e1399. ± Comparison against base commit 08473c2.

♻️ This comment has been updated with latest results.

[fedejeanne]

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?

The request for changes is only because of the change in Table.

[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.

[fedejeanne]

One call is still wrong (look at the parameter)

[fedejeanne]

Thank you for fixing my findings. I took a deeper look and found some other stuff I would like your input on:

1. I noticed there are still some calls inside the widgets and the win32 package that are not using the new method, namely these 3 on top
   
   The first one (in the OS class) is probably OK because it asks for the value of SM_IMMENABLED which I assume is the same regardless of the current DPI settings (please confirm/rebut)
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?

NOTE: The request for changes is only because point nr 1.

[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.

Requested changes were addressed

[fedejeanne]

Thank you for addressing my change requests, @ShahzaibIbrahim . Would it be possible to rebase in origin/master so I can checkout your PR and take a deeper look at it?

[fedejeanne]

Changes look better now, I see that all the widgets go through the new method now ✔️

Since there has been some changes since the beginning of this PR, I would like to validate the behavior of the code myself. Some brief "How to test" and also "Expected (observable?) behavior" sections in the description of this PR would help.

[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