Nyre221 / Kiview

GNU General Public License v3.0
50 stars 1 forks source link

KDE Plasma 6 compatibility #1

Closed RedBearAK closed 3 months ago

RedBearAK commented 6 months ago

@Nyre221

You sent me a link to Kiview and I had a bunch of life things get in the way of trying it out. I just installed it in a virtual machine running KDE Plasma 6 beta. Got through the instructions to build and install the Flatpak, but there were a number of problems.

To be clear, I didn't actually expect it to work just yet on Plasma 6. So I'm just reporting what I saw for your information.

After the install, the app shows up in the app launcher menu, and it opens, but the "flatpak run" command from the README claims that the app is not installed.

When the window opens, it says "No active dolphin window found", even if there is a Dolphin window open.

The "How it works" which appears to be a link does not respond to a mouse click in any way.

That's the end of the story for now, since it won't connect to the Dolphin window, and doesn't appear in the Dolphin context menus.

I'll also be trying it soon in a VM where I have Plasma 5. But the install process takes quite a while on my connection.

If you need a way to test things on Plasma 6 I recommend grabbing the latest KDE Neon Unstable ISO and installing it in a VM, it's one of the simplest ways to get Plasma 6 out of the box, and the debugging output for things like KWin scripts works well enough that I was able to get a couple of KWin scripts ported to Plasma 6 without much trouble.

Happy New Year! 🎉

Nyre221 commented 6 months ago

Hi, thanks for trying to install it and reporting the problem to me.

After the install, the app shows up in the app launcher menu, and it opens, but the "flatpak run" command from the README claims that the app is not installed.

Very strange, the .desktop should contain the same command: flatpak run *****

When the window opens, it says "No active dolphin window found", even if there is a Dolphin window open. The "How it works" which appears to be a link does not respond to a mouse click in any way.

I don't think qdbus is fully functional on Plasma 6 at this time.

and doesn't appear in the Dolphin context menus

I haven't created the action for the context menu at the moment.

If you need a way to test things on Plasma 6 I recommend grabbing the latest KDE Neon Unstable ISO and installing it in a VM, it's one of the simplest ways to get Plasma 6 out of the box, and the debugging output for things like KWin scripts works well enough that I was able to get a couple of KWin scripts ported to Plasma 6 without much trouble.

Thanks for the advice!

You sent me a link to Kiview and I had a bunch of life things get in the way of trying it out.

No problem, there is no rush and, as I already said, you are not obligated to help me.

Happy New Year! 🎉

Happy New Year to you too :)

RedBearAK commented 6 months ago

I don't think qdbus is fully functional on Plasma 6 at this time.

About that.

Neon for some reason doesn’t come with qdbus installed. Instead it has gdbus and of course dbus-send. You’ll either need to put together a generic method to issue the necessary D-Bus commands using whatever tool is available, or make sure qdbus is installed as a required component. I did the former in my installer. GPT can help translate the syntax. An app called d-feet can help also. It’s a GUI app for exploring the D-Bus interface structures.

Nyre221 commented 6 months ago

@RedBearAK

The shortcut part and dbus calls are done via C++ libraries: https://github.com/Nyre221/Kiview/blob/master/src/dolphinbridge.cpp

What I was referring to is that maybe the plasma developers didn't restore all the functionality of the applications ported to qt6 and dbus only partially works (maybe the reason why qdbus is not pre-installed). I say this because I had already tried a plasma version (about one/two months ago) built with qt6 and some things weren't working.

d-feet

Thanks, I didn't know it existed. Qdbusviewer is a bit slow to use and having a more modern alternative could help me.

RedBearAK commented 6 months ago

@Nyre221

You know what? The link does work. I think the actual problem is that it wasn't bringing the browser window to the front, for some reason. I looked at the tabs and realized there were half a dozen tabs with the Kiview repo open. So it just appeared that nothing was happening, because the browser (Firefox) window was hidden behind the maximized terminal window, behind the Kiview window.

Also, I'm seeing "Kiview (nightly)" in the "Open With..." submenu, and sometimes on the main context menu, but just not for folders. When I use that, it appears to be working just like the old stuff.

OK, so I got a little confused about whether installing qdbus-qt6 was helping, but I don't think it made a difference. What does make a difference is A) having a Dolphin window open when starting KiView from the launcher menu, and B) having something selected in the Dolphin files pane, otherwise the KiView window complains about an invalid path.

So in general it seems to be functional in Plasma 6, there's just a little awkwardness about what it does depending on how it gets started, and I was looking for it in the wrong place in the context menus.

I hadn't actually attempted to set up a shortcut for it yet. Guessing it would have to run the whole "flatpak run" command this time around.

Speaking of which, this does work now:

flatpak run com.nyre.kiview

I did shut down the VM and go do other things, so this was a fresh start. That might have something to do with it behaving better this time around. Can't say for sure.

RedBearAK commented 6 months ago

Setting a global shortcut to run the Flatpak command does not appear to have the same kind of filtering that your old script had. It will bring up a KiView window regardless of whether Dolphin has the focus or not. I remember your script would prevent the window from appearing if Dolphin wasn't actually open.

I seem to see things in the C++ link you posted earlier, about checking for the active window, so maybe it is running into some API changes in Plasma 6 that changed the D-Bus stuff. I had to do some abstracting in order to make a KWin script compatible with Plasma 5 & 6, due to some API things being renamed. Also around obtaining the active window and similar. But I can't say for sure how relevant that is to what your code is doing. The KWin scripts are of course JavaScript.

Nyre221 commented 6 months ago

You know what? The link does work. I think the actual problem is that it wasn't bringing the browser window to the front, for some reason. I looked at the tabs and realized there were half a dozen tabs with the Kiview repo open. So it just appeared that nothing was happening, because the browser (Firefox) window was hidden behind the maximized terminal window, behind the Kiview window.

Plasma 6?

Also, I'm seeing "Kiview (nightly)" in the "Open With..." submenu, and sometimes on the main context menu, but just not for folders. When I use that, it appears to be working just like the old stuff.

To make the option appear in the dolphin menu I tried to associate the application with the various file types instead of having to create an extension for dolphin, but unfortunately it is not an ideal solution and brings several problems.

It's probably best to do it the same way I used for QuickView and give directions on how to install the plugin if the user wants it (Since I want to use flatpak it's best to avoid installing things via kiview outside of the sandbox).

Setting a global shortcut to run the Flatpak command does not appear to have the same kind of filtering that your old script had. It will bring up a KiView window regardless of whether Dolphin has the focus or not. I remember your script would prevent the window from appearing if Dolphin wasn't actually open.

hmmm, here I am undecided on what is best to do:
I thought it might be confusing for the user to try to open the application via the launcher and have no indication of why the window is not shown, and that's why I decided to display the message and leave the window open.
This also has the advantage that it is easier to report errors if the window does not open even if it should.

What do you think about it?

I seem to see things in the C++ link you posted earlier, about checking for the active window, so maybe it is running into some API changes in Plasma 6

The application is still in Qt5 (for now) and therefore there should be no such problems.

If you're wondering why I chose Qt5: More material, more guides, easier to develop without changing distribution on my PC (a VM would have slowed me down too much), etc.

I can make the switch to Qt6 later without too much trouble.

I really appreciate your help :)

RedBearAK commented 6 months ago

@Nyre221

Plasma 6?

Yes, all of this was in the Neon Unstable VM.

To make the option appear in the dolphin menu I tried to associate the application with the various file types instead of having to create an extension for dolphin, but unfortunately it is not an ideal solution and brings several problems.

I figured. Guessing it shows up immediately outside of the sub-menu for file types that don't have a default app associated. It is a little less straightforward than the previous "plug-in" approach that left it in a consistent place in the context menu.

I thought it might be confusing for the user to try to open the application via the launcher and have no indication of why the window is not shown, and that's why I decided to display the message and leave the window open. This also has the advantage that it is easier to report errors if the window does not open even if it should.

I had the same thought, and it's not necessarily a bad thought. The way QuickView worked with the global shortcut was after all kind of a hack, pretending that it was a shortcut only for Dolphin. The fact that you can't integrate more deeply with Dolphin is as always not something you've caused or can do much about, without literally becoming part of Dolphin's base code.

The separate app approach does react when you go select something in a Dolphin window while a blank KiView window is open, so its behavior is just as comprehensible as the QuickView hack. But if there is a way to set it up so that it kind of works the same way QuickView worked, I think some users would follow those instructions.

