Bug 309647

Summary:	KWin uses incorrect function prototype for glXSwapInterval
Product:	[Plasma] kwin	Reporter:	Ralf Jung <post>
Component:	compositing	Assignee:	KWin default assignee <kwin-bugs-null>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	NOR
Version:	git master
Target Milestone:	---
Platform:	unspecified
OS:	Linux
Latest Commit:	http://commits.kde.org/kde-workspace/a0ee40355011cc73655af0232e35053665a6115f	Version Fixed In:
Sentry Crash Report:

Description Ralf Jung 2012-11-06 12:38:40 UTC

KWin uses the same function type, and the same function pointer, for the various glXSwapInterval* functions provided by different extensions. However, some of these functions have different parameters or return values:
int glXSwapIntervalSGI (int interval)
int glXSwapIntervalEXT (Display *dpy, GLXDrawable drawable, int interval)
int glXSwapIntervalMESA(unsigned int interval)
These are taken from GL/glx.h and GL/glxext.h. Interesting enough, https://www.opengl.org/registry/specs/EXT/swap_control.txt says that glXSwapIntervalEXT has a void return type since 22 Aug 2011 (version 7). https://www.opengl.org/registry/specs/SGI/swap_control.txt says that 0 is not a valid value for glXSwapIntervalSGI, while it is allowed for the MESA function (http://cgit.freedesktop.org/mesa/mesa/tree/docs/MESA_swap_control.spec).
The fourth function, glXSwapIntervalOML, does not seem to exist at all... neither GL/glxext.h nor https://www.opengl.org/registry/specs/OML/glx_sync_control.txt mention it.

I could fix this bug, but I wonder how you'd prefer it to be fixed. IMHO the cleanest solution would be to directly use the types provided by the GL/glxext.h, but maybe you had a reason for specify all those function types yourself. Also, considering the different prototypes and behaviour, it might be more appropriate to store them all in their own function pointer and provide a simple wrapper which chooses the right one.

Reproducible: Always




I am not sure if bugzilla is the appropriate channel for this kind of problem. Hopefully it is ;-)

Comment 1 Martin Flöser 2012-11-06 16:51:16 UTC

Nice catch.

I think the best way is to properly resolve each of the function pointers and then decide when it is actually used which one to call. Probably in the way:
1. Mesa
2. EXT
3. SGI

Comment 2 Ralf Jung 2012-11-06 17:28:38 UTC

That sounds good. The simple wrapper which is called in the end will also need a display and a drawable, but only use it for the EXT version.
Is it okay to use the function prototypes provided by the system headers? If yes, would you accept a patch that also converts (most of) the other functions to use these prototypes (to keep compatibility, something like "typedef PFNGLXSOMEFUNC glXSomeFunc_type"), so that similar accidents are prevented?

Comment 3 Martin Flöser 2012-11-07 16:35:08 UTC

> That sounds good. The simple wrapper which is called in the end will also
> need a display and a drawable, but only use it for the EXT version.
> Is it okay to use the function prototypes provided by the system headers? If
> yes, would you accept a patch that also converts (most of) the other
> functions to use these prototypes (to keep compatibility, something like
> "typedef PFNGLXSOMEFUNC glXSomeFunc_type"), so that similar accidents are
> prevented?
I would just resolve all three function pointers and don't use an abstraction 
at all or just have this abstraction inside the glxbackend. IIRC it's only 
used from one function inside glxbackend, so that belongs there.

The function resolving should follow the common (and correct) pattern as for 
the other resolved functions. Best look at the OpenGL and/or egl function 
resolving, the glx ones were a little bit buggy :-)

Comment 4 Ralf Jung 2012-11-18 13:31:08 UTC

Git commit a0ee40355011cc73655af0232e35053665a6115f by Ralf Jung.
Committed on 13/11/2012 at 22:19.
Pushed by ralfjung into branch 'master'.

Fix buffer swap prototypes
REVIEW: 107302

M  +19   -7    kwin/glxbackend.cpp
M  +2    -0    kwin/glxbackend.h
M  +15   -12   kwin/libkwineffects/kwinglutils_funcs.cpp
M  +6    -2    kwin/libkwineffects/kwinglutils_funcs.h

http://commits.kde.org/kde-workspace/a0ee40355011cc73655af0232e35053665a6115f