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

[Brian Paul]

On 08/04/2011 07:39 AM, Benjamin Franzke wrote:
> 2011/8/4 Brian Paul<brianp at vmware.com>:
>> 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.
>
> Yea, I saw that, but the problem is it sets GL_BACK/GL_FRONT depending
> on the contexts config,
> which may be different from the config used for the drawable (as
> described in the commit message).
>
> So mixing configs could be defined as invalid, but its also allowed in
> src/mesa/main/context.c:1324
>
> Furthermore st/mesa also modifes the state in make_current, see
> st_manager.c:781.
>
> Any ideas where to put it instead?

OK, I see.  How about doing something like this instead:

ctx->Color.DrawBuffer[0] = ctx->DrawBuffer->ColorDrawBuffer[0];

The gl_framebuffer also carries the current drawbuffer state (since 
GL_EXT_framebuffer_object came along).  The framebuffer's state gets 
set upon creation and when glDrawBuffer() is called.

I think this is the behaviour that I'd expect if I were alternately 
rendering to single and double-buffered windows with one context.

What do you think?

BTW, we'd probably want to copy/update all elements of the 
ColorDrawBuffer[] array plus the ColorReadBuffer state.

-Brian