Skip to content
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

wm: split geometry trigger into size and position #1335

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

Monsterovich
Copy link
Contributor

No description provided.

Copy link
Owner

@yshui yshui left a comment

Choose a reason for hiding this comment

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

even just considering resizing with user interaction, this doesn't work. if you resize from the top-left corner, both (x, y) and (width, height) will change, which will only trigger position change in your code.

@Monsterovich
Copy link
Contributor Author

Monsterovich commented Sep 9, 2024

@yshui

even just considering resizing with user interaction, this doesn't work. if you resize from the top-left corner, both (x, y) and (width, height) will change, which will only trigger position change in your code.

What? It works as it's supposed to, I don't use the position trigger because of the bad behavior, it will probably be more useful in tiling WMs.

2024-09-09_15-10-01.mp4
    { match = "class_g !?= 'xfce4-panel' && class_g !?= 'Xfce4-notifyd'"; animations = (
        {
            triggers = ["geometry"];
            scale-x = {
                curve = "cubic-bezier(0.07, 0.65, 0, 1)";
                duration = "placeholder0";
                start = "window-width-before / window-width";
                end = 1;
            };
            scale-y = {
                curve = "cubic-bezier(0.07, 0.65, 0, 1)";
                duration = "placeholder0";
                start = "window-height-before / window-height";
                end = 1;
            };
            shadow-scale-x = "scale-x";
            shadow-scale-y = "scale-y";
            offset-x = {
                curve = "cubic-bezier(0.07, 0.65, 0, 1)";
                duration = "placeholder0";
                start = "window-x-before - window-x";
                end = 0;
            };
            offset-y = {
                curve = "cubic-bezier(0.07, 0.65, 0, 1)";
                duration = "placeholder0";
                start = "window-y-before - window-y";
                end = 0;
            };
            shadow-offset-x = "offset-x";
            shadow-offset-y = "offset-y";
            placeholder0 = 0.2;
        },
    ) },

I made it so that there is either a change in window dimensions or position, if you need both, then you just need to specify.

triggers = ["geometry", "position"];

@yshui
Copy link
Owner

yshui commented Sep 9, 2024

if you don't even use the position trigger yourself, how do you know if it's working correctly? what is this "bad behavior" you are referring to?

@Monsterovich
Copy link
Contributor Author

Monsterovich commented Sep 9, 2024

@yshui

if you don't even use the position trigger yourself, how do you know if it's working correctly? what is this "bad behavior" you are referring to?

I tested it, it works. The only bad behavior is moving a window from one workspace to another when the window is basically teleported, and I also don't like the animation of moving popups and would like them to appear immediately in the right place.

Like I said, position trigger is rarely needed and is most likely to be useful in very specific cases, like tiling WMs.

@yshui
Copy link
Owner

yshui commented Sep 10, 2024

@Monsterovich the problem is resizing the window from the top-left corner triggers position change, but it should trigger geometry.

@Monsterovich
Copy link
Contributor Author

Monsterovich commented Sep 10, 2024

@Monsterovich the problem is resizing the window from the top-left corner triggers position change, but it should trigger geometry.

@yshui How did you get this behavior, maybe it's the behavior of a particular application? I can't reproduce this on a regular window (e.g. thunar).

2024-09-10_02-18-44.mp4

@yshui
Copy link
Owner

yshui commented Sep 10, 2024

@Monsterovich ah... sorry, i did misread your code.

but there's the other problem: animation interruption doesn't work nicely with this: if you are in the middle of a resize animation, and the position changed, the resize animation will stop and the size jumps to the final size.
which isn't very nice.

right now, with geometry trigger, you will notice if the window geometry changes when it's animating, it will still transition smoothly. i think that would be difficult to implement if we split the trigger.

@Monsterovich
Copy link
Contributor Author

but there's the other problem: animation interruption doesn't work nicely with this: if you are in the middle of a resize animation, and the position changed, the resize animation will stop and the size jumps to the final size.
which isn't very nice.

@yshui This is not true, the transition between animations works just fine.

2024-09-10_13-02-06.mp4

@yshui
Copy link
Owner

yshui commented Sep 10, 2024

@Monsterovich of course it's going to work when you have the same animation for both geometry and position.

didn't you say you don't want animation when you move the window? if so, remove the position trigger, and see how it behaves.

@Monsterovich
Copy link
Contributor Author

Monsterovich commented Sep 10, 2024

@Monsterovich of course it's going to work when you have the same animation for both geometry and position.

didn't you say you don't want animation when you move the window? if so, remove the position trigger, and see how it behaves.

@yshui I deleted it, so what? You can't physically interrupt an animation that lasts 0.1-0.2 seconds, I just wanted to show that the behavior when both triggers are specified is the same as before this PR that's why I set the length to 10s.

Additionally, I can now just specify the position in a separate match with another settings if I need to, in order to try to eliminate the bad behavior of the position trigger that I have. However, in general, it's fine for me to use only geometry without position, I have my use-cases, others have theirs.

@Monsterovich
Copy link
Contributor Author

Monsterovich commented Sep 10, 2024

@yshui In theory, you could add a setting that waits for the previous animation to finish playing, e.g. for those willing to watch the window slowly change for 10 seconds, but is it necessary?

@Monsterovich
Copy link
Contributor Author

@yshui I played around with the config some more, maybe that's what you wanted?

https://www.youtube.com/watch?v=_quxT7xsTO0

That's how I achieved this:

...
            triggers = ["geometry"];
            suppressions = ["position"];
...

@yshui
Copy link
Owner

yshui commented Sep 10, 2024

suppressions = ["position"];

hurmmm, IIUC this means windows will just not react at all if you try to move them when they are in geometry animation? it doesn't feel like the right thing to do.

@Monsterovich
Copy link
Contributor Author

suppressions = ["position"];

hurmmm, IIUC this means windows will just not react at all if you try to move them when they are in geometry animation? it doesn't feel like the right thing to do.

In my opinion, this is the only solution when you don't need a position trigger.

@yshui
Copy link
Owner

yshui commented Sep 10, 2024

ok, might not be a bad idea after all 🤔

@Monsterovich Monsterovich changed the title wm: split geometry trigger into geometry and position wm: split geometry trigger into size and position Oct 20, 2024
@yshui yshui force-pushed the feat-split-geometry-trigger branch from 1c7ded3 to 72f3658 Compare November 14, 2024 11:29
Aliases aren't triggers themselves so their numbering goes beyond
ANIMATION_TRIGGER_COUNT.

Signed-off-by: Yuxuan Shui <[email protected]>
Use a bitflag internally to keep track of which triggers are set.

Signed-off-by: Yuxuan Shui <[email protected]>
@yshui yshui force-pushed the feat-split-geometry-trigger branch from 72f3658 to 29b7282 Compare November 14, 2024 12:59
And keep geometry as an alias for setting both size and position.

Changelog: NewFeature: Separate the "geometry" animation trigger into
"size" and "position" to allow finer-grained control. "geometry" is kept
as an alias for setting both "size" and "position".

Co-authored-by: Monsterovich <[email protected]>
Signed-off-by: Yuxuan Shui <[email protected]>
@yshui yshui force-pushed the feat-split-geometry-trigger branch from 29b7282 to df7037d Compare November 14, 2024 13:03
@yshui yshui merged commit a017833 into yshui:next Nov 14, 2024
15 checks passed
@Monsterovich Monsterovich deleted the feat-split-geometry-trigger branch November 14, 2024 13:27
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.

2 participants