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.

Sign up for GitHub

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

Jump to bottom

feat(animation): bring in changes from #597 #685

Merged
merged 1 commit into from
Jul 10, 2024
Merged

feat(animation): bring in changes from #597 #685

merged 1 commit into from
Jul 10, 2024
+845 −11

Conversation

LGUG2Z
Copy link
Owner

@LGUG2Z LGUG2Z commented Feb 25, 2024

This commit contains all the changes of #597 to make it easier to rebase with the latest changes on master post-v0.1.21.

@LGUG2Z LGUG2Z mentioned this pull request Feb 25, 2024
Closed
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 2 times, most recently from 4a8f15a to b6dde25 Compare February 27, 2024 05:56
@raggi raggi force-pushed the feature/animated-move branch 2 times, most recently from 2b32c12 to e618079 Compare February 27, 2024 06:03
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 3 times, most recently from 437aaed to cb35eeb Compare March 26, 2024 00:55
@krshrimali
Copy link
Contributor

Hi, @LGUG2Z - I understand you must be filled with work around other stuff, was just wondering on the progress with this? Please let me know if you need some help with it and I'll be happy to help wherever possible.

Animations in Komorebi would be great to have! 🎉 Thanks for the work, @raggi and @LGUG2Z 🚀

@raggi
Copy link
Sponsor Collaborator

raggi commented Apr 12, 2024

The main challenge we've seen in testing is that the animations cause chrome & cef windows to render in the wrong place - they start to render outside of their client rect (which isn't visible). This is sort of a chrome bug, but one that the animations robustly trigger.

I was experimenting with some other ways to do some of the actions that Komorebi takes recently, and they seem to work ok on Windows 11 so far, and might help with this problem - specifically avoiding attaching external input handler threads to the processes during window moves and focus events - but this relies on behavior we're not supposed to be allowed to do - lots of testing required.

@LGUG2Z LGUG2Z force-pushed the master branch 3 times, most recently from 11675ef to 6fe4661 Compare April 15, 2024 15:36
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 5 times, most recently from 71c6569 to 836cf66 Compare April 26, 2024 01:32
@renhiyama
Copy link

@raggi would we have to wait for this chrome bug to be fixed before getting this pr merged? if yes, does it affect only google chrome or any other derivatives like edge and vscode, discord, etc ?

I cant wait for komorebi to have animations just like on hyprland, it's the last missing piece of software I needed to make my workflow on windows same as on linux :)

@LGUG2Z
Copy link
Owner Author

LGUG2Z commented Apr 28, 2024

This will probably be released as an "experimental" option (ie. you may have to do a hard restart of apps impacted by bugs and lose data if you can't save beforehand) in v0.1.26; hopefully someone will want to stabilize it enough that they'll drive themselves crazy finding a way to fix these bugs on our end vs. waiting for Electron apps to have this fixed via Chromium. 😅

@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 6 times, most recently from 306513f to 9af834f Compare May 1, 2024 16:58
@LGUG2Z LGUG2Z force-pushed the master branch 2 times, most recently from 2e454a3 to 8176849 Compare May 15, 2024 16:52
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 4 times, most recently from 9fd0067 to b9aef9e Compare June 21, 2024 16:34
@DamianK
Copy link

DamianK commented Jun 21, 2024

Adding a very nice demo of the the current state of this PR from AmN on Discord:

Recording_2024-06-20_033025.mp4

  • Borders are disabled on komorebi and cute-borders is being used to modify the native Windows accent colours

nice!

@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 2 times, most recently from 5fcee6e to 7732e7f Compare June 26, 2024 18:46
@LGUG2Z LGUG2Z force-pushed the master branch 7 times, most recently from 18299c9 to 6fa3f8d Compare July 4, 2024 20:55
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 2 times, most recently from a7f158a to e27da73 Compare July 4, 2024 22:19
@LGUG2Z LGUG2Z force-pushed the master branch 3 times, most recently from a25e7e3 to 2a67c9c Compare July 5, 2024 17:52
@LGUG2Z LGUG2Z force-pushed the feature/animated-move branch 2 times, most recently from b84a244 to de8b201 Compare July 5, 2024 18:19
@LGUG2Z
Copy link
Owner Author

LGUG2Z commented Jul 5, 2024

I think we are at the point where this can now be merged into master and have appropriate documentation added outlining this as an experimental feature with the usual "This feature is not considered stable, and you may encounter visual artifacts from time to time" blurb.

I'll take some time this weekend to update the documentation and hopefully we should be able to get this merged for v0.1.28 🤞

@LGUG2Z
feat(animation): add window animations
06f7954 
Work on this feature was first started by @thearturca in November 2023
before komorebi v0.1.21 in #597 and has undergone numerous revisions
to reach the point of this commit.

Although this is a single squashed commit, almost all of the heavy
lifting for this feature was done by @thearturca, which is where all of
the kudos and gratitude should be directed.

This commit adds a new static configuration block for animations, where
they can be enabled, and have their style, fps and duration set.
Corresponding SocketMessages and komorebic cli commands have also been
exposed.

There are some caveats to the use of this feature, which revolve around
the quality of the Windows compositor (it is not very good):

* There will be visual artifacts with various apps when animations are
  taking place - komorebi can't do anything about this as it is a
  limitation of the Windows compositor
* Since komorebi's borders are implemented as independent windows are
  are not a part of the windows they are drawn around, these borders
  will be hidden while animations are in progress
* If you wish to use borders with this feature, you'll probably better
  off using BorderImplementation::Windows, which uses the native thin
  "accent" borders, which are part of the windows they are drawn around,
  and can be moved with those windows during animations

As a result of these and other caveats, this feature will be marked as
"experimental" for the foreseeable future and will be off-by-default.

Below, a number of now-squashed commits that contributed to the
stabilization of this feature are referenced to help with code
archeology in the future.

fix(animation): Fixed cancelling logic
(57e9b2f)

Added static animation state manager for tracking "in_progress" and
"is_cancelled" states. The idea is not to have states in Animation
struct but to keep them in HashMap<hwnd, AnimationState> behind
reference (Arc<Mutex<>>). So we each animation frame we have access to
state and can cancel animation if we have to.

Need review and testings

refactor(animation): avoid unwrap
(fa6d5bb)

fix(animation): Move cancel call to Animation struct
(306513f)

Only focused window was cancelling its animation because we call cancel
in window::set_position and waiting for its cancelling. And because we
waiting for cancelling second window is still moving. Second window will stop
moving only after the first window. So I moved `cancel` call to
Animation struct so its happening in its own thread and doesn't block
others animation moves and cancels.

refactor(animation): renamed args parameters and variables names
(8abb4b9)

refactor(animation): inverse if-statement in `window::animate_position`
(3de2c6e)

There is was a bug when ease function generates `t` greater the
`SetWindowPos` function will be called instead of `move_window`.
`SetWindowPos` is only for last frame of animation.

fix(wm): add shadow rect to `move_window` calls
(b58620f)

This fixes a bug when windows get shunk during the animation
@LGUG2Z LGUG2Z merged commit e2f2d6b into master Jul 10, 2024
4 checks passed
@LGUG2Z
Copy link
Owner Author

LGUG2Z commented Jul 10, 2024

This has now been merged to master - I'll find some time this week to start working on the docs in prep for the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@LGUG2Z @krshrimali @raggi @renhiyama @thearturca @DamianK