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

[Chia-I Wu]

On Fri, Aug 12, 2011 at 3:30 AM, Brian Paul <brianp at vmware.com> wrote:
> 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.
I did a little experiments today.  It seems NVIDIA does not have the
concept of single-buffered or double-buffered contexts, but
single-buffered or double-buffered drawables.  In GLX spec,
GLX_DOUBLEBUFFER seems to only apply for drawables too.  This is what
I did

  1. create a single-buffered drawable
  2. create a context with a GLX_DOUBLEBUFFER visual
  3. make the context and drawable current
  4. query GL_DRAW_BUFFER returns GL_FRONT

This could conflict with section 4.2.1 of GL spec

  Monoscopic contexts include only left buffers, while stereoscopic
  contexts include both left and right buffers. Likewise, single
  buffered contexts include only front buffers, while double buffered
  contexts include both front and back buffers.  The type of context is
  selected at GL initialization.

But I am not sure how to interpret it.  If whether a context is
monoscopic or stereoscopic can be queried by GL_STEREO; and whether a
context is single-buffered or double-buffered can be queried by
GL_DOUBLEBUFFER, then those states are framebuffer dependent and can
change when a different drawable is bound.

> -Brian
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 
olv at LunarG.com