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

Explicitly set sane line spacing #981

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Explicitly set sane line spacing #981

merged 2 commits into from
Jun 18, 2024

Conversation

in1tiate
Copy link
Member

Fixes absurdly bad line spacing on some user-provided fonts due to Qt's adherence to their values instead of using a sane default. Examples:

Before change:
image

After change:
image

Related to (but not an explicit solution for) #733.

@Salanto
Copy link
Contributor

Salanto commented Jun 17, 2024

Wouldn't this work better if its part of the chatbox configuration? Some fonts may look a bit goofy when the spacing is too close.

@in1tiate
Copy link
Member Author

Wouldn't this work better if its part of the chatbox configuration? Some fonts may look a bit goofy when the spacing is too close.

This solution automatically decides line spacing based on the font's metrics, i.e. it will never decide to space lines too close together, because the font's own metrics will prevent this

Copy link
Contributor

@Salanto Salanto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@@ -3299,6 +3299,14 @@ void Courtroom::initialize_chatbox()
ui_vp_message->move(ui_vp_message->x() + ui_vp_chatbox->x(), ui_vp_message->y() + ui_vp_chatbox->y());
ui_vp_message->setTextInteractionFlags(Qt::NoTextInteraction);

// For some reason, line spacing is done incorrectly unless we set it here.
QTextCursor textCursor = ui_vp_message->textCursor();
QTextBlockFormat *newFormat = new QTextBlockFormat();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, looks like a memory leak.

@Salanto
Copy link
Contributor

Salanto commented Jun 18, 2024

@in1tiate Please re-test if it still works as expected.

It was confirmed by the author to be working correctly still.

@Salanto Salanto merged commit 6c6e46f into master Jun 18, 2024
3 checks passed
@Salanto Salanto deleted the linespacing branch June 18, 2024 17:38
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.

2 participants