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

Reduce usages of LOGPIXELSX and LOGPIXELSY #237

Open
akoch-yatta opened this issue Feb 11, 2025 · 3 comments · May be fixed by eclipse-platform/eclipse.platform.swt#1825
Open

Reduce usages of LOGPIXELSX and LOGPIXELSY #237

akoch-yatta opened this issue Feb 11, 2025 · 3 comments · May be fixed by eclipse-platform/eclipse.platform.swt#1825
Assignees
Labels
HiDPI A HiDPI-Related Issue or Feature SWT Issue for SWT
Milestone

Comments

@akoch-yatta
Copy link

akoch-yatta commented Feb 11, 2025

LOGPIXELSX and LOGPIXELSY are fixed at the startup of an application. If the primary monitor zoom is changed after the startup those value will still return the original values and can therefor lead to unwanted behavior with the monitor-specific scaling flag. There are only few usages left, that could lead to issues and we need to investigate whether their usage is fine or should be adapted. These places are:

  • FontDialog::open
  • Device::getFontList
  • ScalingSWTFontRegistry::getOrCreateBaseSystemFontContainer
@akoch-yatta akoch-yatta converted this from a draft issue Feb 11, 2025
@akoch-yatta akoch-yatta added this to the 4.36 M1 milestone Feb 11, 2025
@akoch-yatta akoch-yatta added SWT Issue for SWT HiDPI A HiDPI-Related Issue or Feature labels Feb 11, 2025
@amartya4256 amartya4256 self-assigned this Feb 11, 2025
@amartya4256 amartya4256 moved this from 🔖 Ready: Atomic to 🏗 In Work: Short in HiDPI Feb 11, 2025
@amartya4256
Copy link

@akoch-yatta Device::getFontList usage of LOGPIXELS* seems to be inevitable. The other two were quite straight forward.

@amartya4256
Copy link

In Device::getFontList, instead of int logPixelsY = OS.GetDeviceCaps(hDC, OS.LOGPIXELSY); we can atleast get the primary monitor and use its handle to obtain the new dpi value using OS.GetDpiForMonitor if changed.

@amartya4256
Copy link

amartya4256 commented Feb 12, 2025

I have done my evaluation. Here are my findings:

  • Device::getFontList is used in a deprecated methods FontRegistry::bestData and bestDataArray whica re used to install fonts in preference store.
  • Other than that its used in font tests and GraphicsExample
  • Also used in TextEditor::getFontNames, but only the name of the FontData is utilized.

Except for the first time, its rather safe to leave it there. The only thing I will be concerned about is its usage in FontRegistry::bestData. Since it is completely specific to the application functionality, it can be safe to use Display.getCurrent or getDefault but since it is not null safe, I am a bit iffy. Another point is that even though, it has nothing to do with printer but its the code in Device and we do not want to make something specific to Display in Device Public API.

Conclusion: The best would be to leave it out like that there as it doesn't seem to be harmful. Or as a second choice we can replace int logPixelsY = OS.GetDeviceCaps(hDC, OS.LOGPIXELSY); with:

	int [] dpiX = new int[1];
	int [] dpiY = new int[1];
	OS.GetDpiForMonitor(Display.getCurrent().getPrimaryMonitor().handle, OS.MDT_EFFECTIVE_DPI, dpiX, dpiY);
        int logPixelsY = dpiY[0];

@amartya4256 amartya4256 moved this from 🏗 In Work: Short to 👀 In Review in HiDPI Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HiDPI A HiDPI-Related Issue or Feature SWT Issue for SWT
Projects
Status: 👀 In Review
Development

Successfully merging a pull request may close this issue.

2 participants