After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 684917 - winsys-glx: Don'T attempt to query the vblank counter when using indirect rendering
winsys-glx: Don'T attempt to query the vblank counter when using indirect ren...
Status: RESOLVED FIXED
Product: cogl
Classification: Platform
Component: GLX
unspecified
Other All
: Normal normal
: ---
Assigned To: Cogl maintainer(s)
Cogl maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-09-26 22:24 UTC by drago01
Modified: 2012-09-28 16:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
winsys-glx: Don'T attempt to query the vblank counter when using indirect rendering (1.40 KB, patch)
2012-09-26 22:24 UTC, drago01
none Details | Review
winsys-glx: Remove the vblank counter feature when indirect rendering (1.33 KB, patch)
2012-09-27 13:17 UTC, Neil Roberts
accepted-commit_now Details | Review

Description drago01 2012-09-26 22:24:17 UTC
See patch, without it cogl segfaults:

(gdb) bt
    at ./winsys/cogl-winsys-glx.c:1132
	#2  0x00007ffff7645d17 in _cogl_winsys_onscreen_swap_buffers (
	    onscreen=0x70ce20) at ./winsys/cogl-winsys-glx.c:1328
		#3  0x00007ffff763d027 in cogl_onscreen_swap_buffers (onscreen=0x70ce20)
		    at ./cogl-onscreen.c:140
			#4  0x00007ffff7ce34a2 in clutter_stage_cogl_redraw (
			    stage_window=<optimized out>) at ./cogl/clutter-stage-cogl.c:498
				#5  0x00007ffff7d54ace in clutter_stage_do_redraw (stage=0x715740)
				    at ./clutter-stage.c:1170
					#6  _clutter_stage_do_update (stage=0x715740) at ./clutter-stage.c:1228
					#7  0x00007ffff7d3881d in master_clock_update_stages (stages=0x773800, 
					    master_clock=0x7170f0) at ./clutter-master-clock.c:386
						#8  clutter_clock_dispatch (source=source@entry=0x676b20, 
						    callback=<optimized out>, user_data=<optimized out>)
							    at ./clutter-master-clock.c:520
								#9  0x00007ffff5bb55b5 in g_main_dispatch (context=0x673cd0) at gmain.c:2715
								#10 g_main_context_dispatch (context=context@entry=0x673cd0) at gmain.c:3219
								#11 0x00007ffff5bb58e8 in g_main_context_iterate (context=0x673cd0, 
								    block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
									    at gmain.c:3290
										#12 0x00007ffff5bb5ce2 in g_main_loop_run (loop=0x70f050) at gmain.c:3484
										---Type <return> to continue, or q <return> to quit---
										#13 0x00007ffff7d3543d in clutter_main () at ./clutter-main.c:944
										#14 0x0000000000414dbb in test_fbo_main (argc=1, argv=0x633950)
										    at test-fbo.c:96
											#15 0x000000000040f192 in main (argc=1, argv=0x7fffffffdd38) at test-main.c:206
Comment 1 drago01 2012-09-26 22:24:20 UTC
Created attachment 225236 [details] [review]
winsys-glx: Don'T attempt to query the vblank counter when using indirect rendering

We set the function pointers to NULL but still attempt to use it causing a
segfault.
Comment 2 Neil Roberts 2012-09-27 13:16:39 UTC
Thanks for the patch. However, I wonder if it might be better to just clear the feature flag for the counter rather than explicitly checking for indirect rendering again wherever it is used. I'm attaching a patch that does that.
Comment 3 Neil Roberts 2012-09-27 13:17:13 UTC
Created attachment 225257 [details] [review]
winsys-glx: Remove the vblank counter feature when indirect rendering

Previously when Cogl detects that the GLX context is indirect it
resets the function pointers for the VBLANK_COUNTER feature to NULL.
However it wasn't removing the VBLANK_COUNTER feature flag. Some other
parts of the winsys check for that feature flag rather than checking
whether the pointer is NULL so it would end up calling an invalid
function pointer and crashing. This just fixes it to also clear the
feature flag.
Comment 4 drago01 2012-09-27 13:23:39 UTC
Review of attachment 225257 [details] [review]:

Yeah makes sense and is cleaner that way.
Comment 5 Robert Bragg 2012-09-28 16:12:57 UTC
Review of attachment 225257 [details] [review]:

This looks good to land to me too
Comment 6 Neil Roberts 2012-09-28 16:20:07 UTC
Ok, thanks, I've pushed it to all three branches.