search

FedoraQt / QGnomePlatform

QPlatformTheme for a better Qt application inclusion in GNOME
GNU Lesser General Public License v2.1
262 stars 38 forks source link

Redesign decoration to LibAdwaita look #145

Closed urFate closed 1 year ago

urFate commented 1 year ago

It's me again. I had to open a new pull request because after the upstream changes, the history of previous commits was erased for some reason, I don't think it's critical.

Result

🌑 Dark Scheme

☀️ Light Scheme
grulja commented 1 year ago

Hi, thanks for opening it again. Before I dive into the proper full review, few comments: 1) Can you squash both commits into one? 2) There seem to be unrelated coding-style changes, like change in the copyright header where you remove some spaces

urFate commented 1 year ago

Hi, thanks for opening it again. Before I dive into the proper full review, few comments:

  1. Can you squash both commits into one?
  2. There seem to be unrelated coding-style changes, like change in the copyright header where you remove some spaces

Yes, sorry again, didn't saw that.

grulja commented 1 year ago

There are still unrelated coding style changes. I recommend running clang-format locally.

urFate commented 1 year ago

There are still unrelated coding style changes. I recommend running clang-format locally.

It should be fine now.

grulja commented 1 year ago

Hi, going through the code, it looks good code-wise, I just didn't have opportunity to test it yet, mainly because I was a week away attending KDE Akademy conference and also because I don't use GNOME by default and running Kinoite makes it harder to install it. My proposal to remove custom Qt theming for Fedora 39 has been approved so I will start intensively working on it next week and I will finally get to test your change and compare it to the current GNOME upstream. Thank you for your patience.

grulja commented 1 year ago

I switched now to GNOME and I can see for example with Nautilus that GTK4/libadwaita apps still have shadows around the window, while your patch removes them completely.

urFate commented 1 year ago

Oh, really? It was not easy for me to notice them on the configuration of my monitor, so I decided that they simply did not exist. As soon as I am free, I will revert the removal of shadows in the commit.

grulja commented 1 year ago

Oh, really? It was not easy for me to notice them on the configuration of my monitor, so I decided that they simply did not exist. As soon as I am free, I will revert the removal of shadows in the commit.

They are very subtle and barely visible, but they exist. I don't think they actually changed between GTK3 and GTK4. and our shadows definitely have room for improvements.

urFate commented 1 year ago

Shadows has been restored.

grulja commented 1 year ago

Hi, just to let you know what's going on. As I mentioned, I'm planning to discontinue QGnomePlatform, but we will still need some kind of client-side decorations for Qt on GNOME. I started looking into libdecor but that's currently a no-go for QtWayland so I gave up on that. Instead, I'm writing a completely new decoration plugin, basically taking parts of QGnomePlatform and Adwaita-qt, but avoiding GTK and Adwaita-qt as dependencies so the plugin can eventually be upstreamed to Qt. I changed the buttons to make them look like GTK4 version and drastically simplified the code. Currently the only missing part is to reimplement shadows and get correct colors from GTK4. I might have a fully working version by the end of this week.

May I ask where did you get the colors? Did you take them from the GTK4 stylesheet? Or did you just took a picture and used some image editor to get them? I don't like the idea of hardcoding these colors, ideally we would have a way to get them on runtime, but that's not probably possible, especially when not having GTK as a dependency.

grulja commented 1 year ago

Sharing what I have in case you want to look and possibly contribute your changes updating colors?

The repo is now here: https://github.com/FedoraQt/QAdwaitaDecoration

I have some other work planned for today and tomorrow, but I can continue with implementing the shadows later. Just saying in case you want to look into implementing the proper colors? Currently our only option is to probably hardcode these colors the same way we have here in QGnomePlatform.

Btw. to build the decoration plugin you need QtWayland from Fedora (API changes) which I suppose you use? And use QT_WAYLAND_DECORATION=adwaita to use them.

Edit: I see you are probably using Archlinux. The patch I'm talking about we have in qt5-qtwayland is this one. It's needed for proper shadows support.

urFate commented 1 year ago

Hi, just to let you know what's going on. As I mentioned, I'm planning to discontinue QGnomePlatform, but we will still need some kind of client-side decorations for Qt on GNOME. I started looking into libdecor but that's currently a no-go for QtWayland so I gave up on that. Instead, I'm writing a completely new decoration plugin, basically taking parts of QGnomePlatform and Adwaita-qt, but avoiding GTK and Adwaita-qt as dependencies so the plugin can eventually be upstreamed to Qt. I changed the buttons to make them look like GTK4 version and drastically simplified the code. Currently the only missing part is to reimplement shadows and get correct colors from GTK4. I might have a fully working version by the end of this week.

May I ask where did you get the colors? Did you take them from the GTK4 stylesheet? Or did you just took a picture and used some image editor to get them? I don't like the idea of hardcoding these colors, ideally we would have a way to get them on runtime, but that's not probably possible, especially when not having GTK as a dependency.

Yep, I was just used eyedropper to get the colors of window decoration.

Sharing what I have in case you want to look and possibly contribute your changes updating colors?

The repo is now here: https://github.com/FedoraQt/QAdwaitaDecoration

I have some other work planned for today and tomorrow, but I can continue with implementing the shadows later. Just saying in case you want to look into implementing the proper colors? Currently our only option is to probably hardcode these colors the same way we have here in QGnomePlatform.

Btw. to build the decoration plugin you need QtWayland from Fedora (API changes) which I suppose you use? And use QT_WAYLAND_DECORATION=adwaita to use them.

Edit: I see you are probably using Archlinux. The patch I'm talking about we have in qt5-qtwayland is this one. It's needed for proper shadows support.

As I understand it, you decided to rewrite the scenery plugin from scratch. I can help with the colors, I'll try to look for the original stylesheet and take the colors from there, although I doubt they will be different from the ones I got from the color picker. However, I still have to figure it all out a bit. Regarding the last two paragraphs, as I understand it, I need a new patch for qt5 to build this project or was it just a clarification? I actually use arch linux.

grulja commented 1 year ago

Hi, just to let you know what's going on. As I mentioned, I'm planning to discontinue QGnomePlatform, but we will still need some kind of client-side decorations for Qt on GNOME. I started looking into libdecor but that's currently a no-go for QtWayland so I gave up on that. Instead, I'm writing a completely new decoration plugin, basically taking parts of QGnomePlatform and Adwaita-qt, but avoiding GTK and Adwaita-qt as dependencies so the plugin can eventually be upstreamed to Qt. I changed the buttons to make them look like GTK4 version and drastically simplified the code. Currently the only missing part is to reimplement shadows and get correct colors from GTK4. I might have a fully working version by the end of this week. May I ask where did you get the colors? Did you take them from the GTK4 stylesheet? Or did you just took a picture and used some image editor to get them? I don't like the idea of hardcoding these colors, ideally we would have a way to get them on runtime, but that's not probably possible, especially when not having GTK as a dependency.

Yep, I was just used eyedropper to get the colors of window decoration.

Sharing what I have in case you want to look and possibly contribute your changes updating colors? The repo is now here: https://github.com/FedoraQt/QAdwaitaDecoration I have some other work planned for today and tomorrow, but I can continue with implementing the shadows later. Just saying in case you want to look into implementing the proper colors? Currently our only option is to probably hardcode these colors the same way we have here in QGnomePlatform. Btw. to build the decoration plugin you need QtWayland from Fedora (API changes) which I suppose you use? And use QT_WAYLAND_DECORATION=adwaita to use them. Edit: I see you are probably using Archlinux. The patch I'm talking about we have in qt5-qtwayland is this one. It's needed for proper shadows support.

As I understand it, you decided to rewrite the scenery plugin from scratch.

It's not completely from scratch, I just took decorations from here and simplified many things and dropped dependendencies on Adwaita-qt and Gtk.

I can help with the colors, I'll try to look for the original stylesheet and take the colors from there, although I doubt they will be different from the ones I got from the color picker. However, I still have to figure it all out a bit.

Taking the colors using the color picker is fine for now, but I would like to just know where these are defined so we can keep these in sync and watch for changes. I would do it myself, it's not a big task, I just thought you want to help and don't want your effort you put into this change being useless.

Regarding the last two paragraphs, as I understand it, I need a new patch for qt5 to build this project or was it just a clarification? I actually use arch linux.

Yes, you need that patch for Qt5. I didn't bother with all the ifdefs we have here in QGnomePlatform as I'm targetting Fedora (where that patch is included) and when potentially upstreaming this, there won't be a Qt 5 version.

grulja commented 1 year ago

QGnomePlatform is now unmaintained and not actively developed. For decoration improvements please submit your change to https://github.com/FedoraQt/QAdwaitaDecoration instead.