Code review comment for lp:~smspillaz/compiz/compiz.gles2.always-swapbuffers

> It seems to work very nicely on intel. I have a couple of concerns though:
>
> 1. The glCopyPixels fallback works and is reliable. I would like to keep it
> unless there is a good reason not to. That way we don't raise the minimum
> system requirements unnecessarily.

It is completely broken on NVIDIA (for reasons unknown, and for reasons that I don't think are worth looking into).

The only hardware that was going to be using this fallback path these days was hardware using the binary nvidia driver older than about 2003 (< GF FX 5xxx) and radeon hardware using the binary fglrx driver < Radeon 9xxx (2003 also). Every other driver supports supports either GLX_EXT_framebuffer_object or GL_MESA_copy_sub_buffer. The binary fglrx and nvidia drivers which support that hardware are, to the best of my knowledge, not even supported on modern distributions at all these days.

>
> 2. We need a configuration option for people to be able to choose the old
> rendering method.

I would prefer not to do this.

Back in the Beryl days we had countless configuration options for controlling how rendering worked internally and we had countless bug reports because people used the wrong configuration for their particular hardware. Configuration should only be exposed for user-facing functionality and not for core functionality. I think we can all agree that the existing configuration options which change how rendering work, like providing an option to sync to vblank is already totally insane. Adding an option to say "do you want to handle vsync in software or hardware" also makes no sense to the user.

Assuming that we might be running in the case of glCopyPixels OR glXSwapBuffers presents a significant divergence in codepath internally. It means we need to keep around and maintain a bunch of old vsync code which we know is flaky at best because its not handled internally in the driver. I would prefer to drop support for GL_MESA_copy_sub_buffer, but I at least decided that for now we should support hardware on the open drivers that is more than ten years old.

We will also be heavily depending on shaders and vertex buffer objects in the future, and these codepaths have higher requirements than GLX_EXT_framebuffer_object anyways.

Its time to move on from the past.

> There is a significant performance hit in (unthrottled)
> benchmark results with this change. So people need to have the option of going
> back to regional updates.

I'd like some information on this.

If it is that the performance hit comes from the fact that we're effectively always being vsync'd because we're using glXSwapBuffers / eglSwapBuffers, then I think the best course of action is to leave VSync on by default and use triple buffering so that you don't get the performance hit when you're really under load.

We're a compositor, not a benchmark. I don't think there's a good reason to maintain the old codepaths.