Bug #1856
opengtk4: Symbols are too small in certain windows
0%
Description
In the gtk4 clients, certain symbols are far too small to recognize. For example in the Unit selection window or the city production drop down. See attached screenshots.
The screenshots are from git revision https://github.com/freeciv/freeciv/commit/c470d233161593a0e517bb76844bd8eec95cbca9
I used the following commands to build the clients (in the source tree):
mkdir build
cd build
meson setup ../ -Dprefix=/home/christian/freeciv-install -Ddefault_library=static -Dclients=gtk3.22,gtk4
ninja installI'm on Arch Linux with a Wayland Plasma session. I have a dual-screen setup with 1920x1200 resolution (so it shouldn't be a HighDPI issue).
The pre-built 3.2.1 Appimage from https://files.freeciv.org/packages/appimage/Freeciv-gtk4-3.2.1-x86_64.AppImage has the same behavior.
Best regards
Christian
Files
Updated by Christian Mauderer 7 days ago
Updated by Marko Lindqvist 6 days ago
Seems to have broken with latest gtk4. Things work with gtk-4.18, but not with gtk-4.20 (I can reproduce)
Not sure if we can do anything ourselves, but I'm not holding my breath that gtk people would fix it either (they are going to replace related code in gtk5, and in the past they have not been keen to fix things in deprecated parts of the library)
Updated by Christian Mauderer 6 days ago
Thanks for looking into that. Do you know what part is broken? Is there already a ticket in gtk, or should I create one?
Updated by Marko Lindqvist 6 days ago
Those images are in GtkCellRendererPixbuf rendered column of GtkTreeView.
I have not yet searched gtk bugtracker for anything related.
Updated by Christian Mauderer 5 days ago
Marko Lindqvist wrote in #note-5:
Those images are in GtkCellRendererPixbuf rendered column of GtkTreeView.
I have not yet searched gtk bugtracker for anything related.
I have checked the bugtracker and I didn't find a bug that sounds like it could be related. The code for the GtkCellRendererPixbuf and GtkTreeView doesn't seem to have any relevant recent changes. So that sounds like a side effect of some other change which might could be hard to find.
Like you said, the code is already deprecated. So I'm not really sure whether it's worth to create a minimum working example and open a ticket for that problem. I'll check how much work it is.
Updated by Christian Mauderer 4 days ago
Just to keep this ticket up to date: Instead of opening a ticket, I thought it would be better to try to replace the deprecated APIs. For that I tried to hack in a GtkColumnView instead of the GtkTreeView. It's still missing most of the functionality, but it can at least show the units. See here: https://github.com/c-mauderer/freeciv/tree/214cb97da4cee1e23bf70ac30b642a63a2007186 Please note: I never used Gtk before and copied most of that together from examples. So the code most likely still has a low quality.
Unfortunately, with that GtkColumnView, I have the same effect. So it's most likely not the GtkTreeView that is causing the problem but more likely a general problem with rendering a GdkPixbuf.
Updated by Marko Lindqvist 4 days ago
Also gtk4x-client in main branch suffers from the same issues.
Since we support gtk4-client in older distributions / older gtk4 versions, we can't replace deprecated constructs that have replacement in newer gtk4 only. Instead we have separate gtk4x-client - future gtk5-client, where such changes are expected to happen (though it has not been updated for a while now).
Updated by Christian Mauderer 3 days ago
I was more fooling around a bit with that patch to (maybe) find a possible path. I don't expect that it will become production ready soon. If I had accidentally managed to get it production ready, and you don't want big changes in the gtk4 client, I would have just suggested applying it to the gtk5 client. By the way: I think the ColumnView has been there for most of the gtk4 history. So in theory, it wouldn't have been a breaking change for old versions.
But it doesn't really matter because the TreeView vs. ColumnView is not the problem, and therefore it doesn't really make sense to continue to hack on that patch. There seems to be a problem with the GdkPixbuf. Unfortunately, it seems that gtk wants to get rid of that one too. See https://gitlab.gnome.org/GNOME/gtk/-/issues/7235
Maybe I'll play around with the GdkPixbuf a bit to find out where the problem is. Don't worry: I don't expect that you accept any half-working patches, ones that you think are hard to maintain or ones that would break some other use cases.
Updated by Christian Mauderer 3 days ago
I gave up on experimenting and instead tried to find the gtk commit that is causing the issue. I created a minimal example that has the same problem here https://gist.github.com/c-mauderer/49287604a9a692f3ad28b4fe339cfe04 and bisected the problem down to the following gtk commit: https://gitlab.gnome.org/GNOME/gtk/-/commit/38b0a153e2e25034eec7722d15c89211dad679d5
Unfortunately, it's related merge request seems to think of that as a bugfix that "has the potential to break existing users": https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8627 So for gtk it's not a bug but something that works like expected. And if you take a look at that merge request, it has been fixed for example in gnome-music. So I assume that the only possible solution is to find a workaround that can be implemented in the freeciv gtk4 client.
Updated by Christian Mauderer 3 days ago
The gtk_cell_render_pixbuf code is using an GtkIconHelper internally. Due to the mentioned change, this limits the size of any pixbuf to the size of the icon in that context. I'm not sure whether that's really an intended behavior in GTK or whether it is a bug. But anyway: With that knowledge, a workaround is possible. It is just necessary to change the maximum size of icons for the GtkTreeView.
I have created a patch here: https://github.com/c-mauderer/freeciv/commit/22d270c3e6a6d447ac8470691aab55fc14b33d7b I checked that `gtk_widget_add_css_class` has been there since gtk4.0. Also I didn't compile it with that version, I wouldn't expect any problems.
Can you please check whether a patch like that would be an acceptable solution? If yes, I will also add it to the other GtkTreeViews in the code that use pixbufs and create a pull request on GitHub.
Updated by Marko Lindqvist 2 days ago
- Target version set to 3.2.3
From what you've told, the basic solution you are proposing is the best we can do.
- css class name should not contain word "unit" though, as it's not specific to units but there can be building icons too (at least in the production selection view)
- Be sure to update also gtk4x-client in the main branch, so that it won't start lagging behind the gtk4-client. There's no gtk4x-client in S3_3 or S3_2 branches, so you certainly need a different patch for those
Both github PRs and attaching patch files to this ticket are fine ways to submit the patch, though in my workflow I need to extract it as a file also from the PR.
Updated by Christian Mauderer 1 day ago
- File 0001-gtk4-unitselect-Remove-unused-fields-from-struct.patch 0001-gtk4-unitselect-Remove-unused-fields-from-struct.patch added
- File 0002-gtk4-Fix-icon-sizes-with-new-gtk-releases.patch 0002-gtk4-Fix-icon-sizes-with-new-gtk-releases.patch added
- File 0003-gtk5-unitselect-Remove-unused-fields-from-struct.patch 0003-gtk5-unitselect-Remove-unused-fields-from-struct.patch added
- File 0004-gtk5-Fix-icon-sizes-with-new-gtk-releases.patch 0004-gtk5-Fix-icon-sizes-with-new-gtk-releases.patch added
Hello Marko,
attached are four patches. Two are for gtk4 and two are for gtk4x/gtk5.
I noted that some fields of one of the structures in the unitselect dialog are unused. These are removed with the patch 0001 and 0003.
The patch 0002 is the ones that introduce the fix for the pixmaps in the gtk4 client. I had to slightly adapt the css because during tests I noted, that the size has been also applied to (for example) buttons in a dropdown list. In the gtk4 client, I checked all dialogs that I changed except for two: I haven't found how to easily trigger the `endgamerprt` in `repodlgs.c`. And I just have no idea, where to find the `editprop.c` dialog. I hope that I haven't missed any others.
Patch 0004 applies the same patches to the gtk5.0 client directory (which should be gtk4x?). I didn't systematically check all dialogs in that client but I clicked through a few and I didn't find anything unexpected related to these pixmaps.
If you prefer to pull the patches from a branch: I pushed them here: https://github.com/c-mauderer/freeciv/tree/c-mauderer/20260104_RM1856
Best Regards
Christian
Updated by Marko Lindqvist 1 day ago
- Status changed from New to In Progress
Thanks.
By our usual principle of 1 patch / 1 ticket, except for the same patch just backported to an older branch, these four patches would require splitting this ticket to four, but let me think it a bit (I'll take care of creating required tickets)
Updated by Marko Lindqvist 1 day ago
- Related to Feature #1871: gtk4: Drop unused fields from unit_select_dialog added
Updated by Marko Lindqvist 1 day ago
- Related to Feature #1872: gtk4x: Drop unused fields from unit_select_dialog added
Updated by Marko Lindqvist 1 day ago
- Related to Feature #1873: gtk3.22: Drop unused fields from unit_select_dialog added
Updated by Marko Lindqvist 1 day ago
- Blocks Bug #1874: gtk4x: Bad unit/building icon sizes in dialogs added
Updated by Marko Lindqvist 1 day ago
Split:
- Unused fields from unit_select_dialog in gtk4-client: #1871
- Unused fields from unit_select_dialog in gtk4x-client: #1872
- This ticket is about fixing icon sizes in gtk4-client (0002-gtk4-Fix-icon-sizes-with-new-gtk-releases.patch)
- Icon sizes in gtk4x-client: #1874
Also opened related ticket about unused fields in unit_select_dialog in gtk3.22-client: #1873
Updated by Marko Lindqvist 1 day ago
- Status changed from In Progress to In Review
- Assignee set to Marko Lindqvist
Updated by Christian Mauderer 1 day ago
By our usual principle of 1 patch / 1 ticket, except for the same patch just backported to an older branch, these four patches would require splitting this ticket to four, but let me think it a bit (I'll take care of creating required tickets)
Sorry, I haven't been aware of the process with one patch per ticket (or one ticket per patch). Otherwise, I would have combined the patches for the clients and would have left out the cleanup patches to reduce the effort.
Thanks for taking care of the split up. Let me know if I can help with something.