On Thu, Jul 7, 2016 at 2:34 PM, Ian Romanick <[email protected]> wrote:
> On 07/07/2016 09:44 AM, Matt Turner wrote:
>> On Wed, Jun 29, 2016 at 2:16 PM, Ian Romanick <[email protected]> wrote:
>>> On 06/29/2016 02:04 AM, Colin McDonald wrote:
>>>> I'm not familiar with the code, other than diving in to fix these
>>>> indirect multi-texture problems, so you will know much more about it
>>>> than me.
>>>>
>>>> But, my understanding is that __glXInitVertexArrayState needs info
>>>> from the server, obtained by calls to _indirect_glGetString &
>>>> __indirect_glGetIntegerv. Those routines need the current context
>>>> from __glXGetCurrentContext, so __glXSetCurrentContext(gc) must have
>>>> been called first.
>>>>
>>>> I see your point about a "layering violation". I think that to avoid
>>>> that would require a more substantial restructuring, so that the
>>>> indirect layer can run some initialisation code (ie
>>>> __glXInitVertexArrayState or similar) separate from the bind
>>>> callback, once a usable context has been setup.
>>>
>>> Maybe... *If* __glXGetCurrentContext is the only problem, then I think
>>> a small refactor of __indirect_glGetString could also solve the problem.
>>> Just make a new function
>>>
>>> const GLubyte *do_GetString(Display *dpy, struct glx_context *gc,
>>> GLenum name);
>>>
>>> that both __indirect_glGetString and indirect_bind_context call. It
>>> might even be worth folding the contents of __glXGetString into the new
>>> function... though that's probably a follow-up patch.
>>
>> I tried that (see attached p.patch)... and I get another segfault.
>
> I think it should be possible to elide the __glXFlushRenderBuffer call.
> Since the context is being made current, its buffer of rendering
> commands must be empty. Does the attached patch help?
Thanks. That gets us past that problem... and on to the next!
Fun, fun. __glXInitVertexArrayState(gc) contains
if (__glExtensionBitIsEnabled(gc, GL_ARB_vertex_program_bit)) {
__indirect_glGetProgramivARB(GL_VERTEX_PROGRAM_ARB,
GL_MAX_PROGRAM_ATTRIBS_ARB,
&vertex_program_attribs);
}
Notice __glExtensionBitIsEnabled takes an explicit gc, passed into
__glXInitVertexArrayState. __indirect_glGetProgramivARB on the other
hand calls __glXGetCurrentContext and gets the dummyContext. I've
plumbed gc into a do_GetProgramivARB, and I see a similar call to
__indirect_glGetIntegerv that has the same problem. That looks like
the only other one at least.
__indirect_glGetProgramivARB is code-gen'd, so I just want to add
'handcode="client"' to GetProgramivARB's XML in gl_API.xml. That in
turn necessitates adding a dpy parameter to
__glXInitVertexArrayState().
Ultimately, we segfault in __glXSetupVendorRequest() because it tries
to get dpy from gc->currentDpy.
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7667eb9 in __glXSetupVendorRequest (gc=0x662080, code=17,
vop=1307, cmdlen=8) at indirect.c:191
191 LockDisplay(dpy);
(gdb) p dpy
$40 = (Display * const) 0x0
(gdb) bt
#0 0x00007ffff7667eb9 in __glXSetupVendorRequest (gc=0x662080,
code=17, vop=1307, cmdlen=8) at indirect.c:191
#1 0x00007ffff769aacb in do_GetProgramivARB (dpy=0x64fc60,
gc=0x662080, target=34336, pname=34989, params=0x7fffffffdab4) at
../../../mesa/src/glx/single2.c:537
#2 0x00007ffff7691944 in __glXInitVertexArrayState (dpy=0x64fc60,
gc=0x662080) at ../../../mesa/src/glx/indirect_vertex_array.c:198
#3 0x00007ffff7688c97 in indirect_bind_context (gc=0x662080,
old=0x7ffff78cf6c0 <dummyContext>, draw=27262978, read=27262978) at
../../../mesa/src/glx/indirect_glx.c:160
#4 0x00007ffff766278a in MakeContextCurrent (dpy=0x64fc60,
draw=27262978, read=27262978, gc_user=0x662080) at
../../../mesa/src/glx/glxcurrent.c:228
#5 0x00007ffff70c1af0 in fgPlatformOpenWindow () from /usr/lib64/libglut.so.3
#6 0x00007ffff70bbb06 in fgOpenWindow () from /usr/lib64/libglut.so.3
#7 0x00007ffff70ba42b in fgCreateWindow () from /usr/lib64/libglut.so.3
#8 0x00007ffff70bbc00 in glutCreateWindow () from /usr/lib64/libglut.so.3
#9 0x00000000004022c8 in main (argc=1, argv=0x7fffffffe0f8) at arbvparray.c:294
I'm not sure this is tractable.
diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
index ed57a29..224537a 100644
--- a/src/glx/glxclient.h
+++ b/src/glx/glxclient.h
@@ -733,7 +733,7 @@ extern void __glEmptyImage(struct glx_context *, GLint, GLint, GLint, GLint, GLe
/*
** Allocate and Initialize Vertex Array client state, and free.
*/
-extern void __glXInitVertexArrayState(struct glx_context *);
+extern void __glXInitVertexArrayState(Display *dpy, struct glx_context *);
extern void __glXFreeVertexArrayState(struct glx_context *);
/*
diff --git a/src/glx/glxext.c b/src/glx/glxext.c
index dc87fb9..462b89a 100644
--- a/src/glx/glxext.c
+++ b/src/glx/glxext.c
@@ -984,10 +984,11 @@ _X_HIDDEN GLubyte *
__glXFlushRenderBuffer(struct glx_context * ctx, GLubyte * pc)
{
Display *const dpy = ctx->currentDpy;
- xcb_connection_t *c = XGetXCBConnection(dpy);
const GLint size = pc - ctx->buf;
if ((dpy != NULL) && (size > 0)) {
+ xcb_connection_t *c = XGetXCBConnection(dpy);
+
xcb_glx_render(c, ctx->currentContextTag, size,
(const uint8_t *) ctx->buf);
}
diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
index 743ed97..5ab8cf5 100644
--- a/src/glx/glxextensions.h
+++ b/src/glx/glxextensions.h
@@ -272,6 +272,19 @@ extern GLboolean __glExtensionBitIsEnabled(struct glx_context *gc,
extern void
__glXEnableDirectExtension(struct glx_screen *psc, const char *name);
+
+
+extern const GLubyte *
+do_GetString(Display *dpy, struct glx_context *gc, GLenum name);
+
+extern void
+do_GetIntegerv(Display *dpy, struct glx_context *gc, GLenum val, GLint *i);
+
+extern void
+do_GetProgramivARB(Display *dpy, struct glx_context *gc, GLenum target,
+ GLenum pname, GLint *params);
+
+
/* Source-level backwards compatibility with old drivers. They won't
* find the respective functions, though.
*/
diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
index bb121f8..808e727 100644
--- a/src/glx/indirect_glx.c
+++ b/src/glx/indirect_glx.c
@@ -126,6 +126,9 @@ SendMakeCurrentRequest(Display * dpy, CARD8 opcode,
return ret;
}
+const GLubyte *
+do_GetString(Display *dpy, struct glx_context *gc, GLenum name);
+
static int
indirect_bind_context(struct glx_context *gc, struct glx_context *old,
GLXDrawable draw, GLXDrawable read)
@@ -152,9 +155,9 @@ indirect_bind_context(struct glx_context *gc, struct glx_context *old,
state = gc->client_state_private;
if (state->array_state == NULL) {
- glGetString(GL_EXTENSIONS);
- glGetString(GL_VERSION);
- __glXInitVertexArrayState(gc);
+ do_GetString(dpy, gc, GL_EXTENSIONS);
+ do_GetString(dpy, gc, GL_VERSION);
+ __glXInitVertexArrayState(dpy, gc);
}
return !sent;
diff --git a/src/glx/indirect_vertex_array.c b/src/glx/indirect_vertex_array.c
index a707343..12ae01e 100644
--- a/src/glx/indirect_vertex_array.c
+++ b/src/glx/indirect_vertex_array.c
@@ -137,7 +137,7 @@ __glXFreeVertexArrayState(struct glx_context * gc)
* supported.
*/
void
-__glXInitVertexArrayState(struct glx_context * gc)
+__glXInitVertexArrayState(Display *dpy, struct glx_context * gc)
{
__GLXattribute *state = (__GLXattribute *) (gc->client_state_private);
struct array_state_vector *arrays;
@@ -191,13 +191,14 @@ __glXInitVertexArrayState(struct glx_context * gc)
if (__glExtensionBitIsEnabled(gc, GL_ARB_multitexture_bit)
|| (gc->server_major > 1) || (gc->server_minor >= 3)) {
- __indirect_glGetIntegerv(GL_MAX_TEXTURE_UNITS, &texture_units);
+ do_GetIntegerv(dpy, gc, GL_MAX_TEXTURE_UNITS, &texture_units);
}
if (__glExtensionBitIsEnabled(gc, GL_ARB_vertex_program_bit)) {
- __indirect_glGetProgramivARB(GL_VERTEX_PROGRAM_ARB,
- GL_MAX_PROGRAM_ATTRIBS_ARB,
- &vertex_program_attribs);
+ do_GetProgramivARB(dpy, gc,
+ GL_VERTEX_PROGRAM_ARB,
+ GL_MAX_PROGRAM_ATTRIBS_ARB,
+ &vertex_program_attribs);
}
arrays->num_texture_units = texture_units;
diff --git a/src/glx/single2.c b/src/glx/single2.c
index 2a1bf06..b9d63dd 100644
--- a/src/glx/single2.c
+++ b/src/glx/single2.c
@@ -467,10 +467,12 @@ __indirect_glGetFloatv(GLenum val, GLfloat * f)
}
void
-__indirect_glGetIntegerv(GLenum val, GLint * i)
+do_GetIntegerv(Display *dpy, struct glx_context *gc, GLenum val, GLint * i)
{
const GLenum origVal = val;
- __GLX_SINGLE_DECLARE_VARIABLES();
+ GLubyte *pc, *pixelHeaderPC;
+ GLuint compsize, cmdlen;
+ xGLXSingleReq *req;
xGLXSingleReply reply;
val = RemapTransposeEnum(val);
@@ -517,6 +519,42 @@ __indirect_glGetIntegerv(GLenum val, GLint * i)
__GLX_SINGLE_END();
}
+void
+__indirect_glGetIntegerv(GLenum val, GLint * i)
+{
+ struct glx_context *gc = __glXGetCurrentContext();
+ Display *dpy = gc->currentDpy;
+
+ do_GetIntegerv(dpy, gc, val, i);
+}
+
+void
+do_GetProgramivARB(Display *dpy, struct glx_context *gc, GLenum target,
+ GLenum pname, GLint *params)
+{
+ const GLuint cmdlen = 8;
+ if (__builtin_expect(dpy != NULL, 1)) {
+ GLubyte const *pc =
+ __glXSetupVendorRequest(gc, X_GLXVendorPrivateWithReply,
+ X_GLvop_GetProgramivARB, cmdlen);
+ (void) memcpy((void *) (pc + 0), (void *) (&target), 4);
+ (void) memcpy((void *) (pc + 4), (void *) (&pname), 4);
+ (void) __glXReadReply(dpy, 4, params, GL_FALSE);
+ UnlockDisplay(dpy);
+ SyncHandle();
+ }
+}
+
+void
+__indirect_glGetProgramivARB(GLenum target, GLenum pname, GLint *params)
+{
+ struct glx_context *gc = __glXGetCurrentContext();
+ Display *dpy = gc->currentDpy;
+
+ do_GetProgramivARB(dpy, gc, target, pname, params);
+}
+
+
/*
** Send all pending commands to server.
*/
@@ -638,17 +676,11 @@ version_from_string(const char *ver, int *major_version, int *minor_version)
*minor_version = minor;
}
-
const GLubyte *
-__indirect_glGetString(GLenum name)
+do_GetString(Display *dpy, struct glx_context *gc, GLenum name)
{
- struct glx_context *gc = __glXGetCurrentContext();
- Display *dpy = gc->currentDpy;
GLubyte *s = NULL;
- if (!dpy)
- return 0;
-
/*
** Return the cached copy if the string has already been fetched
*/
@@ -789,6 +821,19 @@ __indirect_glGetString(GLenum name)
return s;
}
+
+const GLubyte *
+__indirect_glGetString(GLenum name)
+{
+ struct glx_context *gc = __glXGetCurrentContext();
+ Display *dpy = gc->currentDpy;
+
+ if (!dpy)
+ return 0;
+
+ return do_GetString(dpy, gc, name);
+}
+
GLboolean
__indirect_glIsEnabled(GLenum cap)
{
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index 79e1ba1..1c4c4d3 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -7349,7 +7349,7 @@
<param name="target" type="GLenum"/>
<param name="pname" type="GLenum"/>
<param name="params" type="GLint *" output="true" variable_param="pname"/>
- <glx vendorpriv="1307"/>
+ <glx vendorpriv="1307" handcode="client"/>
</function>
<function name="GetProgramStringARB" deprecated="3.1">
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
