Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opengl improvements: double buffering, limited repaints, paint state #679

Closed
totaam opened this issue Sep 14, 2014 · 19 comments
Closed

opengl improvements: double buffering, limited repaints, paint state #679

totaam opened this issue Sep 14, 2014 · 19 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Sep 14, 2014

Issue migrated from trac ticket # 679

component: client | priority: major | resolution: fixed

2014-09-14 10:43:47: totaam created the issue


See also #640#comment:3.

At the moment, only win32 uses double-buffering by default.
Is it required? Useful? The PBO is a bit like our own double buffering already.
Does double buffering limit the number of paints we can do per second because of the synchronization that happens when we swap_buffers? (if there is any!)
This can also cause problems if we have many windows: they will all consume synchronization time. Or will they? See Swap buffer synchronization and Using SwapBuffers() with multiple OpenGL canvases and vertical sync?

This needs testing with:

XPRA_OPENGL_DOUBLE_BUFFERED=0

And maybe a dedicated test app to trigger the paint refresh pattern needed for testing. (many windows? fast refresh? hard!)

Same for Linux, do we want to enable double buffering there?
Does it work reliably? (works here)

Also, when not using double buffering, we can optimize repaints to only paste the fbo contents on the area of the window that needs updating instead of the whole window.

As per [http://www.mesa3d.org/brianp/sig97/perfopt.htm]: SwapBuffer calls and graphics pipe blocking: On systems with 3-D graphics hardware the SwapBuffers call is synchronized to the monitor's vertical retrace. Input to the OpenGL command queue may be blocked until the buffer swap has completed. Therefore, don't put more OpenGL calls immediately after SwapBuffers. Instead, put application computation instructions which can overlap with the buffer swap delay.
See also [http://www.opengl.org/wiki/Swap_Interval]
We should try to ensure we don't make any GL calls after swap_buffers.
For example, the whole set_rgb_paint_state / unset_rgb_paint_state logic should be changed to be done on-demand before each paint rather than assuming a specific state is the default. There is also a glClear there, not sure what to do with it: it doesn't really do anything, but I can only find conflicting information on the performance implications.

Also, we want zerocopy upload for video encodings, especially dec_avcodec2, see #717 - currently disabled because avcodec buffers are not thread safe?

We probably should re-probe the max texture size check when the screen is resized?

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 11:25:48: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 11:25:48: totaam changed owner from antoine to totaam

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 11:25:48: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 11:25:48: totaam changed title from opengl improvements: double buffering, limited repaints to opengl improvements: double buffering, limited repaints, paint state

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 12:03:21: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Sep 14, 2014

2014-09-14 12:03:21: totaam commented


Improvements:

  • r7600 + r7608: only repaints what is needed in single-buffer mode
  • r7601: paint debugging via XPRA_OPENGL_PAINT_BOX env var
  • r7602: adds GL_PERSPECTIVE_CORRECTION_HINT hint
  • r7603: removes the glClear after swap_buffers

No matter how many small glxgears window I start, I seem to only get ~60fps, which is my monitor refresh rate... So we are capped.

Whilst we're here, it would be good to deal with #478 too.

@totaam
Copy link
Collaborator Author

totaam commented Sep 25, 2014

2014-09-25 05:21:50: totaam commented


r7600 caused #693: bug with scaling, which is fixed in r7804.

@totaam
Copy link
Collaborator Author

totaam commented Nov 13, 2014

2014-11-13 05:23:21: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Nov 13, 2014

2014-11-13 05:23:21: totaam commented


#717: bug caused by old versions of pyopengl found in Fedora 20..

@totaam
Copy link
Collaborator Author

totaam commented Dec 10, 2014

2014-12-10 00:07:04: antoine commented


See also: #760, #792

@totaam
Copy link
Collaborator Author

totaam commented Apr 14, 2015

2015-04-14 17:14:22: antoine commented


Re-scheduling.

@totaam
Copy link
Collaborator Author

totaam commented May 1, 2015

2015-05-01 07:23:15: antoine edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented May 23, 2015

2015-05-23 11:51:31: antoine commented


Double-buffering seems to work on Linux see #760#comment:22, but on OSX this hides the box paint feature.

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 16:45:47: antoine commented


A lot of work was completed in #792.

Which only leaves the cleanup of the paint state logic.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 11:10:42: antoine commented


I've just hit this error by starting a new command via the control channel very early and / or changing focus as it appeared:

Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/client/gl/gtk2/gl_window_backing.py", line 39, in gl_expose_event
    self.gl_init()
  File "/usr/lib64/python2.7/site-packages/xpra/client/gl/gl_window_backing_base.py", line 396, in gl_init
    glClear(GL_COLOR_BUFFER_BIT)
  File "/usr/lib/python2.7/site-packages/OpenGL/platform/baseplatform.py", line 402, in __call__
    return self( *args, **named )
  File "errorchecker.pyx", line 53, in OpenGL_accelerate.errorchecker._ErrorChecker.glCheckError (src/errorchecker.c:1218)
OpenGL.error.GLError: GLError(
	err = 1286,
	description = 'invalid framebuffer operation',
	baseOperation = glClear,
	cArguments = (GL_COLOR_BUFFER_BIT,)
)
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/client/gl/gtk2/gl_window_backing.py", line 39, in gl_expose_event
    self.gl_init()
  File "/usr/lib64/python2.7/site-packages/xpra/client/gl/gl_window_backing_base.py", line 396, in gl_init
    glClear(GL_COLOR_BUFFER_BIT)
  File "errorchecker.pyx", line 53, in OpenGL_accelerate.errorchecker._ErrorChecker.glCheckError (src/errorchecker.c:1218)
OpenGL.error.GLError: GLError(
	err = 1286,
	description = 'invalid framebuffer operation',
	baseOperation = glClear,
	cArguments = (GL_COLOR_BUFFER_BIT,)
)

Maybe the expose event fired before we have a proper gl context to use for painting? r9871 should fix that.

@totaam
Copy link
Collaborator Author

totaam commented Jul 31, 2015

2015-07-31 07:43:28: antoine commented


Minor enhancement / fix: see r10172.

@totaam
Copy link
Collaborator Author

totaam commented Sep 1, 2015

2015-09-01 09:55:28: antoine changed status from assigned to closed

@totaam
Copy link
Collaborator Author

totaam commented Sep 1, 2015

2015-09-01 09:55:28: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Sep 1, 2015

2015-09-01 09:55:28: antoine commented


Closing:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant