Merge lp:~gebner/compiz/compiz.vertexbuffer-autoprogram-uniform-segfault into lp:compiz/0.9.10

[Sam Spilsbury]

Adding a uniform to a GLVertexBuffer that uses AutoProgram causes compiz to segfault.

Example:
  gWindow->addShaders("cms", "", fragment_shader);
  gWindow->vertexBuffer()->addUniform("cms_lut", unit);
  // segfault happens later in PrivateVertexBuffer::render

The patch modifies PrivateVertexBuffer::render to set the uniform on the generated AutoProgram instead of the provided program, which in this case is NULL, causing a segfault.

(LP: #1162598)

Adding a uniform to a GLVertexBuffer that uses AutoProgram causes compiz to segfault.

Example:
  gWindow->addShaders("cms", "", fragment_shader);
  gWindow->vertexBuffer()->addUniform("cms_lut", unit);
  // segfault happens later in PrivateVertexBuffer::render

The patch modifies PrivateVertexBuffer::render to set the uniform on the generated AutoProgram instead of the provided program, which in this case is NULL, causing a segfault.

This feature is used in a plugin for full-screen color management, compiz-cms[1], where the unit of a color transformation texture is passed as a uniform.

[1] https://github.com/gebner/compiz-cms

[Sam Spilsbury (smspillaz)]

This makes sense, thanks.

This reminds me that I need to fix the VertexBuffer class to behave in a more sane way when it comes to having no active program set. At the moment it just crashes if you don't call setProgram or setAutoProgram. I remember I wanted to enforce something in its constructor, but at the time didn't want to break ABI. I've filed bug 1162597 about that.

One more thing before we merge this - can you add commit metadata to link this to a bug? I didn't see anything, so I've filed bug 1162598. You can link it by doing this:

bzr commit --empty --fixes lp:1162598
bzr push

On another note, nice work with the color management plugin. If you want it to be included in-tree, just file a bug about it on launchpad and we'll do the paperwork to get it in.

review: Approve

[Daniel van Vugt (vanvugt)]

Please re-target this for lp:compiz/0.9.10

review: Needs Resubmitting

[Sam Spilsbury (smspillaz)]

As above

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3645
http://jenkins.qa.ubuntu.com/job/compiz-ci/116/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/153/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/505

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

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

The "server already running" thing could possibly be caused by:

1. A stale connection on the CI server
2. A race condition where multiple CI jobs are running on the same system (and they both pick :133)

We should probably fix the xorg-gtest code to try and pick another socket in that case, but for now.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'plugins/opengl/src/vertexbuffer.cpp'
--- plugins/opengl/src/vertexbuffer.cpp	2012-11-04 16:15:51 +0000
+++ plugins/opengl/src/vertexbuffer.cpp	2013-04-02 07:11:15 +0000
@@ -523,7 +523,7 @@
     // set per-plugin uniforms
     for (unsigned int i = 0; i < uniforms.size (); i++)
     {
-	uniforms[i]->set (program);
+	uniforms[i]->set (tmpProgram);
     }
 
     //convert paint attribs to 0-1 range