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