PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons #202

kumattau · 2022-01-26T17:46:27Z

Maybe #39 can be fixed using QRawFont and draw PainterPath of Glyph.

Compared to QFont, QRawFont is available without registering to font database,
So this approach has

Pros:

no tremulous spinning
no family name collision (font patch is not required)

Cons:

cannot support color-font (PainterPath of Glyph does not have color)
cannot support font() API (this requires QFont registered to font database)
I tests it on ubuntu only. I cannot check this approach works on other platform or not

If qtawesome switches QFont to QRawFont, font patch in the update_font script ( #194 ) may not be needed.

How do you think, @dalthviz ?

Note: current code is example, so it needs refactoring, optimization, etc.

Edit:

Fixes #39

dalthviz · 2022-01-26T18:45:36Z

Hi @kumattau thanks for checking into this! Testing this locally seems like this also works for Windows 👍

Regarding the pros and cons maybe we could use QRawFont only when needed as you are doing here but still registering the font to still have access to the font API if needed?

Just in case what do you think @ccordoba12 ?

kumattau · 2022-01-28T15:54:11Z

Thank you for testing @dalthviz

On windows, "Stack, offset" bled quite a bit. this is because QPainterPath does not have hinting.
So, I changed it to use QPainterPath only for animations to keep rendering quality.

On my ubuntu, example.py is here.
Could you compare "Stack, offset" and "Stack, offset, draw path" on Windows ?

The drawing method can be selected by "draw" option.

path: do painter.drawPath()
text: do painter.drawText()
auto: do painter.drawPath when animation, otherwise do painter.drawText() (default)

kumattau · 2022-01-28T16:18:46Z

@dalthviz, I finished the update. This PR is ready for review.

dalthviz · 2022-01-28T18:50:22Z

Checking the example.py on Windows this is what I'm getting:

dalthviz

Thanks @kumattau for working on this! I left some suggestions regarding the comments in the code and also maybe we need to also update the docs (at least the example code in this PR) but otherwise this LGTM 👍

example.py

qtawesome/iconic_font.py

kumattau · 2022-01-31T13:39:25Z

Thank you for review @dalthviz, I updated comment.
I've come up with a better way to do it and will experiment with it. Please wait a little longer.

kumattau · 2022-02-01T17:37:11Z

Hi, @dalthviz

I've come up with a better way to do it and will experiment with it.

I try to drawImage a glyph of QRawfont.
On my ubuntu, the drawing quality is same as original drawText with QFont.
Could you try example.py on Windows and feedback me ?

If the drawImage is okey, qtawesome does not require for QFont except for font() API.
I think I want not to patch fonts if possible, so, it is the first step of the use of patch-free font.

dalthviz · 2022-02-01T18:03:17Z

Hi again @kumattau, this is what I'm seeing on Windows with the latest changes:

So, indeed the icon drawing locks cleaner 👍

However seems like the new code is not compatible with Qt <5.15. The tests are failling here since it our CI uses Qt 5.9.7 and locally I tried first testing with an env that had Qt 5.12.2 and the same error appeared. Could be possible to have a fallback in case QImage.setAlphaChannel doesn't exists in the Qt/PyQt/PySide version installed?

kumattau · 2022-02-01T18:30:51Z

@dalthviz

Could be possible to have a fallback in case QImage.setAlphaChannel doesn't exists in the Qt/PyQt/PySide version installed?

Yes. I added fallback code and pushed it.
I will try to refactor the code and add three draw options (path, image, text).

kumattau · 2022-02-07T16:01:00Z

Hi, @dalthviz

cannot support color-font (PainterPath of Glyph does not have color)

I found another issue.
QRawFont requires that the font has glyph.
So, for example, NotoColorEmoji.ttf, which does not have glyphs, cannot be rendered using QRawFont.

fonttools/fonttools#949 (comment)

qtawesome cannot switch the drawing method from drawPath() to the original drawText() if the font does not have glyphs.
Because QRawFont have bool supportsCharacter(char) method to check if the font has a glyph that corresponds to the given character, but this always returns True on ubuntu.

And the following error message output to the console if a glyph is missing, but this message cannot be stopped.

load glyph failed err=24

Should qtawesome support a font which does not have glyphs ?

dalthviz · 2022-02-07T16:33:12Z

Hi again @kumattau !

Should qtawesome support a font which does not have glyphs ?

I would say yes. Probably we need to find a reliable way to detect if the font does not have the needed glyphs, show some sort of warning letting users know that we were unable to use the specified config to draw the icon and then fallback to use the drawText/``text` option.

Maybe there are other ways to check for the character support more reliable than QRawFont.supportsCharacter ?

kumattau · 2022-02-08T13:17:30Z

Hi @dalthviz

I would say yes.

I got it.

I would say yes. Probably we need to find a reliable way to detect if the font does not have the needed glyphs, show some sort of warning letting users know that we were unable to use the specified config to draw the icon and then fallback to use the drawText/``text` option.

I think fallback is possible.
If glyph is missing, the PainterPath is empty, so I can implement the fallback to drawText when PainterPath is emtpy.
The biggest issue at the moment is that I can't stop the following error messages.

load glyph failed err=24

It is quite annoying because it is always output when glyph is missing.

I will do some more research on this matter.

dalthviz · 2022-10-11T17:39:23Z

Hi again @kumattau , hope everything is going well! Since we will try to release v1.2.0 soon, just to know if we can make this part of the release a couple of questions: Did you find any other info that could help with the missing elements here? and also, would you like to finish this or is okay for me or someone else to try finish it? Let us know!

kumattau · 2022-10-12T02:30:31Z

@dalthviz Thank you for follow-up and I'm very sorry not to response.

I will do some more research on this matter.

The investigation has not progressed but I will reboot this PR soon (in this week or next week).
If my schedule is not suite the release, I'm no problem for you to try finish it at all.

dalthviz · 2022-10-12T16:45:58Z

Thanks for the prompt response @kumattau ! If you still want to finish this at some point then no problem, happy to hear that :)

