picom icon indicating copy to clipboard operation
picom copied to clipboard

@FTLabs generalanimation branch merged with next with original git history and compiling (Animation PR)

Open colonelpanic8 opened this issue 1 year ago • 21 comments

@FT-Labs Also seems to contain work from @dccsillag, although his original commits were somehow lost.

Serves as a replacement for #1032 #772

Fixes #217

colonelpanic8 avatar Aug 06 '23 23:08 colonelpanic8

@fdev31 I didnt find it too hard to do this merge with original git history and without any other additional changes.

colonelpanic8 avatar Aug 06 '23 23:08 colonelpanic8

Codecov Report

Merging #1109 (267683e) into next (5d6957d) will decrease coverage by 1.75%. The diff coverage is 12.98%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1109      +/-   ##
==========================================
- Coverage   37.53%   35.78%   -1.75%     
==========================================
  Files          49       49              
  Lines       11187    11716     +529     
==========================================
- Hits         4199     4193       -6     
- Misses       6988     7523     +535     
Files Changed Coverage Δ
src/atom.c 87.50% <ø> (ø)
src/atom.h 100.00% <ø> (ø)
src/backend/dummy/dummy.c 65.33% <ø> (ø)
src/backend/gl/gl_common.c 25.31% <0.00%> (-0.13%) :arrow_down:
src/backend/gl/gl_common.h 30.23% <ø> (ø)
src/backend/xrender/xrender.c 0.00% <ø> (ø)
src/cache.c 62.79% <ø> (ø)
src/common.h 75.00% <ø> (ø)
src/config.h 23.52% <ø> (ø)
src/event.c 67.29% <0.00%> (-2.43%) :arrow_down:
... and 10 more

... and 1 file with indirect coverage changes

codecov[bot] avatar Aug 07 '23 00:08 codecov[bot]

@absolutelynothelix @yshui

Not my code, and I realize that this likely is not yet perfect, but you have to realize that people are using these forks with animations really really widely.

I think at this point, its going to be much better for the community if we just merge this into the next branch and iterate on it from there.

If not, whats inevitably seeming to happen is that someone advances things and then loses steam before things get merged.

We've seen this with a huge number of forks now:

https://github.com/FT-Labs/picom/tree/generalanimation https://github.com/pijulius/picom https://github.com/dccsillag/picom https://github.com/Arian8j2/picom https://github.com/jonaburg/picom

It's gotten to the point where some distros are even packaging these alternative versions of picom:

https://aur.archlinux.org/packages/picom-simpleanims-next-git https://aur.archlinux.org/packages/picom-allusive https://aur.archlinux.org/packages/picom-ftlabs-git https://aur.archlinux.org/packages/picom-arian8j2-git

and its not even JUST in the AUR:

https://github.com/NixOS/nixpkgs/blob/af90462b887484d323baf294ff1d2239ae463b94/pkgs/applications/window-managers/picom/picom-jonaburg.nix#L4

Furthermore, I know that many people recommend using these forks with overlays in nixos (it doesnt even really need to be packaged)

see the simplicity of the above link.

colonelpanic8 avatar Aug 07 '23 00:08 colonelpanic8

@fdev31 I didnt find it too hard to do this merge with original git history and without any other additional changes.

It's a very different release / branch so I don't think it can be compared. When I did as far as I recall it would just apply without conflict but then miss some important changes and some big regressions were visible. Anyway, glad you had better luck.

Since I'm not using picom anymore I'll probably not contribute any further but feel free to add my improvements too (mainly the transient windows) I think they add a great touch!

fdev31 avatar Aug 07 '23 07:08 fdev31

Ah, I feel I should mention that I vaguely remember @FT-Labs basing themselves on @pijulius' fork (which was a fork of my repo, which in turn was a fork of actual picom). Maybe that can help a bit with reconstructing the history?

There was already quite a bit of commits just in my repo + pijulius' work, so if we somehow recover even part of those that would be best (including for review purposes! It's a lot of code). Maybe we can use one of those as a starting point, do a git diff with @FT-Labs's code, at which point a chonky hacked up commit comes in, and then the current history on this branch here?

dccsillag avatar Aug 07 '23 10:08 dccsillag

It's a very different release / branch so I don't think it can be compared. When I did as far as I recall it would just apply without conflict but then miss some important changes and some big regressions were visible. Anyway, glad you had better luck.

No I obviously had to fix a ton of things. It didn't just apply cleanly. You squashed a ton of commits, which doesn't make the conflict resolution process any easier.

There was already quite a bit of commits just in my repo + pijulius' work, so if we somehow recover even part of those that would be best (including for review purposes! It's a lot of code). Maybe we can use one of those as a starting point, do a git diff with @FT-Labs's code, at which point a chonky hacked up commit comes in, and then the current history on this branch here?

@dccsillag It seems that everything was collapsed into this commit https://github.com/yshui/picom/pull/1109/commits/f6b0b04f5bd8bc5f727cc3bb17802a88ca87ed51 by @FT-Labs from what I can tell this is not based on pijulius fork, because it does not seem to have the workspace animations. Unfortunately, its also not equivalent to any commit of your fork either, because it seems to also include a merge. with next.

Unfotunately, I've also noticed that this branch includes a MASSIVE performance regression compared to the head of your animation branch.

I'm thinking that it may simply be better to attempt this merge based on your branch, rather than this one. Its not clear what @FT-Labs fork adds that yours does not already have.

colonelpanic8 avatar Aug 07 '23 17:08 colonelpanic8

