Use fontmetrics for header height #794
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alerque
approved these changes
Oct 8, 2021
Could you share screenshots? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request eliminates hard coded pixel sizes for the header text.
I've still got no clue why clipping occurs if headerHeight is not at least three pixels larger than the pixel height of the font metrics according to Qt5, as I can only account for one pixel, but the glyphCell widgets look good with this change applied on Windows and on Linux, and it gets rid of the hard coded size in pixels. I experimented by drawing a yellow rectangle underneath the text string in drawCellHeaderText and it does seem the clipping comes from painter.drawText (but I'm no Qt5 guru and Python is not my mother tongue).
The inconsistency between my laptop and my desktop disappeared after I installed B&H Luxi Sans. I do not have a better suggestion for a UI font off the top off my head (well, I do, but I'm biased and DINish doesn't come with any Linux distros either :-)
The Qt5 docs suggest but do not spell out that Qt.AlignCenter | Qt.AlignBottom should be Qt.AlignHCenter | Qt.AlignBottom; I'd suggest to make that change for readability.
I verified that this code seems to do the right thing on Ubuntu 20.04LTS and on Windows 10 21H1, but I could not test on MacOS.
Tweaking GlyphCellHeaderHeight from GlyphCellFactoryDrawingController is of course a gross hack, but I could not find a better place to do it (and the usage of GlyphCellHeaderHeight in glyphCellFactory.py and glyphView.py is hackish in its own right, as it's neither a class nor an instance variable).