Tdesktop: Work with official QT

[ItachiSan]

This issue is opened in order to avoid in future the usage of a patched QT version.
The general idea would be to:

• Use system FCTIX QT plugin as many Linux distros can offer this plugin, else it can be compiled as dependency
• Avoid patching of QT stuff or send stuff upstream, in order to have a pure environment
  I would like to have feedback from developers and in case upload some code to patch stuff.

[john-preston]

1. as I understand I can't use it while linking Qt statically: "Note that the QPluginLoader cannot be used if your application is statically linked against Qt. In this case, you will also have to link to plugins statically"
2. no way to link dynamically to Qt (even if you avoid all the patches I use), because my text rendering code is based on the internal code of Qt, not the public interface provided, so I can't rely that the internal methods still exist and do the expected thing.

[YtvwlD]

I think the idea would be to link dynamically.
I don't really understand what's the problem here. I already have QT installed on my system. Why can't Telegram use it?

[ItachiSan]

@john-preston can't be the Telegram Desktop code refactored to use the official text rendering methods...?

@YtvwlD because Telegram Desktop use a patched version of QT5, so the methods in the official QT5 are different to the ones present in the patched version.

[jhasse]

This would also fix that the file dialog doesn't use my theme:

[YtvwlD]

Currently, nothing uses the system theme.

[AndydeCleyre]

@john-preston

my text rendering code is based on the internal code of Qt, not the public interface provided

What's the big advantage here? Lots of users and package maintainers would love for this to use official Qt, dynamically linked. So what is so advantageous about relying on patches and unreliable internal code that makes this worth it?

[john-preston]

@AndydeCleyre Well, when it was first made, I was working on a fast holding in memory + rendering of big amounts of text parts with graphic emoji, so that one message history can hold tens of thousands of text messages (I've tested up to 100 000) and window resize (which triggers the resize of all the messages to count the new window height) should work without lagging and scrolling should be smooth with instant switches between different conversations with big amounts of messages (tdesktop holds all the loaded messages in memory, local database for messages is not yet supported, so it needs all that) with acceptable memory consuming. I don't know how to achieve this using just Qt public interface.

[plinnell]

I can tell you as a developer for a project written in Qt since 2001, the patching of Qt by an application is a very bad idea. See: http:www.scribus.net We also deal with text rendering issues and have our own canvas because of the need to manipulate objects beyond text.

As a long time Linux user (15 years) and having packaged Linux applications for more than 10, not using Distro provided Qt over time is going to be an unholy, difficult to maintain mess. What happens when the patched Qt needs a security patch ? and Qt does get them ;) You are creating your own technical debt and a relatively young project needs to avoid this especially in the early days.

It seems most of what you are trying to fix, could be fixed by pulling in fcitx-qt5-devel, which provides the input capabilities for CJK.fcitx is pretty well supported on most recent distros.

Requiring a patched Qt is a very strong barrier to get your application into Linux distros. I'm one of the Factory maintainers for openSUSE and this would more than likely be rejected to be part of the distro, especially statically linking Qt and/or having your own patched Qt. I'm pretty certain Fedora/RedHat would say the same thing. Moreover, even if you apply patches, separating them out to different platforms is a best practice.

Now enough ranting. I do hope your team considers using https://openbuildservice.org to do your Linux packaging. It is cross distro and has many automation features which can ease package builds and provides infra for distribution. It can be hooked into Jenkins and speaks native git for pulling sources to build.

[gavv]

I totally agree with @plinnell. Also, patched Qt was the main reason why telegram source package was rejected by Gentoo developers. Bundled dependencies are usually very unwelcome by any distro maintainers and Linux community.

[WhyNotHugo]

@plinnell @gavv I don't think there's any rejection to actually stop patching Qt.

As far as I've understood, it's just that the devs haven't been able to get things working as expected with the upstream Qt, and nobody's offered another working solution (in the form of a PR). So rather than "up for debate", I think this issue is actually "awaiting patches".

[guoyunhe]

@gavv Some people already submited some pull request to use existing fcitx-qt5-devel package rather than patch the whole fcitx binding. But these pull requests have never been accepted.

[john-preston]