It would be nice to merge this for the next release v1.2.0 (which will be done probably mid-next week) but do not worry about the time, just knowing that this could be finished in the future and you are willing to keep checking on it are really good news 👍

Co-authored-by: Daniel Althviz Moré <[email protected]>

kumattau · 2022-10-13T21:31:13Z

@dalthviz

I rebased master and fixed "load glyph failed err=24" problem by fall backing draw=text
The draw option accepts text/path/glyphrun/image. If not specified, draw=path is used for non-animation and draw=text is used for animation.
example.py accepts global_default options (key=val format) for test.

In my linux, it is better that draw=image is used for non-animation and draw=text is used for animation.
Could you please try it in windows ?

* Fix QRawFont to be thread-local * Add public iconic.rawfont() same as iconic.font() * Use setCompositionMode instead of setAlphaChannel which is not supported on PyQt 5.9 * Add glyphrun to draw option * Add draw option in global_defaults * Add Fallback to text drawing if path/glyphrun/image drawing cannot be used * Refectoring CharIconPainter._paint_icon()

kumattau · 2022-10-14T03:49:40Z

Looking to spin, I still think draw=path would be better when animation.

dalthviz · 2022-10-14T16:30:52Z

Thanks for the quick update @kumattau ! Doing some testing on Windows:

Default options (no args) - `python example.py`

`draw=image` option - `python example.py draw=image`

`draw=path` option - `python example.py draw=path`

`draw=text` option - `python example.py draw=image`

Checking then seems like the draw=path should be the default when animation is requested and draw=image when not

qtawesome/iconic_font.py

dalthviz · 2022-10-14T16:37:17Z

Besides the suggestion above I think the only missing element is updating the example.py gif and documentation (adding the new draw option and also the possiblity to pass options to the example.py script) but I can take care of that in a follow-up PR before doing the release 👍

And again, thanks @kumattau for taking the time to update this!

qtawesome/iconic_font.py

Co-authored-by: Daniel Althviz Moré <[email protected]>

kumattau · 2022-10-15T13:01:06Z

@dalthviz

How about draw=glyphrun on windows ?

On linux, The quality of draw=glyphrun seems to be same as draw=text.

glyphrun can use rawfont (in other words, does not requires the installation of font) but cannot render emoji (without glyph).

dalthviz · 2022-10-15T21:47:27Z

Checking draw=glyphrun on Windows I got this:

Looking more in detail the different draw options I would say that glyphrun looks quite similar to text and also as you said for some fonts using image doesn't look quite good, so lets leave the default draw option to be text for non-animated icons 👍

Thanks again @kumattau for the work here and checking all the draw options! I think this is ready to be merged then, right? Or maybe do you want to check something else on Windows? If is ready to be merged just mark the PR as Ready for review and I will merge it 👍

kumattau · 2022-10-16T09:52:31Z

@dalthviz

My work has been finished.
The commits keep not to be squashed.
If you want to simplify the PR, please squash and merge it.

dalthviz

Thanks for all the work done here @kumattau !

dalthviz assigned kumattau Jan 26, 2022

kumattau force-pushed the qrawfont branch 3 times, most recently from 721160a to 37d6023 Compare January 28, 2022 15:36

kumattau marked this pull request as ready for review January 28, 2022 15:54

kumattau changed the title ~~[WIP] Use QRawFont and draw PainterPath of Glyph~~ PR: Use QRawFont and draw PainterPath of Glyph when animation Jan 28, 2022

kumattau force-pushed the qrawfont branch 2 times, most recently from ad7aac7 to dcab164 Compare January 28, 2022 16:06

dalthviz added this to the v1.2.0 milestone Jan 28, 2022

dalthviz added the type:Bug label Jan 28, 2022

dalthviz linked an issue Jan 28, 2022 that may be closed by this pull request

Tremulous spinning icons #39

Closed

dalthviz reviewed Jan 28, 2022

View reviewed changes

example.py Outdated Show resolved Hide resolved

qtawesome/iconic_font.py Outdated Show resolved Hide resolved

