[Mesa-dev] [PATCH] mesa/st: Don't modify the context draw/read buffers.

[Chia-I Wu]

On Fri, Dec 9, 2011 at 2:02 PM, Chia-I Wu <olv at lunarg.com> wrote:
> On Thu, Dec 8, 2011 at 10:00 PM,  <jfonseca at vmware.com> wrote:
>> From: José Fonseca <jfonseca at vmware.com>
>>
>> It sets the wrong values (GL_XXX_LEFT instead of GL_XXX), and no other
>> Mesa driver does this, given that Mesa sets the right draw/read buffers
>> provided the Mesa visual has the doublebuffer flag filled correctly
>> which is the case.
> In EGL, when an EGLSurface is created, users can specify whether the
> front or back buffer will be rendered to.  The function is used to
> make a double-buffered context work with an EGLSurface whose front
> buffer is supposed to be rendered to.  But I admit that the function
> is hacky.
and may be wrong for GL.  It is ok for GLES because GLES does not have
GL_DRAW_BUFFER thus the value can be modified.

> Since this is brought up, I did this experiment some time ago:
It was done using GLX and GL.
>  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
>
> Mesa returned GL_BACK and nVidia's proprietary driver returned
> GL_FRONT.  This difference, IMHO, comes from that Mesa uses the visual
> of the context to determine whether the context is double-buffered or
> single-buffered, while nVidia uses the visual of the drawable to make
> the decision (and at the time when the context is first made current).
>
> What I want to argue here is that, maybe there should be no
> single-buffered or double-buffered contexts, but single-buffered or
> double-buffered drawables.  Or more precisely, the type of the context
> should be determined by the type of the current drawable.  I checked
> GLX spec and it seemed that GLX_DOUBLEBUFFER applies for drawables.
> Since GL 3.0, GL_DOUBLEBUFFER is also listed as one of the framebuffer
> dependent values.  That implies the state may change when the current
> drawable changes.  So it is still a correct behavior for the drawable
> to determine the type of the context.
>
> I did not have a chance to look deeper into this due to the lack of
> time.  So I may be terribly wrong here...
>
>
>> ---
>>  src/mesa/state_tracker/st_manager.c |   64 -----------------------------------
>>  1 files changed, 0 insertions(+), 64 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
>> index d5228d3..55699e7 100644
>> --- a/src/mesa/state_tracker/st_manager.c
>> +++ b/src/mesa/state_tracker/st_manager.c
>> @@ -404,54 +404,6 @@ st_visual_to_context_mode(const struct st_visual *visual,
>>  }
>>
>>  /**
>> - * Determine the default draw or read buffer from a visual.
>> - */
>> -static void
>> -st_visual_to_default_buffer(const struct st_visual *visual,
>> -                            GLenum *buffer, GLint *index)
>> -{
>> -   enum st_attachment_type statt;
>> -   GLenum buf;
>> -   gl_buffer_index idx;
>> -
>> -   statt = visual->render_buffer;
>> -   /* do nothing if an invalid render buffer is specified */
>> -   if (statt == ST_ATTACHMENT_INVALID ||
>> -       !st_visual_have_buffers(visual, 1 << statt))
>> -      return;
>> -
>> -   switch (statt) {
>> -   case ST_ATTACHMENT_FRONT_LEFT:
>> -      buf = GL_FRONT_LEFT;
>> -      idx = BUFFER_FRONT_LEFT;
>> -      break;
>> -   case ST_ATTACHMENT_BACK_LEFT:
>> -      buf = GL_BACK_LEFT;
>> -      idx = BUFFER_BACK_LEFT;
>> -      break;
>> -   case ST_ATTACHMENT_FRONT_RIGHT:
>> -      buf = GL_FRONT_RIGHT;
>> -      idx = BUFFER_FRONT_RIGHT;
>> -      break;
>> -   case ST_ATTACHMENT_BACK_RIGHT:
>> -      buf = GL_BACK_RIGHT;
>> -      idx = BUFFER_BACK_RIGHT;
>> -      break;
>> -   default:
>> -      buf = GL_NONE;
>> -      idx = BUFFER_COUNT;
>> -      break;
>> -   }
>> -
>> -   if (buf != GL_NONE) {
>> -      if (buffer)
>> -         *buffer = buf;
>> -      if (index)
>> -         *index = idx;
>> -   }
>> -}
>> -
>> -/**
>>  * Create a framebuffer from a manager interface.
>>  */
>>  static struct st_framebuffer *
>> @@ -471,12 +423,6 @@ st_framebuffer_create(struct st_framebuffer_iface *stfbi)
>>    st_visual_to_context_mode(stfbi->visual, &mode);
>>    _mesa_initialize_window_framebuffer(&stfb->Base, &mode);
>>
>> -   /* modify the draw/read buffers of the fb */
>> -   st_visual_to_default_buffer(stfbi->visual, &stfb->Base.ColorDrawBuffer[0],
>> -         &stfb->Base._ColorDrawBufferIndexes[0]);
>> -   st_visual_to_default_buffer(stfbi->visual, &stfb->Base.ColorReadBuffer,
>> -         &stfb->Base._ColorReadBufferIndex);
>> -
>>    stfb->iface = stfbi;
>>    stfb->iface_stamp = p_atomic_read(&stfbi->stamp) - 1;
>>
>> @@ -776,16 +722,6 @@ st_api_make_current(struct st_api *stapi, struct st_context_iface *stctxi,
>>          if (stread != stdraw)
>>             st_framebuffer_validate(stread, st);
>>
>> -         /* modify the draw/read buffers of the context */
>> -         if (stdraw->iface) {
>> -            st_visual_to_default_buffer(stdraw->iface->visual,
>> -                  &st->ctx->Color.DrawBuffer[0], NULL);
>> -         }
>> -         if (stread->iface) {
>> -            st_visual_to_default_buffer(stread->iface->visual,
>> -                  &st->ctx->Pixel.ReadBuffer, NULL);
>> -         }
>> -
>>          ret = _mesa_make_current(st->ctx, &stdraw->Base, &stread->Base);
>>
>>          st->draw_stamp = stdraw->stamp - 1;
>> --
>> 1.7.7.3
>>
>
>
>
> --
> olv at LunarG.com



-- 
olv at LunarG.com