Merge lp:~bregma/compiz/lp-1101594 into lp:compiz/0.9.12

[Stephen M. Webb]

OpenGL plugin: refine GLTexture API to adhere to class invariants

OpenGL plugin: refactor GLTexture to pass values in the constructor and maintain class invariants instead of using the C-with-classes default-initialize-and-call-initializer-function that makes Coverity comnplain about uninitialized member variables.

The uninitialized-variables complaint is unlikely to be realized in core Compiz plugins but since it;s a public API that arbitrary third-party plugins could use, it's a poor API. The old API has been deprecated by left in to provide ABI backwards compatibility.

Also, scraped some API docs off compiz.org and added them in to the header while the patient was open on the table.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3935
http://jenkins.qa.ubuntu.com/job/compiz-ci/41/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-amd64-ci/31
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-vivid-armhf-ci/31/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-ci/41/rebuild

review: Needs Fixing (continuous-integration)

[lp:~bregma/compiz/lp-1101594]

3936. By Stephen M. Webb on 2015-03-30

fixed typo in EGL codepath

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3935
http://jenkins.qa.ubuntu.com/job/compiz-ci/42/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-amd64-ci/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-armhf-ci/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-i386-ci/32

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-ci/42/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3936
http://jenkins.qa.ubuntu.com/job/compiz-ci/43/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-amd64-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-armhf-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-vivid-i386-ci/33

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-ci/43/rebuild

review: Approve (continuous-integration)

[Christopher Townsend (townsend)]

Ok, seems fine.

review: Approve

[Christopher Townsend (townsend)]

Oh, do we need an ABI bump for this?

[Christopher Townsend (townsend)]

Ok, all good:)

=== modified file 'plugins/opengl/include/opengl/texture.h'
--- plugins/opengl/include/opengl/texture.h	2012-09-18 11:32:20 +0000
+++ plugins/opengl/include/opengl/texture.h	2015-03-30 22:52:49 +0000
@@ -1,6 +1,7 @@
 /*
  * Copyright © 2008 Dennis Kasprzyk
  * Copyright © 2007 Novell, Inc.
+ * Copyright 2015 Canonical Ltd.
  *
  * Permission to use, copy, modify, distribute, and sell this software
  * and its documentation for any purpose is hereby granted without
@@ -71,7 +72,44 @@
 class PrivateTexture;
 
 /**
- * Class which represents an openGL texture
+ * An abstract representation of an OpenGL texture.
+ *
+ * Enabling and disabling the texture, and drawing it with OpenGL
+ * --------------------------------------------------------------
+ *
+ * Because there are different texture binding backends, it is not safe to
+ * enable the texture target using OpenGL directly. Instead, you should use the
+ * ::enable () and ::disable () member functions. The ::enable () member
+ * function also allows you to specifiy a level of texture filtering, either
+ * Fast or Good. If you are drawing the texture at a smaller size then the
+ * render size, then it should be safe to use the Fast filter.
+ *
+ * Getting OpenGL information about the texture
+ * --------------------------------------------
+ *
+ * It is possible to get the ::name () and ::target () of the texture to use
+ * that data for OpenGL purposes.
+ *
+ * Mipmapping
+ * ----------
+ *
+ * Compiz also supports a mipmapping filter to make textures appear smoother
+ * when viewed at angles. To enable this you can use the ::setMipmap and
+ * ::mipmap functions in order to enable this. Mipmapping is not available on
+ * certain hardware. In this case, the function will simply fail silently.
+ *
+ * Overloading the binding of textures
+ * -----------------------------------
+ *
+ * For whatever reason, you may wish to write your own texture binding method
+ * which overloads the standard GLX_ext_texture_from_pixmap method. It is
+ * possible to do this by overloading the ::enable () and ::disable () methods
+ * of the GLTexture through your own class. Then on your plugin's init function
+ * you should use GLTexture::BindPixmapHandle hnd = GLScreen::get
+ * (screen)->registerBindPixmap (YourPixmap::bindPixmapToTexture) and on fini
+ * you must also call GLScreen::get (screen)->unregistrerBindPixmap (hnd).
+ *
+ * A sample implementation can be found in the Copy To Texture plugin.
  */
 class GLTexture : public CompRect {
     public:
@@ -81,6 +119,29 @@
 	    Good
 	} Filter;
 
+	/**
+	 * a texture matrix
+	 *
+	 * Texture Matrix manipulation
+	 *
+	 * Since all textures are 2D, it is possible to perform basic 2D
+	 * Manipulation operations on the texture.
+	 *
+	 * Member variable | Function
+	 * ----------------|----------
+	 *          .x0    | X Position on the screen
+	 *          .y0    | Y Position on the screen
+	 *          .xx    | X Scale Factor
+	 *          .yy    | Y Scale Factor
+	 *          .xy    | X Shear Factor
+	 *          .yx    | Y Shear Factor
+	 *
+	 * The COMP_TEX_COORD functions return matrix-adjusted values for
+	 * texture positions. The "real" value of your co-ordinate on the
+	 * texture itself should be inputted into these functions, and these
+	 * functions will return the appropriate number to feed to functions
+	 * such as glTexCoord2f.
+	 */
 	typedef struct {
 	    float xx; float yx;
 	    float xy; float yy;
@@ -90,8 +151,20 @@
 	typedef std::vector<Matrix> MatrixList;
 
 	/**
-	 * Class which represents a list of openGL textures,
-	 * usually used for texture tiling
+	 * a list of OpenGL textures, usually used for texture tiling
+	 *
+	 * In order to overcome hardware maximum texture size limitations,
+	 * Compiz supports tiled texturing. As such, all texture generation
+	 * functions (such as bindPixmapTotexture, imageDataToTexture,
+	 * readImageToTexture, etc) will all return a GLTexture:List. When
+	 * rendering these textures you must loop through each texture in the
+	 * list in order that you render each part of the texture that the user
+	 * sees.
+	 *
+	 * On especially small textures, it is ok to directly access the first
+	 * item in the list for the sake of optimization, however whenever it is
+	 * not clear what the size of the texture is, you should always loop the
+	 * list.
 	 */
 	class List : public std::vector <GLTexture *> {
 
@@ -112,23 +185,23 @@
     public:
 
 	/**
-	 * Returns the openGL texture name
+	 * Returns the OpenGL texture name.
 	 */
 	GLuint name () const;
 
 	/**
-	 * Returns the openGL texture target
+	 * Returns the OpenGL texture target.
 	 */
 	GLenum target () const;
 
 	/**
-	 * Returns the openGL texture filter
+	 * Returns the openGL texture filter.
 	 */
 	GLenum filter () const;
 
 	/**
 	 * Returns a 2D 2x3 matrix describing the transformation of
-	 * the texture
+	 * this texture.
 	 */
 	const Matrix & matrix () const;
 
@@ -148,42 +221,41 @@
 	virtual void disable ();
 
 	/**
-	 * Returns true if this texture is MipMapped
+	 * Returns true if this texture is MipMapped.
 	 */
 	bool mipmap () const;
 
 	/**
-	 * Sets if this texture should be MipMapped
+	 * Sets if this texture should be MipMapped.
 	 */
 	void setMipmap (bool);
 
 	/**
-	 * Sets the openGL filter which should be used on this
-	 * texture
+	 * Sets the openGL filter which should be used on this texture.
 	 */
 	void setFilter (GLenum);
 	void setWrap (GLenum);
 
 	/**
-	 * Increases the reference count of a texture
+	 * Increases the reference count of a texture.
 	 */
 	static void incRef (GLTexture *);
 
 	/**
-	 * Decreases the reference count of a texture
+	 * Decreases the reference count of a texture.
 	 */
 	static void decRef (GLTexture *);
 
 	/**
-	 * Returns a GLTexture::List with the contents of
-	 * some pixmap
+	 * Creates a collection of GLTextures from an X11 pixmap.
+	 * @param pixmap  pixmap data which should be converted
+	 *                into texture data
+	 * @param width   width of the texture
+	 * @param height  height of the texture
+	 * @param depth   color depth of the texture
+	 * @param source  whether the pixmap lifecycle is managed externally
 	 *
-	 * @param pixmap Specifies the pixmap data which should be converted
-	 * into texture data
-	 * @param width Specifies the width of the texture
-	 * @param height Specifies the height of the texture
-	 * @param depth Specifies the color depth of the texture
-	 * @param source Whether the pixmap lifecycle is managed externall
+	 * @returns a GLTexture::List with the contents of the pixmap.
 	 */
 	static List bindPixmapToTexture (Pixmap                       pixmap,
 					 int                          width,
@@ -193,16 +265,23 @@
 					     = compiz::opengl::InternallyManaged);
 
 	/**
-	 * Returns a GLTexture::List with the contents of of
-	 * a raw image buffer
+	 * Creates a collection of GLTextures from a raw image buffer.
+	 * @param image  a raw image buffer which should be converted
+	 *               into texture data
+	 * @param size   size of this new texture
 	 *
-	 * @param image Specifies a raw image buffer which should be converted
-	 * into texture data
-	 * @param size Specifies the size of this new texture
+	 * @returns a GLTexture::List with the contents of of
+	 * a raw image buffer.
 	 */
-	static List imageBufferToTexture (const char *image,
-					  CompSize   size);
+	static List imageBufferToTexture (const char *image, CompSize size);
 
+	/**
+	 * Creates a collection of GLTextures from raw pixel data.
+	 * @param image   raw pixel data
+	 * @param size    size of the raw pixel data
+	 * @param format  GL format of the raw pixel data (eg. GL_RGBA)
+	 * @param type    GL data type of the raw pixel data (eg. GL_FLOAT)
+	 */
 	static List imageDataToTexture (const char *image,
 					CompSize   size,
 					GLenum     format,
@@ -212,10 +291,10 @@
 	 * Uses image loading plugins to read an image from the disk and
 	 * return a GLTexture::List with its contents
 	 *
-	 * @param imageFileName The filename of the image
-	 * @param pluginName	The name of the plugin, used to find
-	 *			the default image path
-	 * @param size		The size of this new texture
+	 * @param imageFileName  filename of the image
+	 * @param pluginName     name of the plugin, used to find the default
+	 *                       image path
+	 * @param size           size of this new texture
 	 */
 	static List readImageToTexture (CompString &imageFileName,
 					CompString &pluginName,
@@ -224,10 +303,25 @@
 	friend class PrivateTexture;
 
     protected:
+	/**
+	 * Consrtucts a defauly empty GLTexture.
+	 *
+	 * @deprecated This function is only for ABI backwards compatibility and
+	 * will be removed in the future.
+	 */
 	GLTexture ();
+
+	/** Constructs an OpenGL texture. */
+	GLTexture (int width, int height, GLenum target, Matrix const& m, bool mipmap);
 	virtual ~GLTexture ();
 
-	void setData (GLenum target, Matrix &m, bool mipmap);
+	/** 
+	 * Initializer.
+	 *
+	 * @deprecated This function is only for ABI backwards compatibility and
+	 * will be removed in the future.
+	 */
+	void setData (GLenum target, Matrix const& m, bool mipmap);
 
     private:
 	PrivateTexture *priv;

=== modified file 'plugins/opengl/src/privatetexture.h'
--- plugins/opengl/src/privatetexture.h	2012-09-18 11:32:20 +0000
+++ plugins/opengl/src/privatetexture.h	2015-03-30 22:52:49 +0000
@@ -2,6 +2,7 @@
  * Copyright © 2008 Dennis Kasprzyk
  * Copyright © 2007 Novell, Inc.
  * Copyright © 2011 Linaro Ltd.
+ * Copyright 2015 Canonical Ltd.
  *
  * Permission to use, copy, modify, distribute, and sell this software
  * and its documentation for any purpose is hereby granted without
@@ -49,7 +50,10 @@
 
 class PrivateTexture {
     public:
-	PrivateTexture (GLTexture *);
+	PrivateTexture (GLTexture*               texture,
+	                GLenum                   target,
+	                GLTexture::Matrix const& matrix,
+	                bool                     mipmap);
 	~PrivateTexture ();
 
 	static GLTexture::List loadImageData (const char   *image,
@@ -74,7 +78,6 @@
 #ifdef USE_GLES
 class EglTexture : public GLTexture {
     public:
-	EglTexture ();
 	~EglTexture ();
 
 	void enable (Filter filter);
@@ -85,6 +88,13 @@
 					 int                          depth,
 					 compiz::opengl::PixmapSource source);
 
+    private:
+	EglTexture (GLenum                   target,
+	            GLTexture::Matrix const& matrix,
+	            bool                     mipmap,
+	            int                      width,
+	            int                      height);
+
     public:
 	bool        damaged;
 	Damage      damage;
@@ -96,7 +106,6 @@
 
 class TfpTexture : public GLTexture {
     public:
-	TfpTexture ();
 	~TfpTexture ();
 
 	void enable (Filter filter);
@@ -109,6 +118,16 @@
 					 int                          depth,
 					 compiz::opengl::PixmapSource source);
 
+    private:
+	TfpTexture (GLenum                       target,
+	            GLTexture::Matrix const&     matrix,
+	            bool                         mipmap,
+	            Pixmap                       pixmap,
+	            int                          width,
+	            int                          height,
+	            Pixmap                       x11Pixmap,
+	            compiz::opengl::PixmapSource source);
+
     public:
 	Pixmap                       x11Pixmap;
 	GLXPixmap                    pixmap;

=== modified file 'plugins/opengl/src/texture.cpp'
--- plugins/opengl/src/texture.cpp	2014-03-03 16:20:06 +0000
+++ plugins/opengl/src/texture.cpp	2015-03-30 22:52:49 +0000
@@ -1,5 +1,6 @@
 /*
  * Copyright © 2005 Novell, Inc.
+ * Copyright 2015 Canonical Ltd.
  *
  * Permission to use, copy, modify, distribute, and sell this software
  * and its documentation for any purpose is hereby granted without
@@ -48,7 +49,7 @@
 std::map<Damage, TfpTexture*> boundPixmapTex;
 #endif
 
-static GLTexture::Matrix _identity_matrix = {
+static const GLTexture::Matrix _identity_matrix = {
     1.0f, 0.0f,
     0.0f, 1.0f,
     0.0f, 0.0f
@@ -107,7 +108,13 @@
 
 GLTexture::GLTexture () :
     CompRect (0, 0, 0, 0),
-    priv (new PrivateTexture (this))
+    priv (new PrivateTexture (this, GL_TEXTURE_2D, _identity_matrix, true))
+{
+}
+
+GLTexture::GLTexture (int width, int height, GLenum target, Matrix const& matrix, bool mipmap) :
+    CompRect (0, 0, width, height),
+    priv (new PrivateTexture (this, target, matrix, mipmap))
 {
 }
 
@@ -117,14 +124,17 @@
 	delete priv;
 }
 
-PrivateTexture::PrivateTexture (GLTexture *texture) :
+PrivateTexture::PrivateTexture (GLTexture*               texture,
+                                GLenum                   target,
+                                GLTexture::Matrix const& matrix,
+                                bool                     mipmap) :
     texture (texture),
     name (0),
-    target (GL_TEXTURE_2D),
+    target (target),
     filter (GL_NEAREST),
     wrap   (GL_CLAMP_TO_EDGE),
-    matrix (_identity_matrix),
-    mipmap  (true),
+    matrix (matrix),
+    mipmap  (mipmap),
     mipmapSupport (false),
     initial (true),
     refCount (1)
@@ -255,7 +265,7 @@
 }
 
 void
-GLTexture::setData (GLenum target, Matrix &m, bool mipmap)
+GLTexture::setData (GLenum target, Matrix const& m, bool mipmap)
 {
     priv->target = target;
     priv->matrix = m;
@@ -306,9 +316,6 @@
 	return GLTexture::List ();
 
     GLTexture::List rv (1);
-    GLTexture *t = new GLTexture ();
-    rv[0] = t;
-
     GLTexture::Matrix matrix = _identity_matrix;
     GLint             internalFormat;
     GLenum            target;
@@ -341,10 +348,11 @@
     }
     #endif
 
-    t->setData (target, matrix, mipmap);
-    t->setGeometry (0, 0, width, height);
+    GLTexture* t = new GLTexture (width, height, target, matrix, mipmap);
     t->setFilter (GL_NEAREST);
     t->setWrap (GL_CLAMP_TO_EDGE);
+    rv[0] = t;
+
 
     #ifdef USE_GLES
     // For GLES2 no format conversion is allowed, i.e., format must equal internalFormat
@@ -468,7 +476,12 @@
 }
 
 #ifdef USE_GLES
-EglTexture::EglTexture () :
+EglTexture::EglTexture (GLenum                   target,
+                        GLTexture::Matrix const& matrix,
+                        bool                     mipmap,
+                        int                      width,
+                        int                      height) :
+    GLTexture (width, height, target, matrix, mipmap),
     damaged (true),
     damage (None),
     updateMipMap (true)
@@ -489,16 +502,19 @@
 }
 
 GLTexture::List
-EglTexture::bindPixmapToTexture (Pixmap                       pixmap,
-				 int                          width,
-				 int                          height,
-				 int                          depth,
-				 compiz::opengl::PixmapSource source)
+EglTexture::bindPixmapToTexture (Pixmap            pixmap,
+                                 int               width,
+                                 int               height,
+                                 int               depth,
+                                 cgl::PixmapSource source)
 {
     GLTexture::List   rv (1);
     EglTexture        *tex = NULL;
     EGLImageKHR       eglImage = NULL;
+    GLenum            texTarget = GL_TEXTURE_2D;
     GLTexture::Matrix matrix = _identity_matrix;
+    bool              mipmap = GL::textureNonPowerOfTwoMipmap ||
+                              (POWER_OF_TWO (width) && POWER_OF_TWO (height));
 
     const EGLint img_attribs[] = {
 	EGL_IMAGE_PRESERVED_KHR, EGL_TRUE,
@@ -521,23 +537,19 @@
     matrix.yy = 1.0f / height;
     matrix.y0 = 0.0f;
 
-    tex = new EglTexture ();
-    tex->setData (GL_TEXTURE_2D, matrix,
-	GL::textureNonPowerOfTwoMipmap ||
-	(POWER_OF_TWO (width) && POWER_OF_TWO (height)));
-    tex->setGeometry (0, 0, width, height);
+    tex = new EglTexture (texTarget, matrix, mipmap, width, height);
 
     rv[0] = tex;
 
-    glBindTexture (GL_TEXTURE_2D, tex->name ());
+    glBindTexture (texTarget, tex->name ());
 
-    GL::eglImageTargetTexture (GL_TEXTURE_2D, (GLeglImageOES)eglImage);
+    GL::eglImageTargetTexture (texTarget, (GLeglImageOES)eglImage);
     GL::destroyImage (eglGetDisplay (screen->dpy ()), eglImage);
 
     tex->setFilter (GL_NEAREST);
     tex->setWrap (GL_CLAMP_TO_EDGE);
 
-    glBindTexture (GL_TEXTURE_2D, 0);
+    glBindTexture (texTarget, 0);
 
     tex->damage = XDamageCreate (screen->dpy (), pixmap,
 			         XDamageReportBoundingBox);
@@ -609,11 +621,21 @@
     }
 }
 
-TfpTexture::TfpTexture () :
-    pixmap (0),
+TfpTexture::TfpTexture (GLenum                   target,
+                        GLTexture::Matrix const& matrix,
+                        bool                     mipmap,
+                        Pixmap                   pixmap,
+                        int                      width,
+                        int                      height,
+                        Pixmap                   x11Pixmap,
+                        cgl::PixmapSource        source) :
+    GLTexture (width, height, target, matrix, mipmap),
+    x11Pixmap (x11Pixmap),
+    pixmap (pixmap),
     damaged (true),
     damage (None),
-    updateMipMap (true)
+    updateMipMap (true),
+    source (source)
 {
 }
 
@@ -791,12 +813,10 @@
 	    return GLTexture::List ();
     }
 
-    tex = new TfpTexture ();
-    tex->setData (texTarget, matrix, mipmap);
-    tex->setGeometry (0, 0, width, height);
-    tex->pixmap = glxPixmap;
-    tex->x11Pixmap = pixmap;
-    tex->source = source;
+    tex = new TfpTexture (texTarget, matrix, mipmap,
+                          glxPixmap,
+                          width, height,
+                          pixmap, source);
 
     rv[0] = tex;