[Mesa-dev] [PATCH 1/4] glx: Call __glXInitVertexArrayState() with a usable gc.

[Ian Romanick]

On 07/07/2016 04:49 PM, Matt Turner wrote:
> On Thu, Jul 7, 2016 at 2:34 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 07/07/2016 09:44 AM, Matt Turner wrote:
>>> On Wed, Jun 29, 2016 at 2:16 PM, Ian Romanick <idr at freedesktop.org> 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!

"Alex, I'll take yak shaving for 400."

"You get a daily double!"

"What is #fml?"

I think all of these problems are fixable.  At some point it's not worth
all the effort when a fix for the original problem exists.  I redact my
initial NAK.

> 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.
>