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

[Ian Romanick]

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

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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk5EDMkACgkQX1gOwKyEAw8vVACeMghE21gtQMYJgAkWXBDmz5zB
qgkAnRLyvNKzJwJuQTx1Yk4v7hLeohfz
=XZUQ
-----END PGP SIGNATURE-----