Skip to content

Ported to GTK4 #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 108 commits into
base: master
Choose a base branch
from
Open

Ported to GTK4 #290

wants to merge 108 commits into from

Conversation

trigg
Copy link
Contributor

@trigg trigg commented Feb 16, 2025

Extensive changes to bring the code base up to working on GTK4.

Dependencies:

  • gtkmm 3 > 4 (same for gdk, glib so on)
  • gtk-layer-shell > gtk4-layer-shell
  • libdbusmenu-gtk3 > libdbusmenu-glib

Material changes overview:

  • The menus on system tray icons had to be rewritten from scratch as the supporting library appears to not be available for gtk4
    • Menu & items are regenerated when changed over the dbus wire
    • Icons appear to be getting ignored by Popover Menu. From all my reading this is intentional on GTK4s part
    • Dbus library answers that there are no tickbox or radio button variants despite testing intentionally for them. These menu items function correctly but don't display the radio/checkmark
    • I am convinced there are some memory leaks here and am squashing them as I find them
  • The entirety of the event handling side of GTK has been rewritten and redefined.
    • System tray icons, Window list windows and application menu context buttons are all now MenuButton allowing us to connect menu options to them
    • EventBox no longer exists, so event callbacks are connected in directly to widgets
    • Volume control slider popup stops scroll wheel & swipe events on button, so the popup no longer shows when volume changes
  • Size allocation events, Redraw events, Scale change events have been removed from gtk4 and considered "the wrong way to do things" for most uses
    • Widgets should not base their internal logic on position & size
    • Scale is left to GTK4 to account for in all cases
    • Icon size should be changed with CSS rules only
    • All icon_size options have been fed into CSS Styles that are generated & updated from config
  • Output management and hotplugging is managed differently and worth double and triple checking by people with less boring monitor layouts than mine
  • Font override is removed. The only way I can see to do this now is to set it via CSS. While feeding the data in from the config is possible the way people write this options currently won't work in GTK4
    • Size needs to be in pixels or css sizes like : 17px 1.2em 0.9rem

While I was in the area:

  • Implemented CSS directory loading for dock and background
  • Added some hover, open window & close window animations to default css
  • Added a dock window-close-animation option as it has adverse effects if you enable it but don't update your default css
  • Realised we'd be better served with a Gtk Revealer... but that's a bigger task that I'll do in a separate attempt.
  • Reworked Background, needs sanity check as I'm sure it'll leak memory

I'm sure there's more, and will edit this as and when possible.

I look forward to uncrustify.

trigg and others added 30 commits February 8, 2025 16:02
- - Almost everything is broken.
- - This is expected
- - This is insane.
- - No, how is this even usable
- - Well whatever.
- - Someone please valgrind because I'm sure there's lots of bad ideas here.
- Fixes #8
- Partially fixes #9
- - Menu icons are currently still not shown
- - Menu checkboxes and radios show as generic label only
- Fixes #15
- Fixes #20
- Some changes to SNI dbusmenu
- Getting occassional segfaults in SNI dbusmenu so these need a better read-through before calling it
- - Still inherently broken until dbusmenu-glib reports a type back
- Dock takes entire width but sets itself to click-through on the outside
- Switched panel to using CenterBox which does a better job than my implementation
- Moved CSS dir loading into base app class
- First attempt at output changes
- Output hot plugging sorted.
- - This will need testing by people with more than 1 external monitor to see if there's any unknowns.
- Add an option to animate icons removed from dock
- Added to default dock CSS
- Fixed #4
- Fixed rectangle hints in dock and panel
- Fixed volume scrolling
- Fixed changing fill type in background triggering a new image load
- Remove GTK interactive debugging
trigg and others added 21 commits May 5, 2025 02:34
Swap the x texture coordinates because the images were reversed.
background: Rewrite to use GL instead of cairo for rendering
Before, we were uploading a texture twice: once when it's picked and once
after it switches to fade out. This makes it so the old texture id is
reused when fading to a new image, and avoids a call to glTexImage2D.
background: Optimize texture uploading by only calling glTexImage2D once
window-list: Set minimize target more reliably
window-list: Set minimize target even more reliably
@ammen99
Copy link
Member

ammen99 commented Jun 16, 2025

Hi and thanks for the big PR, very nice to see the update! I am going through the code to familiarize myself with whatever changes you have made, in the meantime I tried to copy the default CSS files and to compare the old and the new wf-panel. There are some very annoying differences which I am not sure how to solve, maybe they depend on the theme? Here is a screenshot, above is current master, below is this branch:

slurped

Most notably: backgrounds for menu, command and clock seem to not be unset, and there is a dark/black background behind the launchers instead of semi-transparent. Ditto for the ethernet connection. Icons also appear quite different.

@ammen99
Copy link
Member

ammen99 commented Jun 16, 2025

I am also getting Gtk-WARNING **: 09:47:31.071: Theme parser error: panel-selectors.css:21:11-20: Expected a number which I am not sure what to do about, I thought font could be monospace ...

@ammen99
Copy link
Member

ammen99 commented Jun 16, 2025

Window-list also appears weird:

slurped

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked over the changes, really good work :) My focus was the structure, haven't looked at every single UI call in detail but I assume we will find bugs like that during testing. A few things that I noticed or questions that I have.

@@ -7,3 +7,6 @@
[submodule "subprojects/wayland-logout"]
path = subprojects/wayland-logout
url = https://github.com/soreau/wayland-logout
[submodule "subprojects/gtk4-layer-shell"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we remove the gtk-layer-shell submodule (which is for gtk3)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I've missed something, that is GTK4, not 3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this is the good one, but the 3 version is also there in the gitmodules (unless I am mistaken)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it is, but .git/config had the right of it, and it seemed to be ignoring .gitmodules. Strange, but okay.

* the lifetime of the output */
return true;
// window->signal_close_request().connect(
// sigc::mem_fun(*this, &WayfirePanel::impl::on_delete));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is not needed anymore we cna just delete the code, no need to leave it commented :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a running bad habit of mine. Sorry!

@@ -315,6 +291,24 @@ class WayfirePanel::impl
public:
impl(WayfireOutput *output) : output(output)
{
// Intentionally leaking feels bad.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether these shouldn't be static variables. We don't need one instance per output, right? This creates new objects every time we plug an output, but if they were static variables, they would be initialized just once for all outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved them up into the App class, during on_activate. This is a much better position for them

// So on
new CssFromConfigFont("panel/battery_font", ".battery {", "}");
new CssFromConfigFont("panel/clock_font", ".clock {", "}");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1000 for the idea to translate .ini to css, I really like the solution :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed like the thing to do at the time.

I've had to re-write the font handling as it was broken again, and it hadn't been handling people using fonts like NotoSans 10 to assume 10pt. It now assumes pt if no unit is given from px rem or em

return result;
}

void WayfireWindowListBox::forall_vfunc(gboolean value, GtkCallback callback, gpointer callback_data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this code was to ensure that we can drag and drop toplevels in the list to rearrange them. I assume that custom drawing is no longer as simple as this code implements it?

If so, could we make use of gtk drag and drop to rearrange toplevels, potentially showing a drag icon? Currently, drag and drop does rearrange the toplevels but there is no animation and no indication of what is going on, which I personally find a bit confusing.

In addition, at the end of the drag and drop, the dragged view gets a special status (probably 'active drag and drop') and remains like that, so we have to unset somethinig probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically, we could subclass a LayoutManager, and write our own logic on size and location of children.

In reality I can't make it compile when I try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants