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

[FEAT] Better scratchpad window slide animation. #154

Open
JunaidQrysh opened this issue Nov 28, 2024 · 21 comments
Open

[FEAT] Better scratchpad window slide animation. #154

JunaidQrysh opened this issue Nov 28, 2024 · 21 comments
Assignees
Labels
enhancement New feature or request

Comments

@JunaidQrysh
Copy link

Currently the scratchpad window moves some pixels determined by the default offset value, then moves to special workspace which make the animation feel cutoff.

It will be better to move the whole window out of the monitors edge then move it to the special workspace.

I think the solution is to get the respective at: and size: values from hyprctl command and add them for the default offset value.

@JunaidQrysh JunaidQrysh added the enhancement New feature or request label Nov 28, 2024
@fdev31
Copy link
Collaborator

fdev31 commented Nov 29, 2024

This means an additional call on each hide right...
Is the scratch animation really not looking good for you ? It has been adjusted but the logic is a bit weak, using some delay... so depending on your animations durations it may indeed not look very good.
Are you able to provide a setup reproducing the issue, I am not sure to fully understand your proposition, why would I need to current location of the window?

@JunaidQrysh
Copy link
Author

If a windows has fromTop animation, then get x value from at: and it to x value of size:, that way the window moves completely out of the montor.
I tried implementing it using claud as I don't know python but it was just a mess.

@fdev31
Copy link
Collaborator

fdev31 commented Dec 20, 2024

Isn't it what is already happening, the scratchpad moving out of the monitor? Can you share your configuration and use case so I better understand?

@fdev31
Copy link
Collaborator

fdev31 commented Dec 20, 2024

Are other users experiencing this kind of issue?

@JunaidQrysh
Copy link
Author

JunaidQrysh commented Dec 21, 2024

@fdev31 I implemented the solution myself and here is the before and after:

Before:

Before.mp4

After:

After.mp4

@JunaidQrysh
Copy link
Author

This is the solution, but it has one problem, it does not account for monitor scale, i have 1.2 monitor scale(1600x1000) based on 1920x 1200; therefore the original monitor dimensions are taken instead of scaled making the animations fast for
fromRight and fromBottom type, As you can see in the video.

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        mon_size = [monitor["height"], monitor["width"]] if rotated else [monitor["width"], monitor["height"]]
        mon_size = tuple(mon_size)
        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = []

        if get_animation_type(scratch) in ["fromtop", "fromleft"]:
            for v1, v2 in zip(win_position, win_size):
                scaled.append(v1 + v2)
        else:
            scaled = [m - w for m, w in zip(mon_size, win_position)]

        return cast(tuple[int, int], scaled)

@JunaidQrysh
Copy link
Author

@fdev31 Now the code also takes monitor scale into account. I think you should make a commit if eveything works correctly.

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        mon_size = tuple(
            int(m / monitor["scale"]) 
            for m in ([monitor["height"], monitor["width"]] if rotated else [monitor["width"], monitor["height"]])
        )
        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = []

        if get_animation_type(scratch) in ["fromtop", "fromleft"]:
            for v1, v2 in zip(win_position, win_size):
                scaled.append(v1 + v2)
        else:
            scaled = [m - w for m, w in zip(mon_size, win_position)]

        return cast(tuple[int, int], scaled)

@fdev31
Copy link
Collaborator

fdev31 commented Jan 8, 2025

Hi!
Thank you for the feedback, I guess we have very different animation settings because now I can't see the scratchpads hiding anymore, but indeed it looks good and more responsive (my setup doesn't expose the same behavior as your videos).

One more important remark: it's broken at least when the scratchpad is animated "fromright".
I fixed it by using if get_animation_type(scratch) in ["fromtop", "fromright"]: instead of the code you shared.

I am currently facing some family challenges and don't have much (brain) time for hobbies, I believe more tests are needed before implementing such change (preserve_aspect = true, and the 4 different animations + "not animated" config).

If it looks like no more regression is found, we can consider merging it.
The updated function I used (for reference):

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        mon_size = tuple(
            int(m / monitor["scale"]) for m in ([monitor["height"], monitor["width"]] if rotated else [monitor["width"], monitor["height"]])
        )
        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = []

        if get_animation_type(scratch) in ["fromtop", "fromright"]:
            for v1, v2 in zip(win_position, win_size, strict=True):
                scaled.append(v1 + v2)
        else:
            scaled = [m - w for m, w in zip(mon_size, win_position, strict=True)]

@fdev31
Copy link
Collaborator

fdev31 commented Jan 9, 2025

Did you try setting 'margin' manually? This code breaks too many things but I think you can achieve the same with a custom margin....

@fdev31
Copy link
Collaborator

fdev31 commented Jan 9, 2025

FYI I had to make some fixes (turned to be simplifications) to get the 4 animations working, but it's dropping "margin" support... I guess the default values are not fitting your setup, but on my desktop setup this patch looks really bad:

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = [v1 + v2 for v1, v2 in zip(win_position, win_size, strict=True)]

        return cast(tuple[int, int], scaled)

@fdev31
Copy link
Collaborator

fdev31 commented Jan 9, 2025

A custom offset could be better.... please test if this can fix your setup or if there is a need to find another solution.

@JunaidQrysh
Copy link
Author

FYI I had to make some fixes (turned to be simplifications) to get the 4 animations working, but it's dropping "margin" support... I guess the default values are not fitting your setup, but on my desktop setup this patch looks really bad:

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = [v1 + v2 for v1, v2 in zip(win_position, win_size, strict=True)]

        return cast(tuple[int, int], scaled)

The above code works but only for fromTop and fromLeft

@Sneethe
Copy link

Sneethe commented Jan 12, 2025

Are other users experiencing this kind of issue?

Yes. I find pyprland scratchpad implementation to be far more sluggish than Hyprlands default togglespecialworkspace.
When quickly tapping a bind for togglespecialworkspace, it keeps up with the users input. However with pypr toggle, it will noticeably lag behind.
Interestingly, when turning off animation for both togglespecialworkspace and pyrpland toggle, there is still perceptible lag (albeit it small) between the pyrprland's scratchpad and Hyprlands one.
I would say however, that the current animation is mostly responsible for the overall sluggish experience however.

@fdev31
Copy link
Collaborator

fdev31 commented Jan 13, 2025

@JunaidQrysh any feedback on the usage of the existing configuration options ? (offset, margin, ...)

@Sneethe not surprising that something doing multiple IPC calls to get information before toggle the workspace takes more time than just toggling the workspaces. So comparing X calls + 1 to "just one call" isn't really a fair comparison and will never be the same. In case your system is really slow, some tricks may improve the response time, check https://hyprland-community.github.io/pyprland/Optimizations.html#pypr-command for that matter, maybe it makes a difference on your system. But again, scratchpads implementation is far more complex than a single togglespecialworkspace call and requires multiple interactions with hyprland API.

@JunaidQrysh
Copy link
Author

@fdev31 i think offset and margin are fine, but can your share your hyprland animation config, i dont understand why the code is broken on your setup, do you have multiple monitors? I am still daily driving my own pypr build with zero issues but more smooth experience.

@fdev31
Copy link
Collaborator

fdev31 commented Jan 14, 2025

hum, I think this is really an offset thing... it's only affecting hide and just makes it hide "way more far" than the current code which uses the window height.

With your code, I tried this config on the 4 animation types (to see that half were broken/ animating in the wrong direction):

[scratchpads.term]
animation = 'fromTop'
command = '[term_classed] main-dropterm'
class = 'main-dropterm'
size = '75% 60%'
max_size = '1920px 100%'

and the hyprland anim config:

animations { # {{{
    enabled = yes

    # Some default animations, see https://wiki.hyprland.org/Configuring/Animations/ for more

    bezier = myBezier, 0.05, 0.9, 0.1, 1.05

    bezier = easeInOut,0.65, 0, 0.35, 1
    bezier = bounceOut,  0.57, 1.4, 0.24, 0.95

    animation = fade, 1, 5, easeInOut
    animation = windows, 1, 4, bounceOut, popin 50%
    animation = windowsIn, 1, 7, bounceOut, popin 10%
    animation = windowsOut, 1, 7, easeInOut, popin 80%
    animation = border, 1, 10, default
    animation = borderangle, 1, 8, default
    animation = workspaces, 1, 6, default, slide
    animation = specialWorkspace, 1, 3, bounceOut, slidevert
}

@JunaidQrysh
Copy link
Author

@fdev31 Now the code also takes monitor scale into account. I think you should make a commit if eveything works correctly.

    async def get_offsets(self, scratch: Scratch, monitor: MonitorInfo | None = None) -> tuple[int, int]:
        """Return offset from config or use margin as a ref."""
        offset = scratch.conf.get("offset")
        if monitor is None:
            monitor = await get_focused_monitor_props(self.log, name=scratch.forced_monitor)
        rotated = is_rotated(monitor)
        aspect = reversed(scratch.client_info["size"]) if rotated else scratch.client_info["size"]

        if offset:
            return cast(tuple[int, int], (convert_monitor_dimension(offset, ref, monitor) for ref in aspect))

        mon_size = tuple(
            int(m / monitor["scale"]) 
            for m in ([monitor["height"], monitor["width"]] if rotated else [monitor["width"], monitor["height"]])
        )
        win_position = apply_offset((monitor["x"], monitor["y"]), scratch.meta.extra_positions[scratch.address])
        win_size = convert_coords(scratch.conf.get("size"), monitor)
        scaled = []

        if get_animation_type(scratch) in ["fromtop", "fromleft"]:
            for v1, v2 in zip(win_position, win_size):
                scaled.append(v1 + v2)
        else:
            scaled = [m - w for m, w in zip(mon_size, win_position)]

        return cast(tuple[int, int], scaled)

@fdev31 it tested the above code with your animations and your exact pypr config it still works flawlessly in all animations.

@fdev31
Copy link
Collaborator

fdev31 commented Jan 14, 2025

Maybe I made some mistake when I tested it, I'll give it another try.... but this is breaking compatibility with the margin fallback, probably not a big deal.
Now isn't the best moment for me due to personal matter, but I'll try again this code extensively a bit later (assuming I made some mistake last time), it MUST support rotations & scaling so if there are some adaptations to make you still have time ;)

@JunaidQrysh
Copy link
Author

@fdev31 So i tested the same code with all monitor rotations and it worked BUT you MUST kill pypr & all scratchpads if you change monitor rotation in hyprland config. I think that's why you saw weird animations.

@fdev31
Copy link
Collaborator

fdev31 commented Jan 14, 2025 via email

fdev31 added a commit that referenced this issue Jan 17, 2025
@fdev31
Copy link
Collaborator

fdev31 commented Jan 17, 2025

I added it to the main branch, basic tests showed ok but I didn't test rotation (also combined with scale).

EDIT: not making any release now, but will in a couple of weeks maybe after I can run better tests. Hoping having it on git will raise some eventual issues before that...

fdev31 added a commit that referenced this issue Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants