TWin: Improve Active Screen Borders, add Active Corners support and Aerosnap #331
Merged
blu.256
merged 1 commits from feat/twin-electric-improvements
into master
11 months ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'feat/twin-electric-improvements'
Deleting a branch is permanent. It CANNOT be undone. Continue?
This PR is intended to improve the existing "Active Borders" feature of TWin.
Right now, when you activate "Active Borders", you can switch between desktops by pushing your mouse cursor against one of the desktop's four borders. The two options you have (apart from disabling the feature) is to do this always or only when you are moving a window. The option is accessible in the "Window Behaviour" options dialog, "Advanced" tab.
This PR:
Backports a cleaner implementation of this feature from the KDE 4.0 version of KWin. It works exactly the same, if not a little faster (might be just my impression), but the code is better maintainable and less repetitive. It also provides us with electric corners support, although it is not used yet (but we might make that user-configurable). This introduces a couple of public API changes in the Client class, but I don't think this will break any applications.
Building on the previous changes, I have backported the quick tiling feature (also known as aero-snapping), which allows you to maximize a window to half of the screen by dragging it into a screen border (an option to maximize window when dragging the window towards the top border is also available). So the "active border" feature gets a new mode, the quick tiling feature, as seen in many popular DE's out there.
Adds an option to automatically restore original window size when dragging a tiled or maximized window.
Brings the tiling behaviour closer to the behaviour of existing TDE features.
Updates/adds documentation for all the above changes.
I have marked R14.2.0 as possible milestone, but if you think it can be merged safely in a maintenance release, then it could go into R14.1.1.Note: electric border is the TWin/old KWin term for active border. We get rid of that in this PR.
Known bugs
Doesn't work with wireframe windows yet (the kind of helper windows that appear instead of a window when you move that window while window updates are disabled for moving windows. This option can be set in the "Movement" tab of the "Window behaviour" dialog).FIXEDSee Michele's comments below.FIXEDTWin: Active Screen Borders and Cornersto TWin: Improve Active Screen Borders, add Active Corners support and Aerosnap 1 year ago4db99653fe
to52c7661129
1 year agoHow to test:
twin --replace &
will do)52c7661129
to76a08deba3
1 year agoTHANK YOU Philippe!!!
This is something I really missed from Windows and that I wanted to implement at some point.
I will test it next week after R14.1.0 is hard frozen and provide feedback (== nagging issue list :-) ) as usual. But wanted to say thanks in advance for this feature!
One initial feedback I can alrady provide is that in TCC, the electric border is called "Active border", so it may be good to use the same terminology in the commit message and rename all the
electric*
stuff toactive*
in the code. What do you think?@MicheleC
I don't see why not. Consistent terminology would be good and "active" better fits the description of the feature, as I plan to add user-configurable actions support for both borders and corners.
Great!
I also have a question: do we currently need to touch the border exactly or is there a band of pixels fom the border within which the "active" functionality works? Like for example if the mouse is within 10 pixed from the border, that is it consider as being "active"? Similar to what we have for windows snapping to border functinoality.
The question is important if you use TDE inside a VM in non fullscreen mode, since being exactly on the border could be a bit challenging.
No, you actually have to "push" into the border for a bit to activate the functionality. I suppose this is to prevent accidental activation.
For this case we could add some other way of activation, e.g. options in the window menu?
Currently there is a timer that can be set, so you have to push against the border for the set time before active border changes desktop (for example).
maybe add a keyboard key that needs to be hold down to activate active border, like for example Shift+mouse near border or something like that. Anyway we can work on this later, after this PR is merged.
Hi Philippe,
I did a first round of testing. Love the functionality and I have several ideas for improvement after we merge this PR. But first, feedback on testing and things to look at.
(minor one) In the TCC --> Desktop --> Window Behaviour --> Advanced, the "Desktop switch delay" bar is now much narrower. Although I like it more this way, it is the only one different from the others (which extend till the right side edge), therefore it seems out of place. Also being shorter, it makes moving the slider more sensitive compared to before.
We can have a separate discussion with Slavek on whether we want to make all the sliders shorter in TCC, but that would be a different issue/PR.
would be good to have a slider to set the delay required for the "tile" functionality, so that a user can set its preferred waiting time.
enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Press "Esc" or right click or middle click with the mouse. I was expecting this to abort the tiling operation, but instead it completes it as if I had released the left mouse button (which I am still holding down).
Not sure if this is something caused by testing inside a VM, can you test and let me know?
enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Move the mouse back away from the border without releasing the left mouse button. I was expecting the "half screen" empty window border to disappear since now the mouse is no longer close to the desktop border, but instead it didn't. Or better if I am very very careful and fast to move to-and-away from the border, the "half screen" empty window border disappear. But if I hover on the border just a fraction of time longer, then it seems to get locked and does not disappear.
Again, not sure if this is something caused by testing inside a VM, can you test and let me know?
"tile" functionality should not work for windows which cannot be maximized or resized IMO. Try KCalc and you will see that tiling it will result in a not so nice window size. Note how KCalc cannot be manually resized/maximized
if you right or middle click on the maximize button of a Konsole window, the window will be maximized in either horizontal or vertical direction (depending on your settings). Dragging the window title bar will move the window but without resizing it to the original size.
The "tile" functionality is not consistent with this behavior. After a window got tiled, dragging the window title bar will result in the window going back to its original size before tiling.
For the time being I think we should make "tiling" consistent with the right/middle click maximize behavior. We can later add an option for both functionalities to let the user choose whether to keep the current size or go back to its original size when a window gets dragged, but that will be part of the enhancement follow up.
Overall, great enhancement and expect several requests for additional functionality after this PR is merged :-)
I can setup testing on a real laptop if needed, in case we are not sure whether testing in a VM causes any strange behaviors
Another issue related to point 6. from previous post is the following. Tile a window to one side (for example right side). Then grab the window border to resize it (for example use the left border). A user would expect the window to expand as it usually happens, but again the window goes back to its original size, which is very annoying.
Hello Michele, thank you for the thorough testing. Unfortunately I'm somewhat busy these days so this PR will have to wait a little. I'll get back to it once I have some time on my hands.
I'd like, as part of this PR, to also implement a way to assign actions to active corners, so it might be a while until the PR is merged. I'll give priority to fixing the existing issues first, of course.
No problem Philippe, that your time. I also have a list of improvements I would like to implement, like for example the ability to tile a window to a quarter of the screen, rather than half.
I suggest we take it step by step. For example we fix up and merge this PR with the current functionality. Then we progressivily add improvements, like the one you mentioned and those that I have in mind.
76a08deba3
to1e67f9971f
12 months agoIssues 1, 2, 3 resolved.
Issues 4, 5 resolved.
Well, it is consistent with KWin and my experience with other window managers.
Agreed. I'll just comment out the relevant code for now. I prefer this behaviour, though, and think that it should be the same for maximized windows, too.
Great, I will do a new test this week and feedback as usual.
Hi Philippe,
I did a new round of testing.
a. referring to items 1. and 2., I assume the "Border activation delay" is now common to "Switch desktop" and "Tile window", depending on which one is selected. Am I right? If that is the case, it is fine.
Aestetichally, I suggest a few tweaks. First add a title label to the 3 radio buttons, something like "Function". Second, add a small vertical spacer before "Border activation delay" label. Third: move "Hide utility window..." checkbox inside a groupbox, to make it similar to the rest of the page. What do you think?
b. regarding item 3: ESC function is now ok. The function of right click is also ok as you implemented it. Initially I thought we should abort the move, but other drag-n-drop operations work in a similar way with right click mouse button completing the move.
c. item 4: (this may be caused by screen refreshing within my VM, so please read and double check on your side). There is improvement but it still need more work, IMO. When the mouse touches the border, the black "half screen" border shows up. If we move the mouse away and release the button, the border disappear but only after releasing the mouse button. If we don't release the button, the border stays there all the time. Moreover if we move the window far enough before releaseing the border, it seems the "black border" does some funny things (please check and let me know if it happens on a real pc too, or if it is just my VM playing up)
d. item 5: ok
e. item 6: still not ok. After tiling a window and dragging/resizing it, the window still goes back to the original size before tiling.
In summary:
PS: I am already using this functionality daily, even with the mentioned issues. What a great one!
Hello, Michele! Let's see what we have here...
Yes, it is. Code-wise it already was, BTW.
I agree, and actually I wanted to propose that the active border settings be moved into a separate tab, seeing that I'd like to add configurable active borders in next PR. I'll make the changes you requested, but before creating that separate tab I want your feedback first.
I can reproduce this. I think it has something to do with 3d38021d65 because it behaves correctly when you uncheck "Show contents when moving windows" in the "Moving" tab.
I haven't fixed that one yet `:)
Great! I also use it daily and it's a great time saver :)
3d38021d65
to834d1988e7
12 months agoI've made the suggested aesthetical improvements in the options UI and resolved item 6 by adding an option to turn restoring original size on/off and preventing restoring the original size when resizing the window.
Great, will do another test either tomorrow or Wednesday and feedback.
Sounds good to me, especially if you already have some ideas in mind for things to add.
Or maybe move the "Shading" part under the "Titlebar Actions" tab and keep the "Advanced" tab for the active border? What do you think? Otherwise the "Andanced" tab would look pretty empty and almost pointless.
Thanks Philippe. I did a new test.
Item 1: the eastetichal changes look good, thanks. Just one you missed is moving "Hide utility window..." checkbox inside a groupbox. The reason for asking this is that with QtCurve theme there is no border around a group box, so that checkbox looks out of alignment and also it is not clear that it is not related to the "active border" settings. Let me know if you need a screenshot of what it looks like.
Item 6: great, it works well with the new checkbox. That is a good improvement, thanks. As future enhancement (after this PR) it may be worth adding the possibility to control the behavior with a modifier key as well, but we can discuss this in future
Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible?
Other than the points above, the PR seems to be mostly ready (haven't looked at the code yet though).
The Advanced tab actually is pointless, as:
So I'll try to do exactly that.
Will do.
That's a really nice idea!
Strange, I have already pushed a fix that works for me. Have you re-checked with "Display contents in moving windows" on?
Do you think it should target R14.2.0 or the next maintenance releaes? I've assumed the former, but it shouldn't break anything as nothing changed in the public APIs.
Sounds good to me. Do you want to do this as part of this PR or separately after we merge this one?
I must have made a mistake during testing yesterday, because today it is working fine.. sorry for the wrong reporting. So item 4 is also cleared.
it depends on whether there are API/ABI changes or not. If there aren't any changes to that, we can add to R14.1.x since it is a great feature. If it changes API/ABI, we shall target R14.2.0.
Once the PR is ready, I will go through the code and feedback on this
Probably best to keep that for the next PR where I will be adding configurable actions for Active Corners.
It's okay. Nice to hear that one's solved, too :)
There are only changes internal to TWin and not exposed in public headers.
BTW, just noticed that due to Electric* -> Active* renaming some configuration options have been renamed:
Option values of ActiveBorder are compatible with old option values of ElectricBorder. Considering this, it might make sense either to revert renaming in this case or automatically convert these options using the release script.
Sounds good.
By the way, one of the improvement I will suggest is also to be able to tile windows to a quarter of the screen instead of half. Not sure this clashes with your ideas on active corners or if it is the same, but we can discuss after this PR is merged.
There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*.
This way we don't need any conversion script and the update wil be painless for users.
Hi Philippe,
after this is implemented, we can look at merging this PR. Looks like we may be able to add this to R14.1.1 instead of R14.2.0, that would be great.
EDIT: actually one more thing required before the fix. We need to edit the help docbook and add a short description of the new tiling functionality (I always forget about documentation...)
Done.
Done. I also added documentation on the new "Restore size of maximized/tiled windows when moving" option.
Please review and tell me if I can squash+merge it.
A couple of minor adjustments needed, then we can squash, rebase and merge.
/**
* @returns true if a maximized or tiled window should be reset to its original
* size when dragging it.
* @since R14.2.0
@since R14.1.1 if we decide to add to R14.1.x
ActiveBottomLeft,
ActiveLeft,
ActiveTopLeft,
ELECTRIC_COUNT,
Suggest renaming ELECTRIC_COUNT into ACTIVE_COUNT
Yes, missed that, thanks!
@SlavekB seems we don't brake any public API, so we can probably backport this to R14.1.1. What do you think?
Yes, definitely. This looks like a good new thing for R14.1.1.
@blu.256
not sure if you didn't get the previous notification or if you are just busy, so just a reminder :-)
Sorry, I forgot. I'll get to that in the afternoon.
c704628e19
to6fb6faffa3
11 months agoLooks good
Ready for merging!
Don't forget to squash and rebase first.
6fb6faffa3
to31335a04ed
11 months agoMerging and backporting to
r14.1.x
31335a04ed
into master 11 months ago✔️ Done
Reviewers
31335a04ed
.