On Mon, Jan 5, 2009 at 2:04 AM, Danny Baumann <dannybaum...@web.de> wrote:
> Hi,
>
> since the text plugin interface is among the weirdest ones that we have
> (a structure pointer passed via a character pointer variable, a pixmap
> passed back as void *, usage of the image rendering interface), I
> decided to give cleaning it up a go. Attached is what I have so far, a
> text plugin rewrite and an example implementation of the new interface
> for the ring plugin.
>
> I would like to know about opinions, improvement suggestions etc. about
> this new interface. Ideas I have (and like to hear opinions about) are

Yeah. Text needs to use a bit less dark magic to get it's work done anyways.

>
> - Pass a CompTextData * instead of the Bool as result of the rendering
> functions, which then could be used to check if rendering was successful
> later on (would save some memory in my example implementation, but
> would cause another heap allocation though, not sure if we want to avoid
> that)

Why do we have a CompTextAttrib and and a CompTextData type? Why not
just pass a pointer to the CompTextAttrib and have the image renderer
render to that instead of having to muck around with heap allocation
for another type?

I don't know what you're take on this is though, is it worth having
two separate types?

>
> - Implement more stuff currently implemented via copy'n'paste in the
> various text plugins users. Among those are e.g. automatic binding of
> the pixmap to the texture, a cleanup function, a drawing function which
> puts the texture on screen. I'm not sure doing stuff like automatic
> binding would limit the use cases of the text plugin (e.g. group doesn't
> need it because it does some additional cairo magic). We could make the
> binding optional via a flag, though (and enable it by default).

Definitely.

I still see you've got:

     releasePixmapFromTexture (s, &rs->textTexture);
     initTexture (s, &rs->textTexture);
-    XFreePixmap (s->display->display, rs->textPixmap);
-    rs->textPixmap = None;
+    XFreePixmap (s->display->display, rs->textData.pixmap);
+    rs->textData.pixmap = None;

In the cleanup function, that can probably be put into text as well,
pretty much all plugins that use text have a cleanup function like
that.


Also, I don't quite understand what you have here by rs->type == RingTypeAll

+    if (rd->textFunc->renderWindowTitle (s, rs->selectedWindow,
+                                        rs->type == RingTypeAll,
+                                        &attrib, &rs->textData))

>
> Any thoughts?
>
> Thanks,
>
> Danny
>

Feel free to go ahead with this, would you mind adapting my plugins
(prompt, elements-extendable etc) for the new interface if you do,
I'll be away for the next week or so.

Cheers,

Sam.

> _______________________________________________
> Dev mailing list
> Dev@lists.compiz-fusion.org
> http://lists.compiz-fusion.org/mailman/listinfo/dev
>
>



-- 
Sam Spilsbury
_______________________________________________
Dev mailing list
Dev@lists.compiz-fusion.org
http://lists.compiz-fusion.org/mailman/listinfo/dev

Reply via email to