MuseScore icon indicating copy to clipboard operation
MuseScore copied to clipboard

Use a different workaround for artificially emboldened text

Open AntonioBL opened this issue 2 years ago • 27 comments

Resolves: #14435 Resolves: #12714 (only the rendering part, not the "strange behavior" of bold switch for the title)

The text drawing routines under Windows and Linux, that are using Freetype font engine, were patched with a rough hack to prevent a QT bug due to the artificial emboldening of a font, while at the same time preserve kerning of the text. It was not working for bold+underline, because the "trick" of rescaling the font was rescaling the size of the underline and this could not be overwritten. By setting this env variable, QT does not used cached font glyphs and the bug is gone. I don't know if this change could introduce performance issues (since glyphs are not loaded from the cache).

Most probably some of the vtests are changed by this PR.

  • [x] I signed the CLA
  • [x] The title of the PR describes the problem it addresses
  • [x] Each commit's message describes its purpose and effects, and references the issue it resolves
  • [ ] If changes are extensive, there is a sequence of easily reviewable commits
  • [x] The code in the PR follows the coding rules
  • [x] There are no unnecessary changes
  • [x] The code compiles and runs on my machine, preferably after each commit individually
  • [ ] I created a unit test or vtest to verify the changes I made (if applicable)

AntonioBL avatar Jun 15 '23 12:06 AntonioBL

@AntonioBL Tested on Windows10

#14435 looks good

But now I can't make text bold at least in some way on Windows comparing with master (when testing #12717)

https://github.com/musescore/MuseScore/assets/90187801/b3159905-50de-4cfd-907c-e0da17789295

DmitryArefiev avatar Jun 16 '23 14:06 DmitryArefiev

Actually, the problem comes from Qt itself. Since Qt 5.15.1 there is a size limit over which the fonts without a bold variant (like MuseJazzText) are not synthetically emboldened. See this Qt commit: https://github.com/qt/qtbase/commit/f0637fb2f746d7c3dc0cc9454514642541c8a7b7 and related Qt bug: https://bugreports.qt.io/browse/QTBUG-84570 if the bold text is drawn with fontsize larger than 64 points, then it is not artificially emboldened during the painting routine.

Now, in the workaround that was used, the size of the drawn text actually depended on the zoom level: internally, when changing the zoom, the painter coordinates (diagonal element of the transformation matrix) are modified, but the workaround drew the glyph for painter matrix with 1.0 diagonal elements to avoid the ugly cached glyphs; the text fontsize was changed accordingly to appear at the right (apparent) size (the workaround was a little more comples, but for what concerns the drawing of the single glyphs, that was the behavior). That's why by changing the level of zoom we can see the text emboldened (when zoomed out, so that the drawn fontsize is small) or not (when zoomed in, so that the drawn fontsize is large) in current master.

With this new workaround there is no need for glyph scaling during drawing, so the behavior should now only depend on the font size: when the size is below a certain value (I found it to be roughly 12.9, that corresponds to the "64" limit, by probably considering the factors due to DPI etc.) the bold text appears artificially emboldened, while it is not emboldened for size >= 12.9.

Since Qt 5.15.9 there is an additional environment variable that overrides this limit: QT_NO_SYNTHESIZED_BOLD_LIMIT See the related commit: https://github.com/qt/qtbase/commit/7d9329d8c4f1b04917224ebdb7b8317a397c984a So if and when there will be a switch to Qt 5.15.9 (and higher), we could set this env variable as well and have an artificially emboldened for every size dimension.

Ciao, ABL

P.S: we could think of compiling Qt by ourselves, maybe inside Github actions for the different operating systems so that Qt updates should require little effort. Maybe even including KDE patch collection https://community.kde.org/Qt5PatchCollection

By the way, @cbjeukendrup already introduced Qt 5.15.9 for Mac. I'd like (a lot) to work on this, but I cannot guarantee a short deadline, since I have very little free time at the moment. :disappointed:

AntonioBL avatar Jun 19 '23 15:06 AntonioBL

@AntonioBL OK! I've tested PDF and SVG output on Windows and didn't see a difference between PR's build and master build, probable we can merge it.

@cbjeukendrup Do I need to test something else before merging?

DmitryArefiev avatar Jun 27 '23 15:06 DmitryArefiev

@AntonioBL OK! I've tested PDF and SVG output on Windows and didn't see a difference between PR's build and master build, probable we can merge it.

