Skip to content

Conversation

@SleepProgger
Copy link

@SleepProgger SleepProgger commented Jul 26, 2020

Continuation from #228

I switched back to using showNormal and Co. This simplified the code by a good bit and also allows to benefit from possible future updates in QTs showNormal and especially showMaximize functions.

Everything seem to be fine, but further tests especially on other window manager might be a good idea.
Fixed from last state:

  • Restoring maximized windows now works properly.
  • Maximized window size should no be more often correct. (QT itself uses workarounds to get this playing nice with X11)

The only thing i am not really sure about is that i had to remove setAttribute(Qt::WA_X11NetWmWindowTypeDock, true); because that broke showMaximized for some reason.
I don't see any change on my System tho, but this may be different on other systems .?

@githubuser0xFFFF
Copy link
Owner

Thank you for the pull request.

I just tested it on Ubuntu 20.04 with Qt 5.12.8 and it does not work. The maximize button does not do anything. The icon changes but the window does not change. I think showMaximized and showNormal does not work on Linux - at least this is my experience the last time I tested it.

Removing Qt::WA_X11NetWmWindowTypeDock has the negative effect, that a floating widget that is dragged around cannot be dragged outside of the screen boundaries - this feels kind of weird.

@SleepProgger
Copy link
Author

SleepProgger commented Jul 28, 2020

Args, X11 why do you have to be so annoying.

Everything works fine on XFCE from QT 12.5 to QT 15.0 including being able to drag outside of screen boundaries.
I will test and try to get it running on some more window manager.
Please keep this PR open. I haven't given up yet.
If you have the time can you test with newer QT version on your system too ?

Before removing WA_X11NetWmWindowTypeDock showMaximized also did not work on my system btw.

@SleepProgger
Copy link
Author

SleepProgger commented Jul 29, 2020

I could reproduce your described behavior with Mate 1.16.2.
After changing the windowType from Tool to Window it works on both systems i tested on.
It seems as Tool windows simply can't be resized under some manager.

I am still not sure why temporary setting WA_X11NetWmWindowTypeDock breaks showMaximize on XFCE, but tbh, i am not a fan of that behavior anyway. It allows to drag a floating windows title under desktop panels which can lead to windows where the titlebar is completely hidden.