One thought is avoiding installing or perhaps removing the desktop file (if that's just an automatic part of the build/install process) that makes KiView visible in the app launcher menu, since launching it that way doesn't make all that much sense. In which case users would be pushed more toward making a shortcut, setting up a plug-in, or using the context menu to get to the previewer. Since it really is still tied to Dolphin, I think that would make sense?

Or, when the app is called up from the desktop launcher, maybe just have it display a message about how it is intended to be called from the Dolphin plug-in, with a link or a button that would help with installing the plug-in, and instructions for how to set up the shortcut to call the plug-in, all right there in the window.

I'm sure yours is not the only Flatpak in existence that isn't really meant to be an independent app.

The application is still in Qt5 (for now) and therefore there should be no such problems.

But Plasma and all the apps are now Qt6. I have no idea if that's having any effect.

Plasma 6 is definitely still lacking in the documentation department. I had to get on the KDE discussion forums to figure out how to port the KWin scripts to new API calls, and the only reference for the API calls was some source code somebody linked to.

But apparently in the KDE world they have a tendency to want users to move on to new releases of KDE quickly, and that's going to include Plasma 6, so I don't expect to see many distros trying to hang on to Plasma 5 desktops in the same way as some distros are still stuck on GNOME 40.x (like AlmaLinux 9). A lot of KDE fans are already trying to live in the Plasma 6 beta because of all the fixes it brings, to Wayland scaling/font rendering and such.

Nyre221 commented 6 months ago

I could add a parameter (-s: shortcut) to add in the shortcut command in order to understand the way in which it is launched and consequently make the window disappear in case it is not started with the launcher.

But Plasma and all the apps are now Qt6. I have no idea if that's having any effect.

The only part that must not change is the name of the dbus interfaces. I don't think the rest causes any problems.

Frankly, the part that bothers me the most is trying to make it an official app. there's a lot of stuff to read and apparently I even need a sponsor... I also have to use githlab/kdebugs report (I don't remember the name) and it's a lot of stuff to deal with.

I have already lost two days (before taking a break) trying to put it on kdeapps (from the documentation it is the first step) and I have encountered problems because they are closing the repository and therefore I have to do it another way (the documentation is not updated)

Nyre221 commented 6 months ago

I could add a parameter (-s: shortcut) to add in the shortcut command in order to understand the way in which it is launched and consequently make the window disappear in case it is not started with the launcher.

I modified the code and now it behaves as it should.

I haven't tried the flatpak package yet.

RedBearAK commented 6 months ago

I could add a parameter (-s: shortcut) to add in the shortcut command in order to understand the way in which it is launched and consequently make the window disappear in case it is not started with the launcher.

Seems a decent, and simple, idea.

I have already lost two days (before taking a break) trying to put it on kdeapps (from the documentation it is the first step) and I have encountered problems because they are closing the repository and therefore I have to do it another way (the documentation is not updated)

Trying to get anything into a project that's so big and so old is rarely a cakewalk.

I haven't tried the flatpak package yet.

Drop a note when I should attempt another build with a fresh clone. No hurry at all.

Nyre221 commented 6 months ago

you can try if you want. just add "-s" to the end of the command.

Thanks.

On Wed, Jan 3, 2024, 2:36 AM RedBearAK @.***> wrote:

I could add a parameter (-s: shortcut) to add in the shortcut command in order to understand the way in which it is launched and consequently make the window disappear in case it is not started with the launcher.

Seems a decent, and simple, idea.

I have already lost two days (before taking a break) trying to put it on kdeapps (from the documentation it is the first step) and I have encountered problems because they are closing the repository and therefore I have to do it another way (the documentation is not updated)

Trying to get anything into a project that's so big and so old is rarely a cakewalk.

I haven't tried the flatpak package yet.

Drop a note when I should attempt another build with a fresh clone. No hurry at all.

— Reply to this email directly, view it on GitHub https://github.com/Nyre221/Kiview/issues/1#issuecomment-1874753627, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY2YMIWD3T2ZYJTXD57ZY63YMSYYRAVCNFSM6AAAAABBJ3R6MWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZUG42TGNRSG4 . You are receiving this because you were mentioned.Message ID: @.***>

RedBearAK commented 6 months ago

So, what I did wrong earlier was I didn't delete and re-create the global shortcut. Apparently the edit button only allows changing the displayed name of the shortcut, not the command. You have to make a new one to change the command.