Most probably because PDF and SVG export did not use the workaround (at least, in 3.x it was so), because there was no scaling (and export DPI, linked to scaling, set exactly equal to the internal DPI)

AntonioBL avatar Jun 27 '23 15:06 AntonioBL

@DmitryArefiev If the performance is okay when scrolling around in a large score with many MuseJazz bold texts, then I think this can be merged!

cbjeukendrup avatar Jun 27 '23 17:06 cbjeukendrup

After a long battle against QtWebEngine, I think I managed to make Qt 5.15.10 (with KDE patches) compile for Linux in a Github Action. I had to split compilation into three different jobs 😅 and make extensive use of the cache. I compiled it within an Ubuntu Bionic Docker image, so that there should be no problems of possibly required newer libraries during runtime. Here is the amd64 artifact: https://github.com/AntonioBL/Qt5Binaries/suites/14001144253/artifacts/781475003 I am not yet sure that all the needed modules were compiled, and I have doubts about file permissions: Docker creates binaries belonging to root user, but I tried to add rwx permissions to all. Now I will try for an i386 build. I shudder to think about the compilation under Windows...

AntonioBL avatar Jul 02 '23 11:07 AntonioBL

@AntonioBL One test is failed. Can you rebase please?

DmitryArefiev avatar Jul 03 '23 08:07 DmitryArefiev

@DmitryArefiev : done 😉 But I think the test will still fail since the failure is vtests: text redering slightly changed because the workaround was apparently applied in most of text redering.

AntonioBL avatar Jul 03 '23 09:07 AntonioBL

Seems it isn't just text, and not just MuseJazz, but also many other fonts' musical glyphs, like accidentals, but most/all other gylphs too, clefs, noteheads, etc. All very small changes.

Seems every glyph became a tiny bit thinner.

Jojo-Schmitz avatar Jul 03 '23 10:07 Jojo-Schmitz

Seems it isn't just text, and not just MuseJazz, but also many other fonts' musical glyphs, like accidentals, but most/all other gylphs too, clefs, noteheads, etc. All very small changes.

Seems every glyph became a tiny bit thinner.

Yeah, I see a little difference too.

@oktophonie Can you review vests please?

DmitryArefiev avatar Jul 03 '23 10:07 DmitryArefiev

The only question for me is whether the 'before' or 'after' versions are more accurate, and I don't know how best to determine that.

its-not-nice avatar Jul 03 '23 11:07 its-not-nice

As the differences are a) very small and b) everting seems a tiny bit smaller, so won't result in less measures to fit a system, IMHO this is OK

Jojo-Schmitz avatar Jul 03 '23 12:07 Jojo-Schmitz

Of course not, because this will only affect the drawing, and not the actual layout computations.

cbjeukendrup avatar Jul 03 '23 12:07 cbjeukendrup

Ah, I see, that 'skinnyness' doesn't seem to affect the overall width, doesn't accumulate due to the distance between elements

Jojo-Schmitz avatar Jul 03 '23 12:07 Jojo-Schmitz

Seems it isn't just text, and not just MuseJazz, but also many other fonts' musical glyphs, like accidentals, but most/all other gylphs too, clefs, noteheads, etc. All very small changes.

Seems every glyph became a tiny bit thinner.

@AntonioBL The changes which vtests found are only on display? PDF output, physical print output should not have those differences comparing with master?

DmitryArefiev avatar Jul 03 '23 14:07 DmitryArefiev

They are visible on PNG export, no reason to assume PDF export or print would be different?

Jojo-Schmitz avatar Jul 03 '23 14:07 Jojo-Schmitz

Oh yeah, you're right

DmitryArefiev avatar Jul 03 '23 15:07 DmitryArefiev

In principle, the function drawTextWorkaround was used only in drawing Harmony texts (i.e. chord texts) and Textfragment texts (i.e. other texts, such as Staff Text, Tempo Text, etc.). Maybe musical symbols are drawn as Textfragments, so maybe that's the reason why they very slightly changed. In MuseScore 3.x PDF drawing was using a different drawing function than the one for the displayed score; if evrything was unified, I would expect the pdf output to slightly change as well. The workaround was originally thought to be used only for particular cases, while all other cases relied on Qt text drawing routines, but I think that in the rewriting for MuseScore 4 the original workaround was used even for normal text drawing. Qt routines are also the one used by MuseScore in MacOS. [By the way, if you take pdfs of the same score exported by Linux, Windows and Mac they will probably look slightly different, even without this PR]

