[effects] Add Scale Out effect
AbandonedPublic

Authored by zzag on May 27 2018, 4:21 PM.

Details

Reviewers
ngraham
Group Reviewers
KWin
Plasma
VDG
Summary

This change adds a counterpart to the Scale In effect.

Depends on D13154

Test Plan
  • Enable the Scale Out effect
  • Close a window

Diff Detail

Repository
R108 KWin
Branch
scale-in-out-effect
Lint
No Linters Available
Unit
No Unit Test Coverage
zzag created this revision.May 27 2018, 4:21 PM
Restricted Application added a project: KWin. · View Herald TranscriptMay 27 2018, 4:21 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.May 27 2018, 4:21 PM
zzag planned changes to this revision.May 27 2018, 4:24 PM
zzag edited the summary of this revision. (Show Details)
zzag updated this revision to Diff 35025.May 28 2018, 1:22 PM

Rebase.

zzag planned changes to this revision.May 28 2018, 1:23 PM
zzag edited the summary of this revision. (Show Details)
abetts added a subscriber: abetts.May 28 2018, 4:57 PM

I like it. It is very smooth. How fast is it?

zzag updated this revision to Diff 35057.May 28 2018, 8:31 PM

Make it more subtle.

zzag retitled this revision from WIP: [effects] Add Scale Out effect to [effects] Add Scale Out effect.May 28 2018, 8:32 PM
zzag edited the summary of this revision. (Show Details)
zzag edited the test plan for this revision. (Show Details)
mart added a subscriber: mart.May 30 2018, 8:39 AM

on looks, +1 :)

ngraham accepted this revision.May 30 2018, 8:19 PM
ngraham added a subscriber: ngraham.

Looks fantastic to me too. I hope these patches land!

This revision is now accepted and ready to land.May 30 2018, 8:19 PM

In general I'm against adding new effects to KWin and would rather prefer to remove effects such as scale in. I think everything which is not default should be on store.kde.org and not added to KWin directly.

zzag abandoned this revision.May 31 2018, 8:37 AM
zzag added a comment.May 31 2018, 9:05 AM

In general I'm against adding new effects to KWin and would rather prefer to remove effects such as scale in.
I think everything which is not default should be on store.kde.org and not added to KWin directly.

I don't agree with you about store.kde.org part. Scale animation is _really_ basic thing and if someone has to go somewhere on internet to get it, I think that's a problem. That's like GNOME Shell, you have to install a bunch of third-party plugins/extensions to get basic things (like system tray, etc). No hate about GNOME or GNOME devs, etc.

Maybe, it would be better to create kwin-extras repo(like kio-extras) and dump these effects there.

In D13155#271278, @zzag wrote:

In general I'm against adding new effects to KWin and would rather prefer to remove effects such as scale in.
I think everything which is not default should be on store.kde.org and not added to KWin directly.

I don't agree with you about store.kde.org part. Scale animation is _really_ basic thing and if someone has to go somewhere on internet to get it, I think that's a problem. That's like GNOME Shell, you have to install a bunch of third-party plugins/extensions to get basic things (like system tray, etc). No hate about GNOME or GNOME devs, etc.

Maybe, it would be better to create kwin-extras repo(like kio-extras) and dump these effects there.

I agree with this assessment. These types of effects are considered defaults or fundamental. Generally shipped with the system.

I agree with Vlad and Andy. The scale effect is a pretty basic thing. I don't how making it less discoverable for users is a benefit for our platform.

Also, moving the effect to store.kde.org signals a different sort of contract with users: it suggests that the effect is an unsupported 3rd-party thing that could break at any time, and for which there is no support. We don't accept bugzilla tickets about store.kde.org content, for example. Right now, the effect is actually supported by KDE developers--or at least, it would be, if @graesslin would allow it.

My comment was not a veto, it was meant as a different thought perspective.

Let me explain from the perspective of having been the maintainer of KWin for most of the time we did effects. Most effects are currently unmaintained, bugs pile up and some have been broken for years without anybody noticing. Right now the magnifier effect is once again broken. This has probably been the case for the last few releases, only now it had been reported. Also thumbnail aside is broken in some configurations. So the scale in effect is certainly not maintained, it's just in the random situation that it's part of KWin. Nobody looks at it, nobody fixes bugs. As maintainer I promise with my name for the quality of the overall product. With the amounts of effects we have this is difficult. I'm very glad to see more activity in the effects!

