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

[Win] Disallow autoscale mode "integer" for monitor-specific scaling #1721

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HeikoKlare
Copy link
Contributor

The default auto-scale value is "integer"/"integer200", which makes everything in the UI except fonts not scale according to the actual native zoom value but according to a value rounded to 100 or 200. While most code performing adaptations to zoom considers this for a static zoom value applied to the whole application, it leads to hard-to-resolve issues when scaling every shell according to the current monitor's zoom.

Since that autoscale mode is complex and should be replaced by uniform scaling of everything (in particular when images are based on vector graphics that can sharply be rendered for every zoom factor) anyway, this is not to be supported by the monitor-specific scaling feature currently being introduced for Windows. With this change, that mode will thus be limited to reasonable autoscale modes like "quarter" and "exact" or ones fixing the zoom value like specifying an explicit value or "false".

@HeikoKlare HeikoKlare requested a review from fedejeanne January 13, 2025 20:31
@HeikoKlare
Copy link
Contributor Author

@akoch-yatta

Copy link
Contributor

github-actions bot commented Jan 13, 2025

Test Results

   485 files   -  1     485 suites   - 1   11m 43s ⏱️ + 2m 35s
 4 272 tests  - 37   4 261 ✅  - 35   10 💤  - 3  1 ❌ +1 
16 377 runs   - 37  16 271 ✅  - 35  105 💤  - 3  1 ❌ +1 

For more details on these failures, see this check.

Results for commit d3123ef. ± Comparison against base commit d7be959.

This pull request removes 37 tests.
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare force-pushed the autoscale-quarter-for-monitor-specific-scaling branch from 21aeca3 to 1900fcd Compare January 13, 2025 20:46
@HeikoKlare HeikoKlare marked this pull request as ready for review January 14, 2025 07:59
Copy link
Contributor

@akoch-yatta akoch-yatta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, only one question as stated in my comment.

}
}

private static boolean isSupportedAutoScaleForMonitorSpecificScaling() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thought about: should we have a comment here, why only those values are supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a good idea. I added some documentation about why only those values are supported. @akoch-yatta can you have a look whether you think the description is okay?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think is explanation is good

The default auto-scale value is "integer"/"integer200", which makes
everything in the UI except fonts not scale according to the actual
native zoom value but according to a value rounded to 100 or 200. While
most code performing adaptations to zoom considers this for a static
zoom value applied to the whole application, it leads to hard-to-resolve
issues when scaling every shell according to the current monitor's zoom.

Since that autoscale mode is complex and should be replaced by uniform
scaling of everything (in particular when images are based on vector
graphics that can sharply be rendered for every zoom factor) anyway,
this is not to be supported by the monitor-specific scaling feature
currently being introduced for Windows. With this change, that mode will
thus be limited to reasonable autoscale modes like "quarter" and "exact"
or ones fixing the zoom value like specifying an explicit value or
"false".
@HeikoKlare HeikoKlare force-pushed the autoscale-quarter-for-monitor-specific-scaling branch from 1900fcd to d3123ef Compare January 14, 2025 15:27
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.

Restrict autoScale to Allowed Values Only
2 participants