At this point I think everyone who has ever played with animation in Picom should schedule a day to discuss the best way to implement the feature and then everyone would start mob programming to get this feature off the ground.

joaopauloalbq avatar Aug 25 '23 19:08 joaopauloalbq

At this point I think everyone who has ever played with animation in Picom should schedule a day to discuss the best way to implement the feature and then everyone would start pair programming to get this feature off the ground.

I think it's a very good way to have it never happening - unless some energy coming from nowhere pushes it forward. But I understand the concern of merge "foreign" code missing proper history. In my version I think the changes were really isolated and easy to review IMO - that was one of my goals.

fdev31 avatar Aug 28 '23 14:08 fdev31

Hello guys,

I've added code on dccsillag fork. I will enhance all of these, there are some bugs. I don't think animations should be included in the main branch of picom without all of the bugs solved.

The ones I will handle: desktop switch animations high cpu usage flickering issues (done) monocle layout problems

what i need help with: some opengl error spams to log handling each animation frame with respect to monitor fps shadows create glitch when creating new window (only happens on low end gpu/integrated gpu) per window animation option

I would be glad if someone can reach out and we can talk about how to proceed, thanks.

FT-Labs avatar Aug 28 '23 18:08 FT-Labs

@FT-Labs I think there are performance issue in this branch relative to @dccsillag 's branch.

For me things are completely stutter free and smooth on that branch and they are not with this.

Not sure if it has to do with how things that happened in master interacting wiht your stuff or if you somehow changed the performance characteristics of the animation but its notably different.

colonelpanic8 avatar Aug 28 '23 18:08 colonelpanic8

@fdev31 @joaopauloalbq

I would be glad to do this whenever you guys have free time. I can not reach out to everything because everything takes huge time.

Could you guys come to discord channel then we can arrange some time for this?

FT-Labs avatar Aug 28 '23 19:08 FT-Labs

@colonelpanic8

Could you try the next branch instead of generalanimation?

generalanimation is outdated and has unoptimized code.

FT-Labs avatar Aug 28 '23 19:08 FT-Labs

Due to the amount of forks exposed by @colonelpanic8 , I imagined that there could be a big difference in the approaches used by each one and because of that, a conversation on Discord with the developers @BlackCapCoder @jonaburg @phisch @dccsillag @pijulius @Arian8j2 @FT-Labs and @fdev31 could help to choose the best approach to be implemented in a single repository for all. Maybe a mob programming (Visual Studio Live Share ?) is what's missing to solve this step of unification and specification or maybe I'm thinking too much nonsense. 😅

joaopauloalbq avatar Aug 29 '23 04:08 joaopauloalbq

I'm a bit time-constrained right now, but I would be happy to help on this. I think there might be something to the conversation/meetings idea, even if just a couple of us show up to it. @joaopauloalbq feel free to reach out to me if you want to go ahead with it.

(Visual Studio Live Share is a no-go though, I need my NeoVim :sweat_smile:)

dccsillag avatar Aug 29 '23 10:08 dccsillag

(Visual Studio Live Share is a no-go though, I need my NeoVim 😅)

There's always floobits:

https://github.com/Floobits?q=emacs&type=all&language=&sort=

although I'm not sure that a pair programming session is exactly whats needed.

I'm happy to do any nasty merges like this one for people.

Could you try the next branch instead of generalanimation?

@FT-Labs Ah damn. wish I had known that before I did this merge. Is the next branch up to date with master? Does it have the generalanimation as actual proper git history so I can use all the conflict resolution that I did here at least?

colonelpanic8 avatar Aug 29 '23 18:08 colonelpanic8

Hello. I am new to the @FT-Labs fork. Could you please explain what about window closing and unmapping animations? The corresponding code is presented in sources (animation_for_unmap_window), but in pocom.c it is obvious that the unmapped window is skipped and is not going to be painted. Why animation_for_unmap_window is needed and what does it affect?

XoDefender avatar Aug 30 '23 10:08 XoDefender

discord #development channel have now an animation thread which can be a good start.

fdev31 avatar Aug 31 '23 14:08 fdev31

Hello guys,

I'm open for a meeting, it would be much better if I explain what is going on via screen share.

@XoDefender Both unmap and destroy events are painted and have fade out timers as well, I did not understand which part of the code you have thinked about that.

Try to check win.c init_animation

FT-Labs avatar Aug 31 '23 16:08 FT-Labs

@colonelpanic8 general animation branch is when I started to make desktop switch animations on other DE - window managers aside from pdwm, but it is by far from done. next branch is up to date with yshui/picom next.

Extra info: xrandr monitor data fetching per window should be done correctly and it needs a new function. It only fetches window's position, but some animations like minimize, maximize and so on are either goes to center of the screen or starts from the center of the screen. In single monitor there will be no issues, but on multi-monitor animations will go to center of the whole x viewport.

I am not an expert on opengl, if some of you guys know it well I need some guidance for it. I don't know if you know about the flickering window issue in other animation forks (that uses dccsillag as base) but during spawning - destroying - resizing a window will flicker (like a white flash, or transparency will be reset). I have handled this issue with a really weird solution, I would be glad if someone can help with this.

https://github.com/dccsillag/picom/issues/19

FT-Labs avatar Aug 31 '23 16:08 FT-Labs

@fdev31 Do not have access to the channel, or the link is not correct, could you recheck please? (No text channels)

XoDefender avatar Aug 31 '23 18:08 XoDefender

@fdev31 Do not have access to the channel, or the link is not correct, could you recheck please? (No text channels)

It's on the official discord server

fdev31 avatar Sep 01 '23 17:09 fdev31