[Mesa-dev] [PATCH] intel: Set ctx's drawbuffer according to drawables visual

[Brian Paul]

On 08/11/2011 11:09 AM, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 08/04/2011 06:29 AM, Brian Paul wrote:
>> On 08/04/2011 06:31 AM, Benjamin Franzke wrote:
>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=39588
>>>
>>> egl_dri2 creates contexts with a doubleBufferConfig when PIXMAP and
>>> WINDOW bit is request, so _mesa_init_color sets DrawBuffer[0] to
>>> GL_BACK.
>>> If a pixmap surface is created egl_dri2 will use a single buffer config,
>>> so MakeCurrent has to adjust DrawBuffer[0] to the current drawable.
>>> ---
>>>    src/mesa/drivers/dri/intel/intel_context.c |    6 ++++++
>>>    1 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/intel/intel_context.c
>>> b/src/mesa/drivers/dri/intel/intel_context.c
>>> index fe8be08..0eeffc0 100644
>>> --- a/src/mesa/drivers/dri/intel/intel_context.c
>>> +++ b/src/mesa/drivers/dri/intel/intel_context.c
>>> @@ -970,6 +970,12 @@ intelMakeCurrent(__DRIcontext * driContextPriv,
>>>         readFb = driReadPriv->driverPrivate;
>>>         driContextPriv->dri2.draw_stamp = driDrawPriv->dri2.stamp - 1;
>>>         driContextPriv->dri2.read_stamp = driReadPriv->dri2.stamp - 1;
>>> +
>>> +         if (fb->Visual.doubleBufferMode) {
>>> +            intel->ctx.Color.DrawBuffer[0] = GL_BACK;
>>> +         } else {
>>> +            intel->ctx.Color.DrawBuffer[0] = GL_FRONT;
>>> +         }
>>>          }
>>>
>>>          intel_prepare_render(intel);
>>
>> This doesn't seem right to me.  We shouldn't be changing context state
>> like that during a make-current() call.
>>
>> During context initialization we call _mesa_init_color() where we set
>> ctx->Color.DrawBuffer[0] to either GL_BACK or GL_FRONT depending on the
>> visual's double-buffer flag.  You might investigate why that's not doing
>> the job.
>
> This entire approach is wrong, and it will break GLX.  Page 28 (page 34
> of the PDF) of the GLX 1.4 spec says (emphasis mine):
>
>      No error will be generated if the value of GL DRAW BUFFER in ctx
>      indicates a color buffer that is not supported by draw. In this
>      case, *all rendering will behave as if GL DRAW BUFFER was set to
>      NONE.* Also, no error will be generated if the value of GL READ
>      BUFFER in ctx does not correspond to a valid color buffer. Instead,
>      when an operation that reads from the color buffer is executed
>      (e.g., glReadPixels or glCopyPixels), the pixel values used will be
>      undefined until GL READ BUFFER is set to a color buffer that is
>      valid in read. Operations that query the value of GL READ BUFFER or
>      GL DRAW BUFFER (i.e., glGet, glPushAttrib) use the value set last
>      in the context, independent of whether it is a valid buffer in read
>      or draw.
>
> Page 217 of the GL 3.3 spec says:
>
>      For the default framebuffer, in the initial state the draw buffer
>      for fragment color zero is BACK if there is a back buffer; FRONT
>      if there is no back buffer; and NONE if no default framebuffer is
>      associated with the context.
>
> This seems a little ambiguous and perhaps in conflict with the GLX text.
>   I think the right answer is that the context draw buffer (and read
> buffer) setting is initialized based on the drawable the first time the
> context is bound.  It seems like just changing _mesa_init_color to use
> the double-buffer setting from the drawable (instead of the context)
> should be sufficient.

Yeah, this is all a bit tricky.

Suppose the context is initially in a double-buffered state and we're 
drawing to the back buffer.  Then we bind the context to a 
single-buffered window.  We need to make sure we don't try to render 
to the non-existant back buffer.  There may be a few places in Mesa 
where we're not prepared for that.

Anyway, from what I've gathered we should _not_ change the value of 
GL_DRAW_BUFFER in this situation either.


> I'm putting together a couple piglit GLX tests to exercise this.  I
> don't plan to send them to the piglit list until I can test them on a
> non-Mesa driver.  Since I'm still at SIGGRAPH, that won't be until next
> week.

I also started on a piglit test for this bug got sidetracked. NVIDIA's 
driver seems to do the following:

1. create double-buffered ctx and window, and make current.
2. query GL_DRAW_BUFFER returns GL_BACK
3. create single-buffered window and bind to the context.
4. query GL_DRAW_BUFFER still returns GL_BACK
5. drawing to the single-buffered window has no effect.

-Brian