-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
PR: Disable font hinting to mitigate tremulous spinning to some extent #203
Conversation
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.
Thanks @kumattau for keep checking into this! I left a comment to add a reference to the issue here. Otherwise this LGTM 👍
@@ -140,7 +140,10 @@ def _paint_icon(self, iconic, painter, rect, mode, state, options): | |||
if animation is not None: | |||
animation.setup(self, painter, rect) | |||
|
|||
painter.setFont(iconic.font(prefix, draw_size)) | |||
font = iconic.font(prefix, draw_size) | |||
if animation is not None: |
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.
Could you add a comment here referencing the issue?
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.
I added the comment "Disable font hinting to mitigate tremulous spinning to some extent".
Is it as expected?
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.
That sounds good 👍 it's just missing the addition of something like See spyder-ide/qtawesome#39
at the end
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.
I added more See spyder-ide/qtawesome#39
at the end.
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.
Thanks @kumattau !
Hi @dalthviz.
This PR is to mitigate #39 to some extent.
As #202 (comment), the method using QRawFont and QPainterPath does not support all fonts supported by QFont.
So, I think the method is optional.
Even if QFont is used, by disabling font hinting, the tremulous spinning seems to be mitigated to some extent.
So, I think it is better that the font hinting is disabled as default when animation.
left: default (enable) hinting, right: disable hinting