AntonioBL avatar Jul 03 '23 15:07 AntonioBL

Since the differences seem too tiny to be spotted in real life, I think it's safe to merge this to master (but let's maybe not merge it to 4.1.0 yet). And the problem that bold does not work can be fixed later by updating Qt. Does anyone disagree?

cbjeukendrup avatar Jul 04 '23 16:07 cbjeukendrup

I'd still to prefer to know whether the before or after is more accurate.

its-not-nice avatar Jul 04 '23 17:07 its-not-nice

After. . . . . . ;-)

Jojo-Schmitz avatar Jul 04 '23 19:07 Jojo-Schmitz

And after a second long battle against Qt WebEngine, I managed to obtain a universal build of Qt 5.15.10 (KDE patched)+ Qt WebEngine for MacOS. Here is the artifact: https://github.com/AntonioBL/Qt5Binaries/suites/14163460885/artifacts/794305575 @cbjeukendrup : I shamelessly based the MacOS compilation on your branch https://github.com/cbjeukendrup/musescore_build_qt but I switched to the KDE-patched version because there was a problem during the compilation (and I found that I had to revert one of those patches for a successful compilation, since it was a workaround for an XCode bug already fixed upstream).

AntonioBL avatar Jul 10 '23 12:07 AntonioBL

Great work, @AntonioBL! (And fortunately you found my musescore_build_qt repository, I should really have told you about that...)

I'm not very familiar with the KDE-patched version of Qt. What kind of patches are merged into this? Only very basic build fixes and obviously-correct bug fixes, or also disputable fixes and whole new features? And intuitively, I associate KDE more with Linux than with for example macOS, but are these patches also regularly tested on macOS? And, since we use the 5.15 branch, which version is this? Is it the latest available open source version, i.e. 5.15.10? Or is it Qt 5.15.10 with Qt WebEngine 5.15.14?

cbjeukendrup avatar Jul 10 '23 13:07 cbjeukendrup

@cbjeukendrup : From here they say that they are commits cherry-picked from upstream Qt (probably > 5.15) that patches security issues, or crashes or "functional defects", and relevant towards Open Source products and their viability. So I think they should be mainly fixes already present in Qt 6 of bug found also in Qt 5.15.x, that the Qt group does not intend to backport (yet). I think some of the patches were included in the 5.15 pacth releases (5.15.3, 5.15.4, etc...)

I took the code from the git repository, which at the moment should be 5.15.10 for Qt with 5.15.14 for Qt WebEngine: https://invent.kde.org/qt/qt/qt5/-/tree/kde/5.15?ref_type=heads Here is the list of the patches applied: https://invent.kde.org/qt/qt/qtbase/-/compare/v5.15.10-lts-lgpl...05406c3f5f516d3148254c8294e8883c28a2c95a?from_project_id=1216&straight=false To me it seems that they are including patches not only related to Linux, but also other operating systems, for example patches for MinGW, and MacOS (such as the patch for compilation with new XCode, that I had to revert to make it compile with the even newer XCode). But I think that the patches related to non-Linux OSs are probably not their priority.

AntonioBL avatar Jul 10 '23 13:07 AntonioBL

@AntonioBL Thanks, that sounds good! Let's go ahead with the patched version then. Next step is getting a build for Windows... do you want to look into this, and do you have time for it, or shall I give it a try?

cbjeukendrup avatar Jul 10 '23 22:07 cbjeukendrup

@AntonioBL Thanks, that sounds good! Let's go ahead with the patched version then. Next step is getting a build for Windows... do you want to look into this, and do you have time for it, or shall I give it a try?

If you want, I will try for a Windows build, but I cannot state at the moment how much time it will require. 😅 I was thinking about using tuxliketimeout to split Qt WebEngine compilation, if needed, since the timeout function under Windows is not the same as under Linux.

AntonioBL avatar Jul 11 '23 11:07 AntonioBL

And here is the Windows build: https://github.com/AntonioBL/Qt5Binaries/suites/14488802861/artifacts/819378767 Unfortunately, QtWebEngine requires at least Windows SDK 10.0.19041.0, so I don't think it will run in Windows 8 and 7 (MuseScore 3.x, based on Qt 5.9, could run even under Windows 7).

AntonioBL avatar Jul 24 '23 13:07 AntonioBL