One of the base ideas in the planning for Plasma 5 was to only keep what we can maintain. The effects would have been a good candidate to remove. They weren't as they were not broken and we did not have a place to move - kdelook was proprietary back then and such a no go.

The second observation from looking at thousands of bugs and the requested support information is that hardly any non default effects are used. This contradicts the claims that users expect it. Also I must say that I don't care what users expect from other products. If they provide it: great. But the amount for available effects in compiz has never been a motivation for us to add more. Rather the opposite. Talking with compiz devs it showed that the amount of effects was their biggest debt. I try to learn from the experience others do. And that's why I write here to pass on the experience to the next generation.

What changed from an infrastructure point of view is that we have store.kde.org now. We could move effects there, maybe even under an official KDE.org account. This would bring many advantages such as controlling updates, not bound to release cycles, not bound to distributions. It's something for thought and it's something I should have done quite some time back. But sometimes you don't think about it till it gets on the plate.

Btw. for alt+tab we moved all themes out to kdeplasma-addons for the same reason.

Anyway with my current level of contributions I don't feel in a position to veto such a new effect.

I understand and don't actually disagree with the idea of eventually moving "semi-maintained" code/effects out of the main repo. I think there's definitely some logic to that. But a lot of other things need to happen before it's a feasible plan. store.kde.org first needs better curation, a real versioning system that will prevent users from seeing content that won't work for their Plasma version, and better installation interfaces (both the GHNS and Discover are somewhat rough as frontends to store.kde.org at the moment, and sometimes don't actually work).

For now, since this code is mostly unmaintained, then I think we should welcome Vlad's improvements and efforts to maintain it today, regardless of what its ultimate fate in the future may be.

Here's an idea: instead of adding a new "Scale Out" effect, would/could it make sense to improve the general Scale In effect to cover both window opening and closing, and then rename it to be just "Scale?"

romangg added a subscriber: romangg.Jun 1 2018, 2:06 PM

I agree with you both. Long term it's right what Martin says: We want to reduce the code base in KWin to have it lean and move additional functionality out so users can extend their desktop if they wish to on a solid base.

On the other side the current store.kde.org infrastructure is not yet mature enough to guarantee easy discoverability and maintenance.

In this specific case I would leave the decision to @zzag. If he feels comfortable maintaining the effect let's get it merged. It's not a huge code addition, looks really good and is analogue to the scale in effect.

zzag added a comment.EditedJun 7 2018, 6:29 PM

In this specific case I would leave the decision to @zzag. If he feels comfortable maintaining the effect let's get it merged. It's not a huge code addition, looks really good and is analogue to the scale in effect.

Yeah, I'm okay to maintain both scale effects (as any other that I touched/rewrote recently). Yet, I'm not sure whether that's a right way to split the Scale effect into two parts(In and Out). Most window animation effects(fade, glide, minimize, magic lamp) are monoliths.

Yet(x2), I'd prefer to write the monolith scale effect in C++ and mark the scale in effect as deprecated(and remove with the next major release).

zzag added a comment.Jun 7 2018, 6:40 PM

@graesslin @romangg So what should we do? I see the following options:

  • abandon it (and proceed with the store.kde.org part)
  • continue work on both scale effects
  • as I said above: write monolith scale effect and deprecate the scale in effect (I'm okay to maintain it)
romangg added a comment.EditedJun 7 2018, 6:53 PM

I think it makes sense to have a separate ScaleOut effect and not a monolithic Scale effect. Because a user can then select different effects for mapping/unmapping windows. So if you reclaim this diff, I'll accept it.

EDIT: Ok, I just read you said most other effects are monolithic. I think it makes sense to split it up only if there are examples of other effects splitting it up.

zzag added a comment.Jun 7 2018, 7:14 PM

Because a user can then select different effects for mapping/unmapping windows.

That would be great to have something similar like in Compiz, I agree.

EDIT: Ok, I just read you said most other effects are monolithic. I think it makes sense to split it up only if there are examples of other effects splitting it up.

Exactly! Will the Magic lamp be split in the near future? I don't think so. For the sake of consistency, I'd prefer to have the Scale effect as a monolith.