qtawesome/iconic_font.py Outdated Show resolved Hide resolved

kumattau marked this pull request as draft January 31, 2022 13:37

kumattau mentioned this pull request Feb 28, 2022

PR: Disable font hinting to mitigate tremulous spinning to some extent #203

Merged

dalthviz modified the milestones: v1.2.0, v1.2.1 Oct 7, 2022

dalthviz changed the title ~~PR: Use QRawFont and draw PainterPath of Glyph when animation~~ PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons Oct 12, 2022

kumattau and others added 6 commits October 13, 2022 08:51

Use QRawFont and draw QPainterPath of glyph when animation

2d66328

Add an example for draw option

a198a99

Apply suggestions from code review

703646b

Co-authored-by: Daniel Althviz Moré <[email protected]>

Use a drawPath based on draw_size to match the drawText behavior

7eb4ac5

Test alphaMapForGlyph and drawImage

c15d07d

Fix setAlphaChannel AttributeError on PyQt 5.9

ca700f7

kumattau force-pushed the qrawfont branch 2 times, most recently from 5906d03 to 378d009 Compare October 13, 2022 21:22

kumattau added 2 commits October 14, 2022 12:25

Passing arguments to global_defaults for test

81fc7f4

kumattau force-pushed the qrawfont branch from 378d009 to 0dc4226 Compare October 14, 2022 03:26

dalthviz modified the milestones: v1.2.1, v1.2.0 Oct 14, 2022

dalthviz reviewed Oct 14, 2022

View reviewed changes

qtawesome/iconic_font.py Show resolved Hide resolved

dalthviz reviewed Oct 14, 2022

View reviewed changes

qtawesome/iconic_font.py Outdated Show resolved Hide resolved

Update qtawesome/iconic_font.py

e23c132

Co-authored-by: Daniel Althviz Moré <[email protected]>

kumattau marked this pull request as ready for review October 16, 2022 09:48

dalthviz approved these changes Oct 17, 2022

View reviewed changes

dalthviz merged commit f6b7db8 into spyder-ide:master Oct 17, 2022

dalthviz mentioned this pull request Oct 17, 2022

Release QtAwesome 1.2.0 #208

Closed

kumattau deleted the qrawfont branch October 17, 2022 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons #202

PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons #202

kumattau commented Jan 26, 2022 •

edited by dalthviz

Loading

dalthviz commented Jan 26, 2022

kumattau commented Jan 28, 2022

kumattau commented Jan 28, 2022

dalthviz commented Jan 28, 2022

dalthviz left a comment

kumattau commented Jan 31, 2022

kumattau commented Feb 1, 2022

dalthviz commented Feb 1, 2022

kumattau commented Feb 1, 2022

kumattau commented Feb 7, 2022 •

edited

Loading

dalthviz commented Feb 7, 2022

kumattau commented Feb 8, 2022

dalthviz commented Oct 11, 2022

kumattau commented Oct 12, 2022

dalthviz commented Oct 12, 2022

kumattau commented Oct 13, 2022

kumattau commented Oct 14, 2022

dalthviz commented Oct 14, 2022

dalthviz commented Oct 14, 2022 •

edited

Loading

kumattau commented Oct 15, 2022

dalthviz commented Oct 15, 2022

kumattau commented Oct 16, 2022

dalthviz left a comment

PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons #202

PR: Use QRawFont and draw PainterPath of Glyph for animation to prevent tremulous spinning icons #202

Conversation

kumattau commented Jan 26, 2022 • edited by dalthviz Loading

dalthviz commented Jan 26, 2022

kumattau commented Jan 28, 2022

kumattau commented Jan 28, 2022

dalthviz commented Jan 28, 2022

dalthviz left a comment

Choose a reason for hiding this comment

kumattau commented Jan 31, 2022

kumattau commented Feb 1, 2022

dalthviz commented Feb 1, 2022

kumattau commented Feb 1, 2022

kumattau commented Feb 7, 2022 • edited Loading

dalthviz commented Feb 7, 2022

kumattau commented Feb 8, 2022

dalthviz commented Oct 11, 2022

kumattau commented Oct 12, 2022

dalthviz commented Oct 12, 2022

kumattau commented Oct 13, 2022

kumattau commented Oct 14, 2022

dalthviz commented Oct 14, 2022

Default options (no args) - python example.py

draw=image option - python example.py draw=image

draw=path option - python example.py draw=path

draw=text option - python example.py draw=image

dalthviz commented Oct 14, 2022 • edited Loading

kumattau commented Oct 15, 2022

dalthviz commented Oct 15, 2022

kumattau commented Oct 16, 2022

dalthviz left a comment

Choose a reason for hiding this comment

kumattau commented Jan 26, 2022 •

edited by dalthviz

Loading

kumattau commented Feb 7, 2022 •

edited

Loading

Default options (no args) - `python example.py`

`draw=image` option - `python example.py draw=image`

`draw=path` option - `python example.py draw=path`

`draw=text` option - `python example.py draw=image`

dalthviz commented Oct 14, 2022 •

edited

Loading