[compiz] Re: compiz: Changes to 'master' (Added possibility to load textures with fixed RGBA byte ordering from header files)

[David Reveman]

On Wed, 2007-03-28 at 18:28 +0200, Danny Baumann wrote:
> Hi,
> 
> > I find this change pretty ugly. "Bool fixedRGBA" looks like a quick hack
> > to extend the imageToTexture function with some specific functionality.
> > 
> > Creating a lower level function that accept a format and a type GLenum
> > seems like a more appropriate solution to me.
> 
> To be honest, I'm a bit surprised about that. I asked you about adding
> that kind of functionality before explaining what I'm going to do, and
> you agreed to that. 

Yes, adding this kind of functionality if it's useful is what I agreed
to. I never told you that this patch was the appropriate way to do so.
Sorry, for the confusion.

> Besides that, I'm not sure if we really need such an universal loader
> function. However, I can have a look into it ... do you have any
> proposal on how an interface (prototype) should look like?

Have imageToTexture accept format and type parameters and change
imageBufferToTexture to this:

Bool
imageBufferToTexture (CompScreen   *screen,
		      CompTexture  *texture,
		      char	   *image,
		      unsigned int width,
		      unsigned int height,
		      GLenum	   format,
		      GLenum	   type);

or keep the old imageBufferToTexture and provide a new function that
accepts these new parameters.

> 
> > However, if you're storing textures in header files. I suggest that you
> > store them as png data, in which case you you don't need this
> > functionality in core at all as png plugin will do byte-swapping when
> > necessary.
> 
> Only textures which aren't supposed to be changed and are crucial for
> the operation of a plugin should be stored in header files. In my
> opinion, for those important textures we should avoid creating image
> loader plugin dependencies.

I don't see any good reason to why an image loader plugin dependency
should be avoided. You use less memory when storing it as png data and
that is to me good enough reason to add the png plugin dependency.

- David