Compiz

Merge lp://qastaging/~vanvugt/compiz/fix-1037411 into lp://qastaging/compiz/0.9.8

fix-1037411
Merge into 0.9.8

Proposed by Daniel van Vugt on 2012-09-12

Status:

Merged

Approved by:

Sam Spilsbury on 2012-09-13

Approved revision:

3371

Merged at revision:

3370

Proposed branch:

lp://qastaging/~vanvugt/compiz/fix-1037411

Merge into:

lp://qastaging/compiz/0.9.8

Diff against target:

174 lines (+52/-9)

5 files modified

plugins/opengl/include/opengl/doublebuffer.h (+3/-1)
plugins/opengl/src/doublebuffer/src/double-buffer.cpp (+9/-1)
plugins/opengl/src/doublebuffer/tests/test-opengl-double-buffer.cpp (+27/-0)
plugins/opengl/src/privates.h (+2/-0)
plugins/opengl/src/screen.cpp (+11/-7)

To merge this branch:

bzr merge lp://qastaging/~vanvugt/compiz/fix-1037411

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
jenkins (community)	continuous-integration		Approve on 2012-09-12
Compiz Maintainers		2012-09-12	Pending
Review via email: mp+123878@code.qastaging.launchpad.net

Commit message

Workaround SubBuffer performance regression (LP: #1037411), which is actually
Mesa bug #54763.

Description of the change

Workaround SubBuffer performance regression (LP: #1037411), which is actually
Mesa bug #54763.

Make sure copyFrontToBack is _never_ called unless it's actually needed. Or else you will trigger Mesa bug #54763 (which is a /feature/ from what I've been told so far).

Revision history for this message

jenkins (martin-mrazik+qa) wrote on 2012-09-12:

PASSED: Continuous integration, rev:3370
http://jenkins.qa.ubuntu.com/job/compiz-ci/70/
Executed test runs:
SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/70/console

review: Approve (continuous-integration)

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-09-12:

Looking good. I'm not sure if I like the stateful design, but this can be dealt with elsewhere.

86 + db.set (DoubleBuffer::NEED_PERSISTENT_BACK_BUFFER, false);

This should be unnecessary, as the class will be re-constructed upon every test run. If its touching static data that's bad.

For the EGL case, if we do want to enable swap-by-copying in cases where framebuffer objects are not available, you can set the surface attrib in glInitContext if postSubBufferNV and framebuffer objects are not available (very unlikely), eg

eglSurfaceAttrib (EGL_SWAP_BEHAVIOUR, EGL_PRESERVED);

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-09-12:

I agree line 86 may be redundant, but I was covering for the case where you may not know which test within the fixture gets executed next. At least it's defensive against future maintenance.

I know EGL has the preservation option. There's a similar extension for GLX, but it was never fully implemented when I tried it last.

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-09-12:

> I agree line 86 may be redundant, but I was covering for the case where you
> may not know which test within the fixture gets executed next. At least it's
> defensive against future maintenance.

This doesn't matter. The object is destructed upon test completion.

>
> I know EGL has the preservation option. There's a similar extension for GLX,
> but it was never fully implemented when I tried it last.

Curious. What was it called ?

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-09-12:

http://www.opengl.org/registry/specs/OML/glx_swap_method.txt