I did that, and also tried it in the terminal. Now I see what you're going for. The window will still appear if launched from the app menu, but if you add the "-s" to the shortcut command it will first check for Dolphin being the active window. So it acts like the old Quick View shortcut script.

Seems to do what you intended.

Launching from the app menu also works fine as long as Dolphin just happens to be the foreground app while you're launching KiView. Of course.

Perhaps it would be clearer if the "No active Dolphin window found" instead said "The focused window was not a Dolphin window". To my mind, that cuts right to the heart of why the KiView window may not be displaying anything when it opens. Users may think that a Dolphin window in the background is indeed "active" but that is a different meaning of the word. It needs to have the keyboard focus.

That's just a semantic issue that may not really matter.

Nyre221 commented 6 months ago

Thanks for testing the flatpak version!

Perhaps it would be clearer if the "No active Dolphin window found" instead said "The focused window was not a Dolphin window".

I might change the message but I don't think it's very important now.
Users will still click on "How it works" and after that they will no longer see the message if they use the shortcut or the dolphin plugin.

what do you think about the startup speed of kiview?

I'm afraid that the flatpak package is slowing down the loading speed too much and making kiview look worse than the python version.

RedBearAK commented 6 months ago

I'm afraid that the flatpak package is slowing down the loading speed too much and making kiview look worse than the python version.

Yeeeaaaaahhhh... [Lumbergh holding coffee GIF]

In the VMs I'm working with, it seems maybe a little slower than the Python version. Not enough that I'm like "Argh, this is too slow!" But it certainly isn't faster than the Quick View implementation. Though I have not tried to test them side by side in the same VM. It's sort of 🤷🏽‍♀️ .

One wonders if the KiView Flatpak process could sit around in the background and just pop up the window when called, or something like that. That would have to have some sort of chance of being able to show up quickly.

Nyre221 commented 6 months ago

One wonders if the KiView Flatpak process could sit around in the background and just pop up the window when called, or something like that. That would have to have some sort of chance of being able to show up quickly.

This would mean putting some sort of service that starts on startup and I'm not sure if it's possible to do that with flatpak.

I think a better way is to provide native packages and thus get rid of the slowness of flatpak. The only problem is that providing packages for every distro is not easy.

Consider that the non-flatpak version opens much faster than the python one. It's flatpak that slows down the startup of every Qt and Kde application (doesn't seem to be the case for gtk ones) and I don't know if there's any reason or solution.

RedBearAK commented 6 months ago

This would mean putting some sort of service that starts on startup and I'm not sure if it's possible to do that with flatpak.

I was thinking more of the app having a background stub that would stick around after the first run and just pop open new windows. Closing the preview window wouldn't close out the whole app. Don't know any details about how you'd do that or whether it would be easy to do with a Flatpak app. But I'm pretty sure I've seen Flatpaks that are designed to sit in the indicator tray, basically as a background app. Having a tray icon and making the persistence option might not even be the worst idea.

I think a better way is to provide native packages and thus get rid of the slowness of flatpak. The only problem is that providing packages for every distro is not easy.

There is the openSUSE Build Service, but I have no understanding of how to set up a project there to provide multi-distro package formats. I'd try to massage the Flatpak into something a little faster somehow, if the startup time is bothering you. It's not nearly as bad as I was expecting, actually.

It's flatpak that slows down the startup of every Qt and Kde application (doesn't seem to be the case for gtk ones) and I don't know if there's any reason or solution.

That seems worth asking about in the Flatpak development repo, if you really think the GTK Flatpak apps start up faster for some reason. I can't say that I've noticed anything like that. But I don't pay that much attention to app startup times.

Nyre221 commented 6 months ago

The problem with leaving the application in the background is that the first startup will always be slow.

For me it's not too much of a problem, it's just that it might bother users.

I know about open build service but I don't understand much about how it works. maybe I could use Appimage in the future and fix the slowness issue if it bothers users (not possible now).

Anyway, thanks for the suggestions and help

Nyre221 commented 3 months ago

@RedBearAK Hi, I ported Kiview to Qt6 and I also wrote the requirements in requirements.txt if you're interested in trying it out.

For now I think I'll put the flatpak version aside and try using OBS to create packages for the various distributions.

The Qt6 version temporarily uses QtWebView for viewing videos due to some distributions that do not provide a sufficiently updated version of gstreamer (Kde neon ubuntu 22.04) and this is why the video player is not great at the moment.