@guoyunhe fcitx is the small thing in the patch, it just got there because the patch was already in place.

The main problem is the text-with-emoji recounting/rendering, which uses private Qt apis.

I see no way in not using it trying to make fast-enough and not-so-memory-consuming text-with-emoji processing for tdesktop. And even if there is such way, which I don't know, it will be a huge amount of work to move the existing code base to that another way.

[AndydeCleyre]

@john-preston is it relatively simple to make a branch that uses Qt's slow/mem-heavy methods? How bad is it?

[john-preston]

@AndydeCleyre I don't know, how to make that in an easy way. You need to move all the SourceFiles/ui/text/text_.. on something completely different, that uses only public Qt API.

[john-preston]

@AndydeCleyre I guess you need to create a separate QTextDocument object for each displayed text piece and wrap all the methods that work with Text() right now to work with those documents.

[pschichtel]

What about upstreaming the changes? If your controls are in fact that much more efficient than the standard Qt ones, I guess a lot of other applications could benefit from them as well.

[john-preston]

@pschichtel I'm pretty sure this can't be done :( They're not general controls in any way, just very app-specific pieces of code. Perhaps the approach that was used there could be generalized some how but it is a huge amount of work in its own and after that I still doubt that any of it will be merged to Qt.

[pschichtel]

@john-preston Are entire controls implemented in the Qt patches? I thought more about the optimizations you did. Or you could add in public APIs for the private ones you are using and upstream them.
This all sounds like hacking around limitations of a library instead of just removing the limitations from the library, I don't like that.

[WhyNotHugo]

Is is somehow possible to make those "private Qt APIs" public upstream? Has this even been discussed?

[AndydeCleyre]

This is by far the most important bug in this project, and I would contribute to a bug bounty for it.

[Schekolda]

So, will you stop using patched QT?

[stek29]

https://aur.archlinux.org/packages/telegram-desktop-systemqt-notoemoji/ may help (from #3129)

[KireinaHoro]

As well as https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=telegram-desktop-systemqt .

[AndydeCleyre]

Can someone write here what the deal is with those "systemqt" packages? What works and what doesn't?

[emanuelserpa]

It's working great here, I'm not seeing anything not working in https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=telegram-desktop-systemqt
Also, the deal is that it compiles with the official Qt, without patched Qt

[AndydeCleyre]

Wow, it does seem to work perfectly so far. @john-preston can you check this out and point out what we should expect to be broken -- and if nothing (or very little) is, how this might affect the dependency on your custom Qt going forward?

[agura-lex]

By the way, using system qt fixes this ancient bug: #1026. It appears to be libappindicator bug, I guess, as systemqt version (https://aur.archlinux.org/packages/telegram-desktop-systemqt) uses native tray icon.

[jhasse]

I don't think that has anything to do with Qt, but if the build uses libappindicator or not.

[emanuelserpa]

Any updates?

[portaloffreedom]

Not using official Qt actually breaks the program running over wayland

[orschiro]

@agur4ik what you were saying is related to this theme inconsistency, correct?

[mxa]

I suspect that fixing this would also resolve two other bugs which are quite annoying:

1. make nimf work with Telegram https://github.com/telegramdesktop/tdesktop/issues/2603
2. compose key currently broken in Telegram https://github.com/telegramdesktop/tdesktop/issues/2603

[agura-lex]

@orschiro, it might be related but I'm not sure as I've not tested that. But since you are using GTK-based environment, theme inconsistency more probably has something to do with different GTK and Qt styles. Here's described how to fix it: https://wiki.archlinux.org/index.php/Uniform_look_for_Qt_and_GTK_applications.
(I hope I got your question right)

[Peque]

@john-preston Friendly ping. :innocent:

[john-preston]

@Peque You can read my replies above. I don't know an easy and reliable way of moving this project to the non-custom Qt. Most of the patching is used on Windows and OS X, but some on Linux as well. Right now:

— Text layout uses internal Qt API.. All attempts to work with official Qt up till now still used it, just hoped that the required methods are being present in the dynamic libraries. If Qt some day changes or removes those methods the app would simply fail to launch.

— Open Sans Semibold font that is used widely in the app wouldn't load without patching Qt for that font. I don't know whose fault is that, but I couldn't make it work, Qt saw it just as another "Open Sans" font and all semibold text parts were rendered as normal text (names in chats list etc).

— Detection of the compositing manager was backported from next versions of Qt, so I could know if transparent windows are supported and not try to show them if they aren't.

Fixing bugs only in the upstream Qt is inefficient. If you see bugs / crash reports and you manage to find and fix them in Qt and even if you manage to report and include those fixes in some next version (which requires huge amount of work, I almost never have time for this), your current users still won't be able to use your app.

Also I'll need to deploy current versions anyway, because all current users are using the app and autoupdating to the next versions, if I just remove the linked Qt in some cases the app just won't launch anymore.

[jhasse]

What about using https://github.com/CrimsonAS/gtkplatform/ for your static Qt version? This way the theming would be picked up from system Gtk at least.

[orschiro]

@agur4ik

it might be related but I'm not sure as I've not tested that. But since you are using GTK-based environment, theme inconsistency more probably has something to do with different GTK and Qt styles. Here's described how to fix it: https://wiki.archlinux.org/index.php/Uniform_look_for_Qt_and_GTK_applications.
(I hope I got your question right)

You got it right! However, I tried this already. If I understand correctly, then this would work if Telegram Desktop uses the non-forked version of Qt.

@jhasse

What about using https://github.com/CrimsonAS/gtkplatform/ for your static Qt version? This way the theming would be picked up from system Gtk at least.

Does the user have to use this or the developer?

[Peque]

@john-preston Thanks for your reply. Even though I read your comments above, it is good to have an update on this matter after more than one year. Just in case anything had changed. :wink:

[emanuelserpa]

@john-preston What's your thoughts about https://www.archlinux.org/packages/community/x86_64/telegram-desktop/? They are using official qt now.

[john-preston]

@emanuelserpa This looks promising. If I understood correctly they've just copied small amount of code from Qt source that isn't available through Qt public API and compile it inside the binary.

[agura-lex]

@emanuelserpa I think, in Arch it is easy to incorporate system Qt Telegram build as it uses the latest and greatest software (@john-preston pointed out that some fuctionality is backported from the newer versions of Qt). Distributing Telegram with native Qt would result in the need to provide different TG builds for the different Qt versions. I never thought about this earlier though. Now I think that the issue should be resolved in downstream rather than upstream: let distro maintainers build TG with native Qt (if they will) and TG devs continue providing static build unified for all distros. However, it might be a good idea to simplify building the app against the different Qt versions by making this thing configurable (if that's easily possible) in order to avoid patching from distro maintainers.

P.S.: sorry for my English.

[stek29]

Distributing Telegram with native Qt would result in the need to provide different TG builds for the different Qt versions

It shouldn't. At least when done properly.

[orschiro]

let distro maintainers build TG with native Qt (if they will) and TG devs continue providing static build unified for all distros

Let's take Ubuntu as an example. It doesn't provide Telegram Desktop in its official repositories. But as far as I know there is a Snap as well as Flatpak for TD. Does this make a difference to the argument?

[gasinvein]

Use of patched Qt is a strong argument against adding Telegram to official repositories. Maybe if Telegram would work with distro's native Qt, there will be no need in Snap or Flatpak for it.

[loimu]

Let's take Ubuntu as an example. It doesn't provide Telegram Desktop in its official repositories.

Really? What's that then?

[orschiro]

@loimu forget what I just said. Don't even know how I missed that. And I was sure it didn't have the package the last time I was looking for it in the official repos. :sweat_smile:

[shibotto]

ebuilds for Gentoo are available too https://github.com/reagentoo/gentoo-overlay/tree/master/net-im/telegram-desktop
I've been using this for a couple of weeks and not a problem so far.

[gasinvein]

[3v1n0]

[balsoft]

[mikken]

[ilya-fedin]

[emanuelserpa]

[portaloffreedom]

[ilya-fedin]

[ilya-fedin]

[ilya-fedin]

[portaloffreedom]

[ilya-fedin]

[emanuelserpa]

[ilya-fedin]

[emanuelserpa]

[ilya-fedin]