GNOME Bugzilla – Bug 659360
Optimization in _cogl_winsys_onscreen_swap_region causes tearing
Last modified: 2011-09-19 16:56:52 UTC
Sub stage redraws are trottled / synchronized in _cogl_winsys_onscreen_swap_region using GLX_SGI_video_sync in order to avoid unnecessary waits it compares the sync counter with the last value and skips the wait in this case. This results into tearing when using the NVIDIA proprietary driver where glBlitFrameBuffer (which is not synchronized) is used to do do the sub buffer copies. Just removing this check seems to fix the tearing so I went glx_onscreen->last_swap_vsync_counter is set and it seems to be done later on in the function. Just rereading the value here seems to fix it (even though that disagrees with the comment above ... which I don't get (because not updating the counter is more likely to hit the check next time _cogl_winsys_onscreen_swap_region is called ...).
Created attachment 196840 [details] [review] Fix?
As this has been reported as a regression from clutter 1.6 I have compared the code to what we where doing in 1.6 and found this: --------- if (!backend_glx->blit_sub_buffer_is_synchronized) { /* XXX: note that glXCopySubBuffer, at least for Intel, is * synchronized with the vblank but glBlitFramebuffer may * not be so we use the same scheme we do when calling * glXSwapBuffers without the swap_control extension and * call glFinish () before waiting for the vblank period. * * See where we call glXSwapBuffers for more details. */ glFinish (); wait_for_vblank (backend_glx); } else if (backend_glx->get_video_sync) { /* If we have the GLX_SGI_video_sync extension then we can * be a bit smarter about how we throttle blits by avoiding * any waits if we can see that the video sync count has * already progressed. */ if (backend_glx->last_video_sync_count == video_sync_count) wait_for_vblank (backend_glx); } else wait_for_vblank (backend_glx); ------- When blit_sub_buffer_is_synchronized is FALSE (which is was the case for NVIDIA) we did a glFinish() followed by an unconditional wait_for_vblank (backend_glx); No we no longer have "blit_sub_buffer_is_synchronized" so we always do the counter comparison which leads to tearing. The current code looks like this: --- context->glFinish (); if (have_counter && can_wait) { end_frame_vsync_counter = _cogl_winsys_get_vsync_counter (); /* If we have the GLX_SGI_video_sync extension then we can * be a bit smarter about how we throttle blits by avoiding * any waits if we can see that the video sync count has * already progressed. */ if (glx_onscreen->last_swap_vsync_counter == end_frame_vsync_counter) _cogl_winsys_wait_for_vblank (); } else if (can_wait) _cogl_winsys_wait_for_vblank (); ---
Comment on attachment 196840 [details] [review] Fix? This is wrong.
Created attachment 196847 [details] [review] winsys-glx: Fix synchronisation behaviour in _cogl_winsys_onscreen_swap_region This patch basically restores the logic from 1.6. There we assumed that glXCopySubBuffer won't tear and thus only needs to be throttled to the framerate, while glBlitFramebuffer needs to always wait to avoid tearing.
Review of attachment 196847 [details] [review]: the patch looks okay to me. ::: cogl/winsys/cogl-winsys-glx.c @@ +1135,3 @@ + * we only need it to throttle redraws. + */ + gboolean blit_sub_buffer_is_synchronized = glx_renderer->pf_glXCopySubBuffer != NULL; we should probably store this boolean flag on the GLX CoglRenderer, and set it when we look for the glXCopySubBuffer function pointer.
Created attachment 196865 [details] [review] winsys-glx: Fix synchronisation behaviour in _cogl_winsys_onscreen_swap_region This patch basically restores the logic from 1.6. There we assumed that glXCopySubBuffer won't tear and thus only needs to be throttled to the framerate, while glBlitFramebuffer needs to always wait to avoid tearing. ---- *) Set the is_synchronized boolean as winsys_feature for glx.
I've landed this patch in master as 671a4dfb3425b, thanks.