Merge lp:~bitzesmichail/compiz/splash-gles into lp:compiz/0.9.9

[Michail Bitzes]

Plugin splash ported to OpenGL|ES.

Use GLVertexBuffer.
Enable building for GLES. Enable architectures armel and armhf.

[MC Return (mc-return)]

Compiles without any warnings, works and code also looks good. :)
Just tested the GL desktop build though, but I am sure GLES works as well...

Top job (as always), Michail, I would say. Thanks a lot.

review: Approve

[Sam Spilsbury (smspillaz)]

This all looks great, all according to how it should work. My only two general comments:

262 + stream->colorDefault ();

That is probably not necessary (the new shader based rendering model isn't stateful, I believe the active color is reset on GLVertexBuffer::begin ()

2. Generally speaking, its a lot easier to port old code that uses GL_QUADS to use GL_TRIANGLE_STRIP instead of GL_TRIANGLES as the primitive type, since the vertices are often arranged to wind in right-angled triangles (eg, x1y1, x1y2, x2y1, x2y2), and then you can just generate the first triangle and only take the fourth-point for every other triangle after that.

[Sam Spilsbury (smspillaz)]

review: Approve

[Michail Bitzes (bitzesmichail)]

> This all looks great, all according to how it should work. My only two general
> comments:
>
> 262 + stream->colorDefault ();
>
> That is probably not necessary (the new shader based rendering model isn't
> stateful, I believe the active color is reset on GLVertexBuffer::begin ()

Weird. In my code, I used color4f () before begin () and it works. Is this wrong?

As an experiment, I changed color4f(1.0,1.0,1.0,alpha) to color4f(1.0,0.0,0.0,alpha). Without colorDefault (), the red color is leaked outside the splash logo, and when the animation finishes, the screen is left a red mess.

>
> 2. Generally speaking, its a lot easier to port old code that uses GL_QUADS to
> use GL_TRIANGLE_STRIP instead of GL_TRIANGLES as the primitive type, since the
> vertices are often arranged to wind in right-angled triangles (eg, x1y1, x1y2,
> x2y1, x2y2), and then you can just generate the first triangle and only take
> the fourth-point for every other triangle after that.

Thanks, I'll consider this approach next time.

[MC Return (mc-return)]

Michail, 3 questions:

1. During testing I found that the option Main->"Show on first start" does not work correctly. If disabled Splash will still show on Compiz start, no matter if first or second. Could you try to fix that also, when you are on it ?

2. We introduced some minor indentation problems in the Firepaint GL|ES merge and optimized the if statements in Showmouse, do you want to propose a MP with the Firepaint fixes (indentation and better readability for the if statements), or should I do it ?

3. Unfortunately the Wizard is still a bit unstable when you start to customize the particles, while working stable with the defaults, did you make progress/are you still working on it and the GL|ES version ?

[Michail Bitzes (bitzesmichail)]

 > 1. During testing I found that the option Main->"Show on first start" does not
> work correctly. If disabled Splash will still show on Compiz start, no matter
> if first or second. Could you try to fix that also, when you are on it ?

I can look at it tomorrow.

> 2. We introduced some minor indentation problems in the Firepaint GL|ES merge
> and optimized the if statements in Showmouse, do you want to propose a MP with
> the Firepaint fixes (indentation and better readability for the if
> statements), or should I do it ?

You can do it.

> 3. Unfortunately the Wizard is still a bit unstable when you start to
> customize the particles, while working stable with the defaults, did you make
> progress/are you still working on it and the GL|ES version ?

Unstable = slow or crash?
I made a branch with the GLES port, I haven't proposed it yet.

[MC Return (mc-return)]

> I can look at it tomorrow.
>
Thanks.

> You can do it.
>
+1

> Unstable = slow or crash?
> I made a branch with the GLES port, I haven't proposed it yet.

Unfortunately crash, when messing with the individual particles, but
I noticed caching works a bit different, so it might have to do with
that and so it might already be fixed in your port...

Great to hear you're making progress on that front :)

[Michail Bitzes (bitzesmichail)]

> 1. During testing I found that the option Main->"Show on first start" does not
> work correctly. If disabled Splash will still show on Compiz start, no matter
> if first or second. Could you try to fix that also, when you are on it ?

I worked on this: optionGetFirststart () always returns true (the default value) when called in the constructor of SplashScreen.

I don't know enough yet to find a solution, so this branch should proceed without a fix for this.

[MC Return (mc-return)]

>
> I worked on this: optionGetFirststart () always returns true (the default
> value) when called in the constructor of SplashScreen.
>
> I don't know enough yet to find a solution, so this branch should proceed
> without a fix for this.

Ok, not a big problem, and not related to this MP - we'll fix that later then.

review: Approve

[Sam Spilsbury (smspillaz)]

> > This all looks great, all according to how it should work. My only two
> general
> > comments:
> >
> > 262 + stream->colorDefault ();
> >
> > That is probably not necessary (the new shader based rendering model isn't
> > stateful, I believe the active color is reset on GLVertexBuffer::begin ()
>
> Weird. In my code, I used color4f () before begin () and it works. Is this
> wrong?
>
> As an experiment, I changed color4f(1.0,1.0,1.0,alpha) to
> color4f(1.0,0.0,0.0,alpha). Without colorDefault (), the red color is leaked
> outside the splash logo, and when the animation finishes, the screen is left a
> red mess.

This should probably be fixed in the GLVertexBuffer implementation.

>
> >
> > 2. Generally speaking, its a lot easier to port old code that uses GL_QUADS
> to
> > use GL_TRIANGLE_STRIP instead of GL_TRIANGLES as the primitive type, since
> the
> > vertices are often arranged to wind in right-angled triangles (eg, x1y1,
> x1y2,
> > x2y1, x2y2), and then you can just generate the first triangle and only take
> > the fourth-point for every other triangle after that.
>
> Thanks, I'll consider this approach next time.

[Michail Bitzes (bitzesmichail)]

Changed the code to work if GLVertexBuffer is changed/fixed.

[Sam Spilsbury (smspillaz)]

Dunno why this wasn't marked for merging earlier. Thanks!

review: Approve

[Sam Spilsbury (smspillaz)]

Apologies for the delay, marking for merging again.

See https://lists.launchpad.net/unity-dev/msg00598.html

=== modified file 'debian/compiz-plugins.install.armel'
--- debian/compiz-plugins.install.armel	2013-01-26 19:08:07 +0000
+++ debian/compiz-plugins.install.armel	2013-02-07 10:29:22 +0000
@@ -35,6 +35,8 @@
 debian/tmp/usr/*/compiz/*showdesktop.*
 debian/tmp/usr/*/compiz/*showmouse.*
 debian/tmp/usr/share/compiz/showmouse
+debian/tmp/usr/*/compiz/*splash.*
+debian/tmp/usr/share/compiz/splash
 debian/tmp/usr/*/compiz/*switcher.*
 debian/tmp/usr/*/compiz/*text.*
 debian/tmp/usr/*/compiz/*titleinfo.*
 
=== modified file 'debian/compiz-plugins.install.armhf'
--- debian/compiz-plugins.install.armhf	2013-01-27 00:29:34 +0000
+++ debian/compiz-plugins.install.armhf	2013-02-07 10:29:22 +0000
@@ -35,6 +35,8 @@
 debian/tmp/usr/*/compiz/*showdesktop.*
 debian/tmp/usr/*/compiz/*showmouse.*
 debian/tmp/usr/share/compiz/showmouse
+debian/tmp/usr/*/compiz/*splash.*
+debian/tmp/usr/share/compiz/splash
 debian/tmp/usr/*/compiz/*switcher.*
 debian/tmp/usr/*/compiz/*text.*
 debian/tmp/usr/*/compiz/*titleinfo.*
 
=== modified file 'plugins/CMakeLists.txt'
--- plugins/CMakeLists.txt	2013-02-02 23:01:02 +0000
+++ plugins/CMakeLists.txt	2013-02-07 10:29:22 +0000
@@ -46,7 +46,6 @@
     set (COMPIZ_DISABLE_PLUGIN_BENCH ON)
     set (COMPIZ_DISABLE_PLUGIN_SHOWREPAINT ON)
     set (COMPIZ_DISABLE_PLUGIN_WIDGET ON)
-    set (COMPIZ_DISABLE_PLUGIN_SPLASH ON)
     set (COMPIZ_DISABLE_PLUGIN_THUMBNAIL ON)
     set (COMPIZ_DISABLE_PLUGIN_WIZARD ON)
 
 
=== modified file 'plugins/splash/src/splash.cpp'
--- plugins/splash/src/splash.cpp	2012-09-07 23:29:42 +0000
+++ plugins/splash/src/splash.cpp	2013-02-07 10:29:22 +0000
@@ -186,6 +186,7 @@
 		       	     unsigned int mask)
 {
     GLMatrix sTransform = transform;
+    GLVertexBuffer *stream = GLVertexBuffer::streamingBuffer ();
 
     bool status = true;
 
@@ -208,12 +209,8 @@
 
     sTransform.toScreenSpace (output, -DEFAULT_Z_CAMERA);
 
-    glPushMatrix ();
-    glLoadMatrixf (sTransform.getMatrix ());
     glEnable (GL_BLEND);
     glBlendFunc (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
-    glColor4f (1.0, 1.0, 1.0, alpha);
-    glTexEnvf (GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE);
 
     if (back_img.size ())
     {
@@ -229,7 +226,6 @@
 		mesh[x][y][1] =
 		    (y / (MESH_H - 1.0) ) +
 		    (0.02 * sin ( (mesh[x][y][0] * 8) + mMove) );
-		;
 	    }
 	}
 
@@ -256,11 +252,15 @@
 
 	    GLTexture::Matrix mat = tex->matrix ();
 
-	    glTranslatef (x, y, 0);
+	    sTransform.translate (x, y, 0);
 
 	    float cx1, cx2, cy1, cy2;
 
-	    glBegin (GL_QUADS);
+	    std::vector<GLfloat> coords;
+	    std::vector<GLfloat> vertices;
+
+	    coords.reserve (12 * (MESH_W - 1) * (MESH_H - 1));
+	    vertices.reserve (18 * (MESH_W - 1) * (MESH_H - 1));
 
 	    for (x = 0; x < MESH_W - 1; x++)
 	    {
@@ -271,30 +271,56 @@
 		    cy1 = (y / (MESH_H - 1.0) ) * backSize.height ();
 		    cy2 = ( (y + 1) / (MESH_H - 1.0) ) * backSize.height ();
 
-		    glTexCoord2f (COMP_TEX_COORD_X (mat, cx1),
-				  COMP_TEX_COORD_Y (mat, cy1) );
-		    glVertex2f (mesh[x][y][0] *
-			        backSize.width (),
-			        mesh[x][y][1] * backSize.height ());
-		    glTexCoord2f (COMP_TEX_COORD_X (mat, cx1),
-				  COMP_TEX_COORD_Y (mat, cy2) );
-		    glVertex2f (mesh[x][y + 1][0] *
-				backSize.width (),
-				mesh[x][y + 1][1] * backSize.height ());
-		    glTexCoord2f (COMP_TEX_COORD_X (mat, cx2),
-				  COMP_TEX_COORD_Y (mat, cy2) );
-		    glVertex2f (mesh[x + 1][y + 1][0] *
-				backSize.width (),
-				mesh[x + 1][y + 1][1] * backSize.height ());
-		    glTexCoord2f (COMP_TEX_COORD_X (mat, cx2),
-				  COMP_TEX_COORD_Y (mat, cy1) );
-		    glVertex2f (mesh[x + 1][y][0] *
-				backSize.width (),
-				mesh[x + 1][y][1] * backSize.height ());
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx1));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy1));
+
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx1));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy2));
+
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx2));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy2));
+
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx2));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy2));
+
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx2));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy1));
+
+		    coords.push_back (COMP_TEX_COORD_X (mat, cx1));
+		    coords.push_back (COMP_TEX_COORD_Y (mat, cy1));
+
+		    vertices.push_back (mesh[x][y][0] * backSize.width ());
+		    vertices.push_back (mesh[x][y][1] * backSize.height ());
+		    vertices.push_back (0.0f);
+
+		    vertices.push_back (mesh[x][y + 1][0] * backSize.width ());
+		    vertices.push_back (mesh[x][y + 1][1] * backSize.height ());
+		    vertices.push_back (0.0f);
+
+		    vertices.push_back (mesh[x + 1][y + 1][0] * backSize.width ());
+		    vertices.push_back (mesh[x + 1][y + 1][1] * backSize.height ());
+		    vertices.push_back (0.0f);
+
+		    vertices.push_back (mesh[x + 1][y + 1][0] * backSize.width ());
+		    vertices.push_back (mesh[x + 1][y + 1][1] * backSize.height ());
+		    vertices.push_back (0.0f);
+
+		    vertices.push_back (mesh[x + 1][y][0] * backSize.width ());
+		    vertices.push_back (mesh[x + 1][y][1] * backSize.height ());
+		    vertices.push_back (0.0f);
+
+		    vertices.push_back (mesh[x][y][0] * backSize.width ());
+		    vertices.push_back (mesh[x][y][1] * backSize.height ());
+		    vertices.push_back (0.0f);
 		}
 	    }
 
-	    glEnd ();
+	    stream->begin (GL_TRIANGLES);
+	    stream->color4f (1.0, 1.0, 1.0, alpha);
+	    stream->addVertices (vertices.size () / 3, &vertices[0]);
+	    stream->addTexCoords (0, coords.size () / 2, &coords[0]);
+	    if (stream->end ())
+		stream->render (sTransform);
 
 	    if (screen->outputDevs ().size () > 1)
 	    {
@@ -313,7 +339,7 @@
 		y = (screen->height () - backSize.height ()) / 2;
 	    }
 
-	    glTranslatef (-x, -y, 0);
+	    sTransform.translate (-x, -y, 0);
 
 	    tex->disable ();
 
@@ -346,34 +372,68 @@
 
 	    GLTexture::Matrix mat = tex->matrix ();
 
-	    glTranslatef (x, y, 0);
-
-	    glBegin (GL_QUADS);
-	    glTexCoord2f (COMP_TEX_COORD_X (mat, 0), COMP_TEX_COORD_Y (mat, 0) );
-	    glVertex2f (0, 0);
-	    glTexCoord2f (COMP_TEX_COORD_X (mat, 0),
-			  COMP_TEX_COORD_Y (mat, logoSize.height ()) );
-	    glVertex2f (0, logoSize.height ());
-	    glTexCoord2f (COMP_TEX_COORD_X (mat, logoSize.width ()),
-			  COMP_TEX_COORD_Y (mat, logoSize.height ()) );
-	    glVertex2f (logoSize.width (), logoSize.height ());
-	    glTexCoord2f (COMP_TEX_COORD_X (mat, logoSize.width ()),
-			  COMP_TEX_COORD_Y (mat, 0) );
-	    glVertex2f (logoSize.width (), 0);
-	    glEnd ();
-
-	    glTranslatef (-x, -y, 0);
+	    sTransform.translate (x, y, 0);
+
+	    GLfloat coords[12];
+	    GLfloat vertices[18];
+
+	    coords[0] = COMP_TEX_COORD_X (mat, 0);
+	    coords[1] = COMP_TEX_COORD_Y (mat, 0);
+
+	    coords[2] = COMP_TEX_COORD_X (mat, 0);
+	    coords[3] = COMP_TEX_COORD_Y (mat, logoSize.height ());
+
+	    coords[4] = COMP_TEX_COORD_X (mat, logoSize.width ());
+	    coords[5] = COMP_TEX_COORD_Y (mat, logoSize.height ());
+
+	    coords[6] = COMP_TEX_COORD_X (mat, logoSize.width ());
+	    coords[7] = COMP_TEX_COORD_Y (mat, logoSize.height ());
+
+	    coords[8] = COMP_TEX_COORD_X (mat, logoSize.width ());
+	    coords[9] = COMP_TEX_COORD_Y (mat, 0);
+
+	    coords[10] = COMP_TEX_COORD_X (mat, 0);
+	    coords[11] = COMP_TEX_COORD_Y (mat, 0);
+
+	    vertices[0] = 0;
+	    vertices[1] = 0;
+	    vertices[2] = 0;
+
+	    vertices[3] = 0;
+	    vertices[4] = logoSize.height ();
+	    vertices[5] = 0;
+
+	    vertices[6] = logoSize.width ();
+	    vertices[7] = logoSize.height ();
+	    vertices[8] = 0;
+
+	    vertices[9] = logoSize.width ();
+	    vertices[10] = logoSize.height ();
+	    vertices[11] = 0;
+
+	    vertices[12] = logoSize.width ();
+	    vertices[13] = 0;
+	    vertices[14] = 0;
+
+	    vertices[15] = 0;
+	    vertices[16] = 0;
+	    vertices[17] = 0;
+
+	    stream->begin (GL_TRIANGLES);
+	    stream->color4f (1.0, 1.0, 1.0, alpha);
+	    stream->addVertices (6, vertices);
+	    stream->addTexCoords (0, 6, coords);
+	    if (stream->end ())
+		stream->render (sTransform);
+
+	    sTransform.translate (-x, -y, 0);
 
 	    tex->disable ();
 	}
     }
 
-    glTexEnvf (GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE);
-
     glDisable (GL_BLEND);
     glBlendFunc (GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
-    glColor4usv (defaultColor);
-    glPopMatrix ();
     return status;
 }