Merge lp://qastaging/~compiz-linaro-team/compiz/gles2 into lp://qastaging/compiz/0.9.8

The diff is quite large, however it should be possible still to do a code review:

+static bool
+unproject (float winx, float winy, float winz,
+ const GLMatrix &modelview,
+ const GLMatrix &projection,
+ const GLint viewport[4],
+ float *objx, float *objy, float *objz)
+{
+ GLMatrix finalMatrix = projection * modelview;
+ float in[4], out[4];
+
+ if (!finalMatrix.invert ())
+ return false;
+
+ in[0] = winx;
+ in[1] = winy;
+ in[2] = winz;
+ in[3] = 1.0;
+
+ /* Map x and y from window coordinates */
+ in[0] = (in[0] - viewport[0]) / viewport[2];
+ in[1] = (in[1] - viewport[1]) / viewport[3];
+
+ /* Map to range -1 to 1 */
+ in[0] = in[0] * 2 - 1;
+ in[1] = in[1] * 2 - 1;
+ in[2] = in[2] * 2 - 1;
+
+ for (int i = 0; i < 4; i++)
+ {
+ out[i] = in[0] * finalMatrix[i] +
+ in[1] * finalMatrix[4 + i] +
+ in[2] * finalMatrix[8 + i] +
+ in[3] * finalMatrix[12 + i];
+ }
+
+ if (out[3] == 0.0)
+ return false;
+
+ out[0] /= out[3];
+ out[1] /= out[3];
+ out[2] /= out[3];
+
+ *objx = out[0];
+ *objy = out[1];
+ *objz = out[2];
+
+ return true;
+}
+

+static bool
+project (float objx, float objy, float objz,
+ const float modelview[16], const float projection[16],
+ const GLint viewport[4],
+ float *winx, float *winy, float *winz)
+{
+ unsigned int i;
+ float in[4];
+ float out[4];
+
+ in[0] = objx;
+ in[1] = objy;
+ in[2] = objz;
+ in[3] = 1.0;
+
+ for (i = 0; i < 4; i++) {
+ out[i] =
+ in[0] * modelview[i] +
+ in[1] * modelview[4 + i] +
+ in[2] * modelview[8 + i] +
+ in[3] * modelview[12 + i];
+ }
+
+ for (i = 0; i < 4; i++) {
+ in[i] =
+ out[0] * projection[i] +
+ out[1] * projection[4 + i] +
+ out[2] * projection[8 + i] +
+ out[3] * projection[12 + i];
+ }
+
+ if (in[3] == 0.0)
+ return false;
+
+ in[0] /= in[3];
+ in[1] /= in[3];
+ in[2] /= in[3];
+ /* Map x, y and z to range 0-1 */
+ in[0] = in[0] * 0.5 + 0.5;
+ in[1] = in[1] * 0.5 + 0.5;
+ in[2] = in[2] * 0.5 + 0.5;
+
+ /* Map x,y to viewport */
+ in[0] = in[0] * viewport[2] + viewport[0];
+ in[1] = in[1] * viewport[3] + viewport[1];
+
+ *winx = in[0];
+ *winy = in[1];
+ *winz = in[2];
+ return true;
+}

This is re-usable in cube and should be put somewhere. GLMatrix?

=== added file 'cmake/FindOpenGLES2.cmake'
--- cmake/FindOpenGLES2.cmake 1970-01-01 00:00:00 +0000
+++ cmake/FindOpenGLES2.cmake 2012-08-10 08:27:48 +0000
@@ -0,0 +1,51 @@
+# - Try to find OpenGLES
+# Once done this will define
+#
+# OPENGLES2_FOUND - system has OpenGLES
+# OPENGLES2_INCLUDE_DIR - the GLES include directory
+# OPENGLES2_LIBRARY - the GLES library
+# OPENGLES2_LIBRARIES - Link this to use OpenGLES
+#
+
+FIND_PATH(OPENGLES2_INCLUDE_DIR GLES2/gl2.h
+ /usr/openwin/share/include
+ /opt/graphics/OpenGL/include /usr/X11R6/include
+ /usr/include
+)
+
+FIND_LIBRARY(OPENGLES2_LIBRARY
+ NAMES GLESv2
+ PATHS /opt/graphics/OpenGL/lib
+ /usr/openwin/lib
+ /usr/shlib /usr/X11R6/lib
+ /usr/lib
+)
+
+FIND_L...

Thanks Sam. Please try to keep your comments a manageable size. Or make each point in a new comment.

Initially I thought *project should go in GLMatrix too. However then I cleaned up project() and realized it should not be using GLMatrix. It looks like unproject similarly should not be using GLMatrix. Though *project() should probably live somewhere in the opengl plugin, I agree.

Set to work in progress again.

We cannot realistically consider merging this until the incompatibilities with Unity are also fixed:
https://bugs.launchpad.net/unity/+bugs?field.tag=gles

> 2. It's ugly, bad design. Member data should not be repeatedly passed as
     a parameter. I know there is a limitation in the current wrapping/
     interface design that makes it impossible to access vertexBuffer as a
     member, but that's no excuse for making the code worse than it already
     was.

Can you clarify this please? It didn't come up in the original review and I'm suprised you've put it as a reason for reverting it now.

While passing member data through function arguments is not great, it at least restricts the scope in which that data can be accessed, which is the correct thing to do.

I can understand reverting it in the meantime because unity is difficult to port with ifdefs, but if you're going to do so please consult with the person who wrote it (who happens to live in the same timezone as you, so its really not an issue to do so) for advice on how to handle it rather than taking the unilateral approach of just reverting code.

I was extremely frustrated yesterday and just wanted the code to build. I wasn't feeling like consulting.

Sorry, I thought it did come up in the original review. I was thinking it all along but (as usual) failed to articulate all of my thoughts into words.

It's foreseeable that I will approve the changes again. But not until Unity has been updated to support them first. That would be enclosed in "#if COMPIZ_OPENGL_ABI >= 6", I imagine.

> I was extremely frustrated yesterday and just wanted the code to build. I
> wasn't feeling like consulting.
>
> Sorry, I thought it did come up in the original review. I was thinking it all
> along but (as usual) failed to articulate all of my thoughts into words.
>
> It's foreseeable that I will approve the changes again. But not until Unity
> has been updated to support them first. That would be enclosed in "#if
> COMPIZ_OPENGL_ABI >= 6", I imagine.

Okay, thanks. Noted, will re-propose the unrevert of those changes at an appropriate time.

Cheers,

Sam