-
Notifications
You must be signed in to change notification settings - Fork 63
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
GPII-1716: Windows font-size #506
base: master
Are you sure you want to change the base?
Conversation
CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details. |
ok to test |
"type": "struct", | ||
"name": "NONCLIENTMETRICS" | ||
}, | ||
"fWinIni": "SPIF_UPDATEINIFILE|SPIF_SENDCHANGE", // It won't update if these flags aren't set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank God we now have comments!
testData/solutions/win32.json5
Outdated
"name": "NONCLIENTMETRICS" | ||
}, | ||
"fWinIni": "SPIF_UPDATEINIFILE|SPIF_SENDCHANGE", // It won't update if these flags aren't set. | ||
"verifySettings": false // The next GET will be different to the last SET. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why will the next GET be different to the last SET?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated comment..
// The next GET will be different to the last SET, because the value of lfHeight sets the character // height (negated), but receives it as the cell height.
LOGFONT: https://msdn.microsoft.com/library/dd145037
Not only is the value different, it also uses the sign to specify if you're talking about the character height (-) or cell height (+).
More info: https://support.microsoft.com/en-us/help/32667/info-font-metrics-and-the-use-of-negative-lfheight
So why not just specify the cell height? The preference is already the character height, and calculating the cell height requires knowing the DPI, which is a run-time value.
I think the LOGFONT structure is from when raster fonts where common.
CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details. |
Is there an acceptance test profile that exercises this solution? |
|
That one is being tracked as https://issues.gpii.net/browse/GPII-2456 |
CI job passed. |
# Conflicts: # testData/deviceReporter/installedSolutions.json
CI job passed. |
CI job passed: https://ci.gpii.net/job/universal-tests/473/ |
CI job passed: https://ci.gpii.net/job/universal-tests/526/ |
bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @stegru,
first, I'd say that we need a few things before merging this into master:
- We need an acceptance test that exercises this new solution
- We need to add a description of this new solution in https://github.com/GPII/universal/tree/master/testData/solutions/solutionsDescription
- We need at least one np set that uses this solution for us to test it
testData/solutions/win32.json5
Outdated
}, | ||
// The pixel heights will grow, but will not shrink back, so set the value to something so it will | ||
// be restored. | ||
"CaptionHeight": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all these last transformations don't have mappings to common terms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values set the height of the element - they're listed here so they are set back to the same value.
Without them, a larger font will make the height grow, but restoring a smaller font doesn't make the height shrink back down.
|
CI job failed: https://ci.gpii.net/job/universal-tests/809/ |
CI job passed: https://ci.gpii.net/job/universal-tests/814/ |
# Conflicts: # testData/deviceReporter/acceptanceTests/win7_builtIn.json
CI job passed: https://ci.gpii.net/job/universal-tests/837/ |
CI job failed: https://ci.gpii.net/job/universal-tests/907/ |
CI job failed: https://ci.gpii.net/job/universal-tests/913/ |
CI job failed: https://ci.gpii.net/job/universal-tests/914/ |
CI job passed: https://ci.gpii.net/job/universal-tests/916/ |
Windows font-size support (via SPI, not the nicer DPI method).
FYI @javihernandez: If this is applied, it might change the way HST appears, because "sammy" increases the fontSize.
Windows portion: GPII/windows#115