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

List:       kwin
Subject:    Re: Effects as plugins
From:       Rivo Laks <rivolaks () hot ! ee>
Date:       2007-04-09 16:23:22
Message-ID: 200704091923.23022.rivolaks () hot ! ee
[Download RAW message or body]

Ühel kenal päeval (pühapäev 08 aprill 2007) kirjutas Lubos Lunak:
> 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.

Done. Each of the three macros (see below) now takes two parameters: first is 
the effect's (internal) name, second is class/function of that effect.

The library is first searched for in the corresponding .desktop file. If the 
file doesn't exist, library name is assumed to be the effect's name lowercase 
(+ the prefix).

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

Done. First argument of the macro is now name of the library, without 
the "kwin4_effect_" prefix. The rest are sources for that library.

> > * 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.

There's now KWIN_EFFECT(), KWIN_EFFECT_SUPPORTED() and KWIN_EFFECT_CONFIG(). 
(the latter two are optional of course and config isn't even used yet).
Are those ok or do you have better naming ideas?

>  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.

Yeah, I removed both supported() and init()

I did some further cleanups as well.

Updated patch is attached. Both PresentWindows and Shadow effects are now put 
into single plugin kwin4_effects_builtins (yeah, the name sucks).

Rivo

["effects-as-plugins.tar.bz2" (application/x-tbz)]

_______________________________________________
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]