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

[Benjamin Franzke]

2011/8/4 Brian Paul <brianp at vmware.com>:
> 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?

That looks like the better solution.

I now think its not really the drivers task, to update the contexts
Draw/ReadBuffer,
so I'd put it directly into _mesa_make_current. New patch attached.

Of course this implies that the framebuffer's
ColorDrawBuffer/ColorReadBuffer is setup
properly by all drivers.
So do you think its safe to always copy those over in _mesa_make_current?

Thanks so far.

> BTW, we'd probably want to copy/update all elements of the ColorDrawBuffer[]
> array plus the ColorReadBuffer state.
>
> -Brian
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mesa-Apply-drawables-draw-readbuffers-to-context-in-.patch
Type: application/octet-stream
Size: 1815 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110804/f1e6e79c/attachment.obj>