Bug 264274 - net-im/telegram-desktop: add option to not embed fonts (allows using personal font preferences)
Summary: net-im/telegram-desktop: add option to not embed fonts (allows using personal...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Sergey A. Osokin
URL:
Keywords: feature
Depends on:
Blocks:
 
Reported: 2022-05-27 06:10 UTC by Volodymyr Kostyrko
Modified: 2023-04-23 02:54 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (osa)
arcade: maintainer-feedback?


Attachments
add NO_FONTS option (1.19 KB, patch)
2022-05-27 06:10 UTC, Volodymyr Kostyrko
no flags Details | Diff
rename option, clarify description (1.27 KB, patch)
2022-06-03 07:03 UTC, Volodymyr Kostyrko
no flags Details | Diff
patch to skip embedding fonts, fix for latest version (1.24 KB, patch)
2023-01-09 12:09 UTC, Volodymyr Kostyrko
no flags Details | Diff
Option for system fonts (1.35 KB, patch)
2023-03-09 11:49 UTC, Max Brazhnikov
no flags Details | Diff
fixed SYSTEM_FONTS patch (1.50 KB, patch)
2023-03-22 12:01 UTC, Volodymyr Kostyrko
arcade: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Volodymyr Kostyrko 2022-05-27 06:10:44 UTC
Created attachment 234251 [details]
add NO_FONTS option

Hello.

Idea was found in arch repo - just truncate fonts and don't use DemiBold.
Comment 1 Max Brazhnikov freebsd_committer freebsd_triage 2022-05-31 22:45:26 UTC
(In reply to Volodymyr Kostyrko from comment #0)
Please, don't name option with "NO" prefix. I'd suggest EMBEDDED_FONTS or BUNDLED_FONTS as an alternative.
Comment 2 Volodymyr Kostyrko 2022-06-03 07:03:09 UTC
Created attachment 234409 [details]
rename option, clarify description
Comment 3 Sergey A. Osokin freebsd_committer freebsd_triage 2022-11-21 03:44:18 UTC
Hi,

net-im/telegram-desktop has been updated to version 4.2.4.

I've just taken a look on patch, so I couldn't find how to apply the patch.

Could you please review and update the patch.

Thank you.
Comment 4 Volodymyr Kostyrko 2022-12-01 06:11:36 UTC
(In reply to Sergey A. Osokin from comment #3)

Just checked and it applied cleanly for me:

cd /usr/ports/net-im/telegram-desktop
fetch -o - 'https://bz-attachments.freebsd.org/attachment.cgi?id=234409' | patch -p3
Comment 5 Sergey A. Osokin freebsd_committer freebsd_triage 2022-12-31 15:56:27 UTC
(In reply to Volodymyr Kostyrko from comment #4)

Thanks for the update.


net-im/telegram-desktop has been updated to version 4.5.0.
Could you please update the patch as well, thank you.
Comment 6 Volodymyr Kostyrko 2023-01-09 12:09:29 UTC
Created attachment 239360 [details]
patch to skip embedding fonts, fix for latest version

Fixed regarding WEBKIT removal.
Comment 7 Sergey A. Osokin freebsd_committer freebsd_triage 2023-01-11 23:32:31 UTC
(In reply to Volodymyr Kostyrko from comment #6)

I've forgotten to add that this new option can be optional and disabled by default, hope it's ok.
Comment 8 Volodymyr Kostyrko 2023-03-03 07:47:09 UTC
It should not be disabled by default, default preference is to follow official build. By disabling this option you can prevent build from bundling fonts in to use your own preferred ones.

This was done differently in first patch, but changed according to comment #1.
Comment 9 Sergey A. Osokin freebsd_committer freebsd_triage 2023-03-03 20:23:40 UTC
(In reply to Volodymyr Kostyrko from comment #8)

Hi Volodymyr,

thanks for the update.

I've contacted the Telegram Desktop Development team for the patch review.
It's been recommended to keep default behavior as is, otherwise it may cause an issue with default fonts for the program.

Please raise a corresponding issue on the project page in GitHub, hope your patch will be accepted there.

Thank you.
Comment 10 Max Brazhnikov freebsd_committer freebsd_triage 2023-03-07 10:04:56 UTC
I believe this patch can be added: it does not change default behavior of the package, it's not intrusive, and it allows users to use system fonts in telegram for better visual integration with their desktop.
Comment 11 Sergey A. Osokin freebsd_committer freebsd_triage 2023-03-07 14:17:24 UTC
(In reply to Max Brazhnikov from comment #10)

I'm definitely ready to add the feature as an optional, but Volodymyr wants to see that be default, and the vendor is disagree with that.
Comment 12 Gleb Smirnoff freebsd_committer freebsd_triage 2023-03-07 17:42:59 UTC
I'd rather work with upstream to get the patch there or abandon the patch. IMHO, the ports system shouldn't turn into a fork of upstream software.
Comment 13 Max Brazhnikov freebsd_committer freebsd_triage 2023-03-09 11:49:37 UTC
Created attachment 240694 [details]
Option for system fonts

Turns out upstream already provides build option for using system fonts and I set it unconditionally in [1] because default fonts looked ugly on my monitor. I've missed when it was reverted, apparently Telegram ships better fonts now.

Let's make it optional. Volodymyr, could you test the attached patch?

[1] https://svnweb.freebsd.org/ports?view=revision&revision=552150
Comment 14 Max Brazhnikov freebsd_committer freebsd_triage 2023-03-09 11:51:19 UTC
Reopen for new patch.
Comment 15 Volodymyr Kostyrko 2023-03-20 21:32:39 UTC
(In reply to Max Brazhnikov from comment #13)

Well, that option never worked for me before. And right now your patch also doesn't change anything for me, I still see Telegram using embedded fonts after enabling it.

> pkg info -l telegram-desktop-qt6
telegram-desktop-qt6-4.5.0:
        /usr/local/bin/telegram-desktop
        /usr/local/share/applications/org.telegram.desktop.desktop
        /usr/local/share/icons/hicolor/128x128/apps/telegram.png
        /usr/local/share/icons/hicolor/16x16/apps/telegram.png
        /usr/local/share/icons/hicolor/256x256/apps/telegram.png
        /usr/local/share/icons/hicolor/32x32/apps/telegram.png
        /usr/local/share/icons/hicolor/48x48/apps/telegram.png
        /usr/local/share/icons/hicolor/512x512/apps/telegram.png
        /usr/local/share/icons/hicolor/64x64/apps/telegram.png
        /usr/local/share/licenses/telegram-desktop-qt6-4.5.0/GPLv3
        /usr/local/share/licenses/telegram-desktop-qt6-4.5.0/LICENSE
        /usr/local/share/licenses/telegram-desktop-qt6-4.5.0/catalog.mk
        /usr/local/share/metainfo/org.telegram.desktop.metainfo.xml

^^^ Here fonts are not available as separate files, they are embedded into the binary with all other resources.

# original build
.rwxr-xr-x 89M root 20 Mar 13:28 /usr/local/bin/telegram-desktop

# SYSTEM_FONTS
.rwxr-xr-x 89M root 20 Mar 22:35 /usr/local/bin/telegram-desktop

# EMBEDDED_FOINTS
.rwxr-xr-x 88M root 27 Feb 10:48 /usr/local/bin/telegram-desktop

All those are same Telegram version, built against same static libraries.

(In reply to Sergey A. Osokin from comment #11)

I guess you got me wrong. Enabled EMDEDDED_FONTS option doesn't alter the build at all, keeping it the same as intended by developer. Disabling this option results in embedded fonts being truncated prior to build, so they are still "embedded" to the binary, but being "incorrect" they are simply ignored and system preferences are kicking in.

In fact, I highly doubt Telegram Developers will ever consider allowing other fonts on their build. The reason for that is state of font rendering in QT, which is... flawed. Telegram Developers choose to stick to one font and make sure this font is properly positioned/used everywhere in the program. On the other hand OpenSans is a poorly hinted font, and on higher resolutions or with better hinting it looks worse then other fonts available. Other reason to switch out from it is bold text, which might be rendered indistinguishable from normal text.

Turning off OpenSans also has it's own deficiencies. Some fonts are too high, and engine is not fitting them correctly into interface, resulting in underlines being pushed out of the text box, so you can't see which text is underlined in the interface. Others are calculated too low, so they are rendered higher then optimal text position, resulting in unread numbers being rendered not at the center of the circle.

Probably I got suggestion wrong from the start, initial version just added another option that stripped embedded fonts when enabled, and I named it NO_FONTS. Probably I just need to rename it to SYSTEM_FONTS as suggested by Max and invert the logic? (i.e. disabled by default and removes fonts when enabled)

Thanks in advance!
Comment 16 Sergey A. Osokin freebsd_committer freebsd_triage 2023-03-21 00:25:00 UTC
(In reply to Volodymyr Kostyrko from comment #15)

That makes sense.

I'm still ready to add a disabled by default option, so that makes possible to build the program with a desired behavior.  Also, I'd like to ask you to raise a request on GH, so the Telegram Desktop development team can get all details on this feature and provide their comments on this feature request.

Thank you.
Comment 17 Volodymyr Kostyrko 2023-03-22 12:01:13 UTC
Created attachment 241052 [details]
fixed SYSTEM_FONTS patch

After some digging on GitHub I got out with idea Max approach should actually work. So I think I fixed his patch and now it works properly.

Or maybe I just missed his idea on the first approach... Anyway, attached patch adds SYSTEM_FONTS option and removes one extra setting that was shadowing configurable set by option.
Comment 18 Volodymyr Kostyrko 2023-03-22 12:03:19 UTC
Ah, regarding default build. DESKTOP_APP_USE_PACKAGED_FONTS defaults to off by design, so line I removed was actually noop until someone attempted to actually enable it.
Comment 19 Max Brazhnikov freebsd_committer freebsd_triage 2023-03-22 16:30:06 UTC
(In reply to Volodymyr Kostyrko from comment #17)
Right, I forgot to remove that line.
Comment 20 commit-hook freebsd_committer freebsd_triage 2023-04-23 02:53:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f7596f833caa1b18c84714e4648008a3fc78bbb2

commit f7596f833caa1b18c84714e4648008a3fc78bbb2
Author:     Sergey A. Osokin <osa@FreeBSD.org>
AuthorDate: 2023-04-23 02:50:34 +0000
Commit:     Sergey A. Osokin <osa@FreeBSD.org>
CommitDate: 2023-04-23 02:50:34 +0000

    net-im/t*: update Telegram Desktop to 4.8.0

    Also, add option to use system fonts. (*)

    PR:             264274          (*)

 net-im/telegram-desktop/Makefile | 9 +++++----
 net-im/telegram-desktop/distinfo | 6 +++---
 net-im/tg_owt/Makefile           | 4 ++--
 net-im/tg_owt/distinfo           | 6 +++---
 4 files changed, 13 insertions(+), 12 deletions(-)
Comment 21 Sergey A. Osokin freebsd_committer freebsd_triage 2023-04-23 02:54:33 UTC
Committed, thanks!