Skip to content

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Aug 21, 2025

The FontDialog implementation creates a font handle based on the selection in the dialog to extract information about the font size out of that created font. Due to point/pixel conversion and roundings that depend on the current DPI (affected by the primary monitor zoom), the resulting font size selected differs from what the user has selected. For example, when selecting a 10pt font, the result of the dialog will contain a height of 9.75pt on a 100% monitor and of 10.2pt on a 125% monitor.
At the same time, the dialog manages the logically selected size, i.e., in the given scenario the selected 10pt, which could be used instead.

This change replaces the complex and imprecise font height extraction logic with a simple read of the actual value selected by the user.

Note that this will not have any visible impact, as font sizes are rounded anyway, such that all the imprecise values (such as 9.75pt or 10.2pt) will still lead to a 10pt font.

How to reproduce

The behavior can be reproduced by any FontDialog, such as in Snippet133:

Without the change you will see 9.75pt on a 100% monitor, with the change you will see 10pt on a 100% (and any other) monitor.

The FontDialog implementation creates a font handle based on the
selection in the dialog to extract information about the font size out
of that created font. Due to point/pixel conversion and roundings that
depend on the current DPI (affected by the primary monitor zoom), the
resulting font size selected differs from what the user has selected.
For example, when selecting a 10pt font, the result of the dialog will
contain a height of 9.75pt on a 100% monitor and of 10.2pt on a 125%
monitor.
At the same time, the dialog manages the logically selected size, i.e.,
in the given scenario the selected 10pt, which could be used instead.

This change replaces the complex and imprecise font height extraction
logic with a simple read of the actual value selected by the user.
Copy link
Contributor

Test Results

   546 files  ±0     546 suites  ±0   31m 11s ⏱️ - 5m 1s
 4 426 tests ±0   4 409 ✅ ±0   17 💤 ±0  0 ❌ ±0 
16 750 runs  ±0  16 623 ✅ ±0  127 💤 ±0  0 ❌ ±0 

Results for commit 3359fd7. ± Comparison against base commit 93a484e.

@HeikoKlare HeikoKlare marked this pull request as ready for review August 21, 2025 10:59
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.

Works as described (tested on Windows 10), however I wonder which problem does this PR try to solve?

According to the documentation, when requesting a font from Windows...

For all height comparisons, the font mapper looks for the largest font that does not exceed the requested size.

... which means that if the current code (before this PR) yields 9.75 as the font height then the mapper should return a font of size 9 (which it doesn't seem to do). The current PR seems to hide this error by rounding up to 10.

@HeikoKlare
Copy link
Contributor Author

Works as described (tested on Windows 10), however I wonder which problem does this PR try to solve?

I solves the problem that the FontData created from a FontDialog does not represent the actually selected logical font height but some value extracted out of the font created based on the selection in the dialog. Since that font has a integer-based height in pixels, the conversion back to logical points is imprecise because of rounding, resulting in a value that is slightly off to the actually selected value (such as 9.75 points instead of the selected 10 points). Due to the rounding happening in SWT for creating a font out of this FontData this should not lead to a visual difference, but the FontData simply contains imprecisely data.

According to the documentation, when requesting a font from Windows...

For all height comparisons, the font mapper looks for the largest font that does not exceed the requested size.

... which means that if the current code (before this PR) yields 9.75 as the font height then the mapper should return a font of size 9 (which it doesn't seem to do). The current PR seems to hide this error by rounding up to 10.

That parts refers to the pixel height of a font, not to the logical height. The pixel height at this point is by design an integer value which follows the described pattern (i.e., it's the largest height that not exceed the requested size). This also indicate that it's a "rounded" value, which leads to the underlying issue that converting this back to logical points is not exactly the size as originally selected in the dialog.
Note that this font mapping leads to differently "rounded" font heights depending on the monitor DPI, such that the conversion back to logical points leads to the different results (such as 9.75 or 10.2 points for a selected 10pt font).

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.

Ah, I see it now. You're right, if one selects a size of 10 points in the font dialog then the resulting height in the font data should be exactly 10 and not 9.75 as it was before this PR.

The JavaDoc also says that the height is in points.

/**
* The height of the font data in points
* (Warning: This field is platform dependent)
* <p>
* <b>IMPORTANT:</b> This field is <em>not</em> part of the SWT
* public API. It is marked public only so that it can be shared
* within the packages provided by SWT. It is not available on all
* platforms and should never be accessed from application code.
* </p>
*
* @noreference This field is not intended to be referenced by clients.
*/
public float height;

Now the problem becomes clear, thank you!

@fedejeanne fedejeanne merged commit fdbb769 into eclipse-platform:master Sep 8, 2025
16 of 17 checks passed
@fedejeanne fedejeanne deleted the fontdialog-correct-size branch September 8, 2025 11:59
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.

Imprecise selected font size from FontDialog
2 participants