Please let me know if it works for you and if you insist on the WA_X11NetWmWindowTypeDock flag (In which case i'd investigate some more why that breaks on XFCE).

@githubuser0xFFFF
Copy link
Owner

githubuser0xFFFF commented Jul 29, 2020

@SleepProgger I tested your latest changes on Ubuntu 20.04 (Qt 5.12) and Ubuntu 18.04 (Qt 5.9) and cannot see any difference: the maximize button is not working and it is not possible to drag the floating widgets out of the screen borders which feels kind of wired. I do not insist on WA_X11NetWmWindowTypeDock. If you think it is o.k. for Linux Users if dragging of floating widgets stops on screen boundaries and if this is a nice user experience then I'm o.k. with it. I hope all other Linux users are also o.k. with this change.

Thanks for all your efforts - these are the things that drive me crazy on linux. Maybe you can check the QDockWidget source code to see how Qt handles this problem?

@SleepProgger
Copy link
Author

Ahh annoying, had my hopes up for this one. Will try to get it running tomorrow on ubuntu too.

The main problem with "WA_X11NetWmWindowTypeDock" is that i really don't see why it breaks on XFCE AFTER disabling it again.
It doesn't seem to have any effect on windowtype nor on windowflags, but maybe i'll find a solution to this.
Enabling/disabling it via define or setting would be nice tho in case we can get it working.

On my Mate test system QDockWidget can't be maximized too (tested with wmctrl) which makes sense since they are of the tool type too.
Additionally floating QDockWidget also can't be dragged outside of the screen borders.
So sadly nothing to learn there i fear.

Quick question tho: ADS is extending QDockWidgets for the FloatingContainer. Is there a reason for this i missed or is this just like it always was and never was changed ?

@githubuser0xFFFF
Copy link
Owner

githubuser0xFFFF commented Jul 29, 2020

@SleepProgger I had trouble to get the floating widgets working on Linux without QDockWidget. Initially the code was the same on Windows and Linux - QWidget. But with QWidget I could not get the floating widgets to work properly. If I remember correctly I had issues with the stacking, that means a dragged widget appeared behind the main window or above the drop icons, so it was not possible to see the drop indicators anymore.

Then I switched to QDockWidget because I thought: they made docking work on Linux and Windows so it should also work for me. And indeed, switching to QDockWidget fixed all my troubles I had so far. But maybe there are other X11 window flags that could also fix this. Because I mainly work on Windows and because my application that uses ADS also runs on Windows, I did not spent too much time on the Linux implementation. I also only use Ubuntu here, so this is my only Linux test environment.

@SleepProgger
Copy link
Author

I see. Thanks for the info.
I just noticed i messed it up when cleaning the code.
Let me know if it works now on ubuntu. Take your time tho, i'll be off to bed.

(Trivia (kinda): Strangely it doesn't work when using xForwarding on the XFCE machine to itself but works with xForwarding from the XFCE machine to the mate one oO)

@githubuser0xFFFF
Copy link
Owner

@githubuser0xFFFF Now the maximize button works - thank you. If I set the attribute WA_X11NetWmWindowTypeDock static, then the maximize button does not work. But in the docking system the attribute is set dynamically. That means , it will be set if the widget is dragged around to enable dragging outside of the screen borders and it will be cleared if dragging finishes. This enables maximizing.

Therefore I would like to keep WA_X11NetWmWindowTypeDock in the code. At the moment for me it looks like something is broken in XFCE.

@SleepProgger
Copy link
Author

SleepProgger commented Aug 4, 2020

Ok it took me some time and required to dig into xcb and X specs (no fun), but here we go...

There are some problems with using Qt::Window instead of Qt::Tool.

  1. Each window has an entry in the Taskbar and Pager
  2. Windows are not automatically always on top of the Main window
  3. Windows are not being minimized when the main window is

I think i solved all of these issues.
Currently on another branch ready to test: https://github.com/SleepProgger/Qt-Advanced-Docking-System/tree/native_floating_titlebar_on_linux

I also completely got rid of the custom titlebar and used the native one. This means whatever the windowmanager of the user supports is now supported (same as on the other systems).
This is done by listening to the moveEvent and relying on the fact that all window manager seem to activate a window as soon as dragging is finished. (Potential this also could work on the other systems).
There is no code for linux anymore to abort dragging () because every WM i tested supported that out of the box for all windows anyway.

Tested on different versions of Mate (Based on GNOME2), Ubuntu 20.04 (GNOME3), Kubuntu 20.04 (KDE) and XFCE plus X forwarding on all of them.
I think it should be ready to go, and if you are ok with the general direction i'll move that branch over to this PR.

Additional info about the changes:
I had to manually use the xcb functions to communicate with the X server since the required feature are not implemented in Qt (Or are buggy). (Utility functions are in ads_globals, but let me know if i should move them)
This means that for Linux systems the X11Extra module (QT += x11extra) is now required. Even when using Wayland, which might be a problem, but it should still work,
X11Extras is only required to get the xcb connection so there might be a way around this dependency, although i didn't found a way.

I solved the taskbar/pager issue by sending _NET_WM_STATE_SKIP_TASKBAR and _NET_WM_STATE_SKIP_PAGER window states. These are sadly ignored by Qt for whatever reason, so they will be lost upon show. For this reason i overwrote show to add them again. I see no way to get around having to use xcb directly for this.

I solved the "always on top" issue by setting all floating windows to be always on top as soon as the main application window gets activated and remove it again when deactivated. This leads to it behaving like a Qt::Tool window.
Because of how Qt itself sets windows flags i could not use the setWindowFlags functions for this but had to send it directly via xcb.

By simply hiding all floating windows when the main WIndow is being minimized that one was easy to solve.
I had to introduce an overwritten hide function in FloatingDockContainer tho to avoid triggering hideEvent.
The implementation might have a potential race condition, but i have not enough knowledge with Qt to definitely say that.

@githubuser0xFFFF
Copy link
Owner

Wow, that sounds really great and like a lot of work. I'm on holidays right now for the next two weeks and will review and test the patch as soon as I'm back.

Thank you

@SleepProgger
Copy link
Author

I am planing on using this library, so its in my own interest too ;)
There will be likely other PRs/Issues from me in the future.

Enjoy your holiday

@githubuser0xFFFF githubuser0xFFFF merged commit 13545ff into githubuser0xFFFF:master Aug 18, 2020
@githubuser0xFFFF
Copy link
Owner

I just merged your pull request with some changes. I added support for styling the maximize button via CSS and provided some default icons.

Thank you.

@SleepProgger
Copy link
Author

SleepProgger commented Aug 18, 2020

@githubuser0xFFFF Wait, no. This branch is messed up.
It shows the floating windows in the taskbar and doesn't have the always on top feature and other issues.

https://github.com/SleepProgger/Qt-Advanced-Docking-System/tree/native_floating_titlebar_on_linux
Is the branch i am currently working on, and even that one still has some issues on Kubuntu (although i should be able to fix them the coming days) and also Ubuntu (others tho).
It is not in this branch because i wanted to get your ok first with using decorated windows instead of doing our own titlebar which leads to a bunch of issues with different window managers.

I'd suggest reverting this merge to be honest.

@githubuser0xFFFF
Copy link
Owner

Reverted the change. Feel free to open a new pull request if you are ready.

@githubuser0xFFFF
Copy link
Owner

I just tested your implementation with native linux titlebar. I looks very promising - I like it a lot. I hope you can sort out all issues quickly.

@SleepProgger
Copy link
Author

Thanks.
Sadly i noticed that KDE (ie Kubuntu mainly) does not send continuous move events while being moved. This means the way i detected moving floating windows won't work on KDE (works on Gnome3, Mate, Xfce and OpenBox).

So atm. i see 3 possible ways:

    1. Use the custom (ie not native) titlebar as currently on master and live with the fact that some features of the windowmanager won't work, like docking to edges and such.
    1. Run a tight loop in another thread to observe mouse movement and clicks system wide and emulate NonClientAreaMouseButtonPress and NonClientAreaMouseMove. This might be the only way to solve this reliable on ALL X11 window manager. Its for sure iffy though.
    1. Investigate if there is some flag that could be set to archive the same behavior as on all other (tested) window manager.

If 3. fails i'd suggest going with 1. which would be done in some days and i would likely just move the relevant stuff from the native_titlebar branch into this on.

Did i mention that the whole X11 WindowManager area is a freakin mess ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants