tlinstaller and tl-setup should use the new graphical profile for ThinLinc.
Loading images unconditionally is a bad idea: > 2022-05-20 08:46:40,224: Traceback (most recent call last): > 2022-05-20 08:46:40,224: File "/home/cendio/tl-4.14.0post-server/libs/tlinstaller", line 758, in <module> > 2022-05-20 08:46:40,224: IIiI11I1I1 ( ) > 2022-05-20 08:46:40,224: File "/home/cendio/tl-4.14.0post-server/libs/tlinstaller", line 693, in IIiI11I1I1 > 2022-05-20 08:46:40,224: O0o = tlgtk . GdkPixbuf . Pixbuf . new_from_file_at_scale ( > 2022-05-20 08:46:40,224: AttributeError: module 'thinlinc.tlgtk' has no attribute 'GdkPixbuf'
We now consider the branding of tl-setup and tlinstaller done. A summary of what has been done: - Intro and banners for tl-setup - Minor layout adjustments Marking as resolved.
Apparently, my lack of sleep made the summary in comment #14 read okay. Here is an improved version that should be readable by anyone, sleep-deprived or not: A summary of what has been done: - Intro and outro banners for tl-setup and tlinstaller - Minor layout adjustments
Note that we have an issue with low contrast with the graphics added for this bug in combination with darker themes. Since the blue that we use in the graphical profile is rather dark it does not work well with dark backgrounds. Since I have very little insight into the graphical profile work, I'll reopen this bug until we have a clearer picture of what can be done here. Ideally, we would have color agnostic SVGs that can be recolored programmatically or graphics with a pre-defined background color that preserves contrast and works with dark themes. Note that this issue may not be that pressing since both tl-setup and tlinstaller are always run under the root user which in the vast majority of cases uses Adwaita or another light Gtk theme.
Another issue with our graphics as they are added to tl-setup and tlinstaller at the moment is seen when running them with scaling or HiDPI. I used GNOME's scaling set to 300% on a 4k monitor. The layout looks great, elements and text look very sharp and nice! However, our first page and last page graphics stand out in a bad way since they look very blurry. These images are much larger on disk than the size currently being used in tl-setup and tlinstaller. We should have some extra pixels to use in HiDPI or scaled mode.
The issues mentioned in comment 18 have now been resolved. Though, the issues with the current graphical material mentioned in comment 17 remain.
Unfortunately pycairo's set_device_scale() is too new.. it was introduced in pycairo 1.14.0. My RHEL 7 test machine only has pycairo-1.8.10: > 2022-06-02 16:42:59,760: *** ThinLinc installer started *** > 2022-06-02 16:42:59,761: Architecture: x86_64 > 2022-06-02 16:42:59,762: Distribution: RHEL > 2022-06-02 16:42:59,762: Package format: rpm > 2022-06-02 16:42:59,899: Traceback (most recent call last): > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/tlinstaller", line 743, in <module> > 2022-06-02 16:42:59,899: IIiI11I1I1 ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/tlinstaller", line 695, in IIiI11I1I1 > 2022-06-02 16:42:59,899: II1iii1IiIiii = IiI . run ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/wizard.py", line 110, in run > 2022-06-02 16:42:59,899: return self . _run_gui ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/wizard.py", line 141, in _run_gui > 2022-06-02 16:42:59,899: self . _banner_path , self . _banner_scale ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/wizard.py", line 150, in add_intro > 2022-06-02 16:42:59,899: banner_path , banner_scale ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/wizard.py", line 92, in _add_intro_or_finish > 2022-06-02 16:42:59,899: IiI11I , Ooo0OO , I1i1Ii , > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/factory.py", line 218, in scale_aware_surface_from_pixbuf > 2022-06-02 16:42:59,899: iiOoO0O0OO0 . set_device_scale ( pixbuf . get_width ( ) / width , > 2022-06-02 16:42:59,899: AttributeError: 'cairo.ImageSurface' object has no attribute 'set_device_scale'
After discussing the matter, we can seemingly ignore the issue with bad image/background contrast with dark backgrounds. There are currently two ways of getting a dark background in GTK: - Custom themes (not officially supported) - Dark mode (GTK4 feature) Since we are currently targeting GTK3 and only care about officially supported features we can simply assume a light background.
Comment #21 has now been resolved. This is ready for testing again.
We found two issues with the changes made here: * The checkmark used at the end of tl-setup and tlinstaller should be an SVG. It will be easier to edit in the future. It was created as an SVG initially, but that file doesn't use transparent objects like we want. It will require some adjustments before we can swap out the current PNG. * The intro graphic/banner should not contain padding in the image itself, this should be part of the layout instead. Note that changing this will require some layout work since currently, the banner image is responsible for the wide window we get. The goal is for the GUI to look the same, but for the image to be cropped to the content.
Doesn't work on SLES 12 or SLES 15 as they've failed to mark pycairo as a dependency of GTK's python bindings: > 2022-06-09 10:49:58,075: Traceback (most recent call last): > 2022-06-09 10:49:58,075: File "/home/cendio/tl-4.14.0post-server/libs/tlinstaller", line 748, in <module> > 2022-06-09 10:49:58,075: i1111 ( ) > 2022-06-09 10:49:58,075: File "/home/cendio/tl-4.14.0post-server/libs/tlinstaller", line 700, in i1111 > 2022-06-09 10:49:58,075: Ii1IIiiI1II = OOo0o00o . run ( ) > 2022-06-09 10:49:58,075: File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/wizard.py", line 110, in run > 2022-06-09 10:49:58,075: return self . _run_gui ( ) > 2022-06-09 10:49:58,075: File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/wizard.py", line 141, in _run_gui > 2022-06-09 10:49:58,076: self . _banner_path , self . _banner_scale ) > 2022-06-09 10:49:58,076: File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/tlgtk/wizard.py", line 150, in add_intro > 2022-06-09 10:49:58,076: banner_path , banner_scale ) > 2022-06-09 10:49:58,076: File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/tlgtk/wizard.py", line 92, in _add_intro_or_finish > 2022-06-09 10:49:58,076: IiI11I , Ooo0OO , I1i1Ii , > 2022-06-09 10:49:58,076: File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/tlgtk/factory.py", line 250, in scale_aware_surface_from_pixbuf > 2022-06-09 10:49:58,076: pixbuf , OOoO0oOo0 , for_window = None > 2022-06-09 10:49:58,076: TypeError: Couldn't find foreign struct converter for 'cairo.Surface'
Same thing on Ubuntu 22.04.
Found an existing report for Debian for this: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995784 I also reported the issue to Ubuntu, to get more attention to it: https://bugs.launchpad.net/ubuntu/+source/pygobject/+bug/1978098
Found an issue for OpenSUSE: https://bugzilla.opensuse.org/show_bug.cgi?id=1179584 Trying to find how to report this to SUSE Enterprise.
Once the pycairo packaging issue has been worked around, bug 7905 should be marked as resolved as well.
The cairo bug also affects Red Hat. :/ A default install of RHEL 9 gives you python3-gobject-base, but no python3-gobject. This gives you PyGObject without any cairo bindings. Which means we detect PyGObject, and never suggest installing python3-gobject.
This is getting very complicated with all the different components we need, and the variation in dependencies between distributions. For tl-setup that is fine since we just need a binary yes/no if everything is installed. But syscheck.sh tries to perfectly map missing features with packages, which is very complex. I think we need to reduce the ambition of what syscheck.sh does. I see two paths: a) syscheck.sh works similarly to tlsetup, where the check is a simple yes/no and we try to install everything no matter how much or little is missing. b) we stop trying to bootstrap GTK from syscheck and fall back to text mode if something is missing (bascially revert to before bug 5742) The motivation for bug 5742 was that we were stuck on PyGTK whilst everyone else had moved on to PyGObject. That is no longer the case, so reverting to the simpler behaviour might not be much of an issue in practice. Also note that you generally have to start the installer from a terminal anyway, because of bug 7655.
(In reply to Pierre Ossman from comment #39) > The cairo bug also affects Red Hat. :/ > > A default install of RHEL 9 gives you python3-gobject-base, but no > python3-gobject. This gives you PyGObject without any cairo bindings. Which > means we detect PyGObject, and never suggest installing python3-gobject. Note that this also exposes that there is another fun state the system can be in; it's possible to have both PyGObject and Pycairo installed, but the integration between them missing. So it's insufficient to just test for them separately. Fortunately, PyGObject provides the function "require_foreign()" to test that the integration is working correctly.
For everything to work correctly, the following components need to be in place: * PyGObject core * PyGObject GDK/GTK "fixes" (called "overrides") * PyGObject cairo integration * Pycairo * libcairo * GTK+ 3 * GTK+ 3 typelibs All of these are packaged in various ways in the distributions. tl-setup needs to make sure all of these are installed. There are (at least) two ways to go about this: Either, we have a large list of packages that include all of these. That makes sure we don't rely on correct dependency chains in the distributions, but adds the risk of things breaking if the distributions split or rename things. Or we try to have a minimal list, ideally of just the top level things we need. This is probably the cleanest, as we can't keep track of the entire dependency tree. However, this makes us vulnerable to missing dependency chains. Which evidently already exist, so this method would need to have some extra packages added to compensate for bugs in the distribution dependency chains.
Everything should work well now. Just need to test things on actual distributions.
Another error spotted, apparently SVG support cannot be taken for granted: > 2022-06-15 14:01:45,837: Traceback (most recent call last): > 2022-06-15 14:01:45,837: File "/opt/thinlinc/sbin/../libexec/tl-setup.py", line 317, in <module> > 2022-06-15 14:01:45,837: oOoO00 ( ) > 2022-06-15 14:01:45,837: File "/opt/thinlinc/sbin/../libexec/tl-setup.py", line 273, in oOoO00 > 2022-06-15 14:01:45,837: IiiI1III1iI = Iioo0Oo0oO0 . run ( ) > 2022-06-15 14:01:45,837: File "/opt/thinlinc/modules/thinlinc/wizard.py", line 110, in run > 2022-06-15 14:01:45,837: return self . _run_gui ( ) > 2022-06-15 14:01:45,837: File "/opt/thinlinc/modules/thinlinc/wizard.py", line 144, in _run_gui > 2022-06-15 14:01:45,837: iI1iI ( iiI1111II ) > 2022-06-15 14:01:45,837: File "/opt/thinlinc/modules/thinlinc/tlsetup/summary.py", line 60, in IIiiI11ii > 2022-06-15 14:01:45,837: banner_scale = 1.2 , > 2022-06-15 14:01:45,837: File "/opt/thinlinc/modules/thinlinc/tlgtk/wizard.py", line 174, in add_finish > 2022-06-15 14:01:45,837: banner_scale = banner_scale ) > 2022-06-15 14:01:45,837: File "/opt/thinlinc/modules/thinlinc/tlgtk/wizard.py", line 98, in _add_intro_or_finish > 2022-06-15 14:01:45,837: preserve_aspect_ratio = True , > 2022-06-15 14:01:45,837: GLib.GError: gdk-pixbuf-error-quark: Couldn’t recognize the image file format for file “/opt/thinlinc/share/pixmaps/setup_finish.svg” (3) Seen on RHEL 7.
Also seen on RHEL 9. It seems librsvg2 is not a hard dependency of GTK. So we need to make sure that is installed.
This is not an issue on SLES, as libgtk-3-0 depends on gdk-pixbuf-loader-rsvg. Debian/Ubuntu is in a gray area as libgtk-3-0 *recommends*, but doesn't *depends* on librsvg2-common. I.o.w., a normal system will get librsvg2 if gtk3 is installed. But it is possible to disable this behaviour, or uninstall librsvg2 later, without losing gtk3.
Tested that the correct packages are pulled in from a minimal installation on: * RHEL 7 * RHEL 9 * SLES 12 * Ubuntu 22.04 Also checked that it doesn't complain about missing packages on a second run on all above. Checked on Ubuntu 22.04 that it falls back to text mode if something is missing, even if it has X11: * Missing PyGObject * "Broken" PyGObject (gi is just a namespace) * Missing Cairo integration * Missing GTK bindings * Missing SVG support Also checked that tl-setup installed the missing pieced in the above scenario. Checked on Ubuntu 22.04 that it tries to pop up a dialog if both GTK+ and a terminal is missing. Tried kdialog and zenity. This mess should finally be properly resolved.
I now get these warnings in the terminal when running the graphical installer or tl-setup: > (tl-setup.py:57774): GdkPixbuf-WARNING **: 07:12:33.593: GdkPixbufLoader finalized without calling gdk_pixbuf_loader_close() - this is not allowed. You must explicitly end the data stream to the loader before dropping the last reference. > Gtk-Message: 07:12:33.626: Failed to load module "pk-gtk-module" Seen on Ubuntu 22.04
There is something off with the size on SLES 12 as well. The window (for at least tl-setup) ends up much wider than on other systems. The side margins look the same, so it's the content are that has expanded. No obvious reason why, though.
Tested the Cairo and SVG dependency changes using server-bundle #2705. Looking at https://pygobject.readthedocs.io/en/latest/getting_started.html it seems like we have it all covered, with the addition of svg-support. Moreover, I performed the following tests on Ubuntu 18.04, Fedora 36, and SLES15. • Test that we actually install the required GTK dependencies: 1. Install ThinLinc on a minimal system 2. Run tl-setup once to install the GTK deps 3. Run tl-setup again and verify that it's run in graphical mode • Test that tl-setup can properly detect the presence of all package: 1. Identify each independent dependency subgraph of the GTK packages that we install 2. Remove each subgraph individually from the system, making sure that tl-setup starts in text-mode and detects the removed dependency subgraph as missing. Things worked fine in all cases but one: On SLES15, removing `python3-gobject-Gdk' and running tl-setup results in the following stacktrace: > ┌──── > │ Traceback (most recent call last): > │ File "/home/cendio/tl-4.14.0post-server/libs/libexec/tl-ssh-askpass.py", line 59, in <module> > │ OOoOoo000O00 ( ) > │ File "/home/cendio/tl-4.14.0post-server/libs/libexec/tl-ssh-askpass.py", line 50, in OOoOoo000O00 > │ visibility = False ) > │ File "/home/cendio/tl-4.14.0post-server/libs/modules/thinlinc/tlgtk/dialog.py", line 116, in __init__ > │ self . vbox . pack_start ( self . entry , expand = False , fill = False , padding = 0 ) > │ AttributeError: 'EntryDialog' object has no attribute 'vbox' > │ sudo: no password was provided > │ sudo: a password is required > └──── Reinstalling the removed package resolves the problem. Also, note that the issue described in comment 59 is fixed as of server build #2705
Reported that latest SUSE issue here: https://bugzilla.opensuse.org/show_bug.cgi?id=1200614
Retested PyGObject/GTK detection and installation on: * RHEL 9 * SLES 12 * Ubuntu 22.04 Still works well, and now detects if the overrides are missing.
(In reply to Pierre Ossman from comment #60) > There is something off with the size on SLES 12 as well. The window (for at > least tl-setup) ends up much wider than on other systems. The side margins > look the same, so it's the content are that has expanded. No obvious reason > why, though. We've done some initial digging in to this and something is seriously rotten in the state of SUSE. The inspector reveals that GTK on SLE 12 mishandles points and pixels dimensions, and the conversion factor is applied twice. Hence, we end up with a too large font. We can see this by inspecting a normal text widget, that is using the default size. It is supposed to be 11pt, which at 96 DPI becomes 14.666...px. This is what we see on working systems. But on SLE 12 we instead see 11px in the inspector. This is of course completely bogus, as the output ends up being the correct 14.666...px. So somewhere between the CSS and the output, the conversion factor gets applied a second time. So far, we only see this on our applied CSS. If we change font sizes in the UI, everything turns out as expected. A pt size is specified, and the correct px height is rendered. At this point, we don't know if this is SLE 12 specific, or if it's because it's using an older GTK.
As diagnosed, the problem is a double conversion. GTK+ transforms all CSS values to pixels when loading, which is where the first conversion happens. It then messes up the units, and calls pango_font_description_set_size() with the pixel value it gets from CSS. But that function takes points, not pixels. So Pango will do a second conversion from points to pixels as part of its rasterisation. This was fixed here: https://gitlab.gnome.org/GNOME/gtk/-/commit/df08fc91bdc1d2e4c866122304fabe4dd298a7de The first stable release to include this fix is 3.22.0. Unfortunately, this is way before GTK+ got the style classes we rely on. Which means we can't just adjust the values in our fallback CSS as there are many versions where the fallback is needed, but the values are correctly interpreted. We'll have to come up with something more dynamic to support GTK+ 3.20 properly.
Retested the cairo/librsvg2-related changes after the change added in response to comment 65 using server build #2712. Testing was done on SLES15, Ubuntu 18.04 and Fedora 36. The GTK detection stuff now seems to work as intended and can be considered tested.
This bug is set for testing.
Tested latest build #2726 on Ubuntu22.04 / GNOME RHEL7 / GNOME RHEL8 / GNOME SLES12 / GNOME openSUSE15 / GNOME Fedora36 / MATE Fedora36 / Xfce Fedora36 / KDE Fedora36 / GNOME (HiDPI setup) Concerning tlinstaller and tl-setup, the new graphical profile pertain the intro and summary pages, more specifically the newly introduced intro banner and finish checkmark images. All the setups listed above exhibit good quality images with associated margins and sizes working as intended, and in particular in the HiDPI setup the images are displayed with proper sharp resolution.
Tested running tlinstaller and tl-setup using X11 forwarding to Fedora 35 on: * RHEL 7 * RHEL 9 * SLES 12 * SLES 15 * Ubuntu 18.04 * Ubuntu 20.04 All started in a minimal state to check syscheck.sh. Then all GTK stuff was installed, and the installation was actually done. Everything works well and looks good.
Also tested running tl-setup under Cinnamon and LXDE on Ubuntu 22.04. Looks good there as well.
Tested the following distro/DE combinations using server build #2731. • Ubuntu 18.04 • ☑ Ubuntu desktop • ☑ XFCE • Ubuntu 20.04 (the first Ubuntu LTS with proper HiDPI support) • ☑ Ubuntu desktop (HiDPI) • ☑ XFCE (HiDPI) • ☑ KDE (HiDPI, with GDK_SCALE set) Looks good to me. Closing.
Found a regression from 4.14.0. The following output is seen in tl-setup when running in text mode and via ssh without enabling X11-forwarding. > System Check > ============ > > Analyzing system... Unable to init server: Could not connect: Connection refused > Unable to init server: Could not connect: Connection refused > done. This regression was caused by the following line from commit r38571: > from gi.repository import Gtk Solving this is non-trival, and this issue has been moved to bug 7069.
The issue mentioned in comment 95 was seen on Ubuntu 20.04.
Another regression from 4.14.0 found. On Ubuntu 22.04 and Fedora 35, the following output is seen when running in dev mode. > System Check > ============ > > Analyzing system... <frozen importlib._bootstrap>:671: ImportWarning: DynamicImporter.exec_module() not found; falling back to load_module() > <frozen importlib._bootstrap>:671: ImportWarning: DynamicImporter.exec_module() not found; falling back to load_module() > done. This regression was caused by the following line in commit r38564: > from gi.repository import GdkPixbuf Since dev mode is disabled for our released versions, this issue is less pressing since it will be hidden from the end users. However, it is still non-trivial to solve, so it is moved to bug 7069.
