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

Font width issue #2566

Closed
DarthGandalf opened this issue Nov 20, 2023 · 11 comments · Fixed by #2573
Closed

Font width issue #2566

DarthGandalf opened this issue Nov 20, 2023 · 11 comments · Fixed by #2573

Comments

@DarthGandalf
Copy link
Member

Gentoo Linux amd64, version 4a51c0e
Qt 5.15.11

When the last time I accidentally deleted my config by trying test version of kvirc (yes, I need better backups) something happened with calculation of font width. See the video: https://youtu.be/tvh8nV_K2vM

This actually makes it hard to select a substring, because when I click it the string visually moves, and I end up selecting a different substring than I intended.

@ctrlaltca
Copy link
Contributor

It seems in #2567 that this font can trick QFontDialog into thinking it's bold while it's not, and this could be also the cause of a miscalculation of the glyph width.
Does it happen with another font, too?

@DarthGandalf
Copy link
Member Author

I'll check.

Before the accident I was using the same font, and I have another kvirc setup with this font (different version though), which also works fine.

@ctrlaltca
Copy link
Contributor

It would be nice to check if the value of fontIrcView is the same in the different configs, and / or how the new version handles the old config

@DarthGandalf
Copy link
Member Author

Changing font didn't fix it, same for changing style

@DarthGandalf
Copy link
Member Author

This happens even on a brand new config, when running kvirc -f -n /tmp/kvircrc, without changing any fonts.

@DarthGandalf
Copy link
Member Author

I ran git bisect, and the regression happened in 2829217

Looks like @pragmaware tried to fix this in 148d8d9 but for me it made the effect even stronger

@ctrlaltca
Copy link
Contributor

I guess 2829217#diff-457dd4d023c2f347f0f23f4d9413c1d71ff648494e2d8cefdfd01bd007c88692L412 it what is causing the issue (similar changes apply to input and topic widgets aswell.

We used some tricks before to ensure correct font width calculation, disabling kerning and the QFont::ForceIntegerMetrics style, that doesn't exist anymore in Qt6.
Qt's advice is to use QFontMetrics to restore the old behavior (see https://doc.qt.io/qt-5/qfont.html#StyleStrategy-enum ).
So, in 2829217 i switched from QFontMetricsF (float) to QFontMetrics (int).
This probably caused some other problems (overlapping texts), so @pragmaware reverted this last change in 148d8d9

Right now I'm not able to reproduce this, but I'll try harder.

@DarthGandalf
Copy link
Member Author

I tried to replace horizontalAdvance with width, but it's still broken. Would need to debug further

@ctrlaltca
Copy link
Contributor

A possible fix is here: master...ctrlaltca:KVIrc:fix_surrogate
At least, it fixes emojis as reported by @Gizmokid2005 yesterday on IRC
Since the inputeditor needs a similar fix, i'll open a PR when that's ready as well

@ctrlaltca
Copy link
Contributor

ctrlaltca commented Nov 29, 2023

Can you please test these two different attempts at fixing the issue?

Attempt1: Attempt at fixing font width size by enable full font hinting, aka forcing glyphs to be aligned to pixels in order to disable subpixel positioning (source: https://www.mail-archive.com/development@qt-project.org/msg43330.html)
This mimics the old "ForceIntegerMetrics" style that got removed in Qt6
ctrlaltca@c9a84af

Attempt2: Attempt at fixing font width problem by making the selection code use floats, too
ctrlaltca@7c56944

@DarthGandalf
Copy link
Member Author

Attempt1 fixes it, Attempt2 does not.

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 a pull request may close this issue.

2 participants