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

List:       kwin
Subject:    Re: Effects as plugins
From:       Lubos Lunak <l.lunak () suse ! cz>
Date:       2007-04-08 9:39:02
Message-ID: 200704081139.02617.l.lunak () suse ! cz
[Download RAW message or body]

Dne pátek 06 duben 2007 20:14 Rivo Laks napsal(a):
> Hi!
>
> The attached patch makes it possible to dynamically load effects from
> plugins.

 Looks good to me in general after skimming through it, with some notes below.

> A bit more detailed changelog:
> * Instead of being built in, every effect is in it's own library which can
> be loaded at runtime.

 We've already talked about this on IRC, it should be possible to group 
effects together in one lib for performance reasons. It should be a matter of 
changing the factory macro to add the effect name to the symbol and finding 
out which lib to load from a .desktop file. I don't consider this to be a 
showstopper for committing to SVN though.

 This also relates to the KWIN4_ADD_EFFECT CMake macro which doesn't support 
this either.

> * EffectFactory stuff has been replaced by a macro for now. I'm not sure
> about whether or not the factory should still be used. I could use just
> three macros: one for creating effect and two optional ones for checking
> whether effect is supported and getting the config widget.

 I think calling the macro KWIN_EFFECT_FACTORY is a bit of misnaming, since it 
doesn't create a factory but the effect itself. But yes, I think there should 
be three macros, one creating function for creating the effect (i.e. current 
KWIN_EFFECT_FACTORY), one creating function for effect_supported and one 
creating function for the config.

 Also related to that, having Effect::supported() looks like a leftover of 
something, since the function called from effect_supported should be probably 
a completely standalone function.

> TODO:
> * API additions for OpenGL stuff such as vertices or shaders. Vertex class
> will need to be moved to kwineffects.h
> * Loading of default effects. Should be easy.

"TODO: maybe make Toplevel::effectWindow() return the Impl??" - Yes, I'd say 
so.

> Questions:
> * do I need to call library->unload();  after deleting an effect in
> EffectsHandler::unloadEffect() ?

 Yes. Every load() should be matched with unload(), and this is 
reference-counted, so as long as the counts match the lib will be unloaded 
only when it's really not needed anymore.

> * I hope *Impl class names are fine

 I don't have any better idea.

> I've also ported two effects to the new system: my presentwindows effect
> and the shadow effect. You can load them using the usual dbus loading
> methods, except that the effect name is either "presentwindows" or "shadow"
> (i.e. lowercase).
>
> To try it, first apply the patch, then extract the other files into the
> kwin source dir.

-- 
Lubos Lunak
KDE developer
--------------------------------------------------------------
SUSE LINUX, s.r.o.   e-mail: l.lunak@suse.cz , l.lunak@kde.org
Lihovarska 1060/12   tel: +420 284 028 972
190 00 Prague 9      fax: +420 284 028 951
Czech Republic       http//www.suse.cz
_______________________________________________
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]