[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwin
Subject:    Re: Updates to effects
From:       Philip Falkner <philip.falkner () gmail ! com>
Date:       2007-06-25 13:54:41
Message-ID: 200706250954.45173.philip.falkner () gmail ! com
[Download RAW message or body]

On Monday 04 June 2007 11:51:26 Lubos Lunak wrote:
> On Sunday 03 of June 2007, Philip Falkner wrote:
> > Sorry for the long silence.

Agh!  If it's not one thing, it's a motherboard suddenly dying.  Different 
system, but still r200/AIGLX.

>  Just in case you have it already split into smaller patches and combined
> it together only for this mail, then please don't.  Smaller patches are
> much easier to read.

Ok.

> > I've added some features like animation, following color settings, etc.
>
>  I'm not quite sure I like the animation. IMHO it makes finding a window to
> select harder. But Compiz has it too, doesn't it? Hmm ... config option :-/
> ?

I'm not happy with the quality of the animation itself, but the fact of the 
animation is a workaround for a different problem.  With the previous code, 
if you have tons of windows and alt-tab, the windows shrink and shrink and 
shrink.  Eventually they'll get too small to see.  An upper bound on the 
number shown would keep them visible, but what do you do when you have more 
windows than the upper bound?

Animating like this allows for a fairly restrictive upper bound, and nice big 
thumbnails, no matter the window count.  Current non-composited tabbox 
shrinks once, then starts dropping clients if that's not enough.  Ugh.  Which 
reminds me that I need to fix that...

> > But the real meat, is an updated
> > api for effects replacing the tabbox.  It's a bit more verbose, but
> > covers I think what most replacements would want to do.  I also think it
> > can be extended to allow for new tabbox modes without disturbing existing
> > effects.
>
>  Ok. There's one more thing related to this I'd like to handle somehow:
> Currently DesktopPresentEffect has the nice window filtering feature that
> got quite some people drooling, but I don't think it really belongs there.
> I see no reason why Alt+Tab shouldn't be able to do the same, even in the
> non-composited mode. The same way DesktopPresentEffect should probably also
> be able to, among other things, to go forward/backward in the focus chain.
> It'd be nice to share this functionality. Do you have any idea how feasible
> this would be? If I'm getting it right with your tabbox API it is
> tabbox.cpp that decides many things and I wonder if that wouldn't be too
> limiting for the effects (although I admit I don't completely understand
> the API).

In a sense, the tabbox already has a non-optional filtering mechanism, dealing 
with modal windows and such.  What if we allow effects to add to that?

I'm thinking that effects could have a "filtering function" 
(effects->windowListFilter?).  Tabbox::createClient/DesktopList() then calls 
that function, passing a QList; effects deal with the list as they like.  The 
final QList is returned to the tabbox, which would only then call 
windowListChanged(), etc.

This would mean each effect (and the non-composited tabbox) that wanted 
as-you-type filtering would have to implement it independently, though.

> > It doesn't substantially change how the tabbox itself works.  Also, the
> > changes to toplevel.cpp and deleted.cpp are simply to stop kwin from
> > sending windowDamaged events for windows whose effectWindows have been
> > deleted.
>
>  I don't understand this, and the change seems to be wrong. The
> EffectWindow object is passed from the original window to Deleted and only
> Deleted eventually destroys it.

~Deleted() deletes effectWindow(), but then ~Toplevel() is called, which calls 
discardWindowPixmap(), which calls addDamageFull(), which calls 
windowDamaged() with effectWindow() which, at this point, is deleted but not 
NULL.  In theory, it should be set to NULL in ~Deleted(), but effect_window 
is private to Toplevel.

If the way I wrote it doesn't appeal, we could make effect_window protected; 
then ~Deleted() could set it NULL no problem.

>  Also, note that "delete point_that_is_null;" is valid, so there's no need
> to check the pointer for validity.

Ok, thanks. :)

> > Second is a new Fade.  Unashamedly taken from the compiz fade, it can now
> > fade between any changes in opacity/saturation/brightness.  That is, it
> > can handle fading for other effects.  The question I have is, do we want
> > this? It gives us a single place for the user decide whether they want
> > things fading in and out, and how quickly.  On the other hand, maybe
> > effects are better off with different speeds for different purposes, e.g.
> > should the fade in PresentWindows be able to be a different speed than
> > the fade in DialogParent? I think the centralised way is better, but I'm
> > open to being told that's wrong. :)
>
>  I think only practice will tell. Let's give it a try :).

I've found an issue with this new fade (hence not committed).  It only works 
for those effects that are loaded before it, which currently means by default 
effects whose names come before the letter F.  I can't think of how to 
guarantee the fade code will run only just before finalPaintWindow(), unless 
we either add a new hook there, make some way for effects to influence their 
loading order, or incorporate this fading code (i.e. the bit that smoothly 
animates changes in opacity/brightness/saturation, NOT the decision to fade 
on windowAdded/Closed) into kwin proper.

I like Rivo's idea of having an interface for changing window appearance 
(WindowPaintData?) in a time defined by the requestor, e.g. change saturation 
to 0.4 in 150ms.  I wonder how complex/simple it might be.

By the way, almost unrelated, is there any practical reason why effects 
shouldn't use KConfigXT vs. the KConfig stuff in EffectsHandler?

-- 
Philip Falkner
_______________________________________________
Kwin mailing list
Kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]