Re: [Mesa-dev] [RFC] st/mesa: Add both front and back buffers for double-buffer visuals

2017-01-19 Thread Boyan Ding
2017-01-18 18:23 GMT+08:00 Józef Kucia :
> On Wed, Jan 18, 2017 at 2:25 AM, Boyan Ding  wrote:
>> I don't think I find that. The only place in state tracker where
>> _mesa_add_renderbuffer is called is st_framebuffer_create and only one
>> of the front and back buffers is allocated based on the default
>> behavior. When update_framebuffer_state() in state tracker is called
>> after draw buffer changes to front, state tracker will find it refers
>> to a null renderbuffer and remove it afterwards.
>
> Buffers are allocated on demand in st_DrawBuffers() and
> st_ReadBuffer(). The issue is though a bit different. A single GL
> context is used with two windows (drawables).
> glDrawBuffer/glDrawBuffers() is called when the first window is
> current, then after glXMakeCurrent() with the second window a new
> st_framebuffer is created but GL_FRONT renderbuffer is never allocated
> for the newly created st_framebuffer.

Yeah, you're right. I overlooked the on demand allocation, which turns
out to be the key to the problem. After calling _mesa_drawbuffers in
main, renderbuffers will be allocated on demand in state tracker with
DrawBuffers driver hook. But there is one path in main where
DrawBuffers hook isn't invoked after calling _mesa_drawbuffers --
update_framebuffer in main/framebuffer.c. Adding it fixes the problem.

Before sending out the patch, I'm just wonder if putting
_mesa_drawbuffers and DrawBuffers hook invocation into a function will
be a better style. Because there is a function
_mesa_update_draw_buffers, which is _mesa_drawbuffers without
DrawBuffers hook used when switching contexts. _mesa_drawbuffers and
DrawBuffers hook invocation can also fit into a function used to
reconfigure draw buffers for window system framebuffers.

Regards,
Boyan Ding
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC] st/mesa: Add both front and back buffers for double-buffer visuals

2017-01-18 Thread Józef Kucia
On Wed, Jan 18, 2017 at 2:25 AM, Boyan Ding  wrote:
> I don't think I find that. The only place in state tracker where
> _mesa_add_renderbuffer is called is st_framebuffer_create and only one
> of the front and back buffers is allocated based on the default
> behavior. When update_framebuffer_state() in state tracker is called
> after draw buffer changes to front, state tracker will find it refers
> to a null renderbuffer and remove it afterwards.

Buffers are allocated on demand in st_DrawBuffers() and
st_ReadBuffer(). The issue is though a bit different. A single GL
context is used with two windows (drawables).
glDrawBuffer/glDrawBuffers() is called when the first window is
current, then after glXMakeCurrent() with the second window a new
st_framebuffer is created but GL_FRONT renderbuffer is never allocated
for the newly created st_framebuffer.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC] st/mesa: Add both front and back buffers for double-buffer visuals

2017-01-17 Thread Boyan Ding
2017-01-18 1:31 GMT+08:00 Brian Paul :
> On 01/17/2017 06:44 AM, Boyan Ding wrote:
>>
>> st will only add back buffer attachment to window framebuffer when
>> visual is in double-buffer mode. However, some applications may
>> render to front buffer even if they have chosen a double-buffer visual.
>> In this case, no color buffer will be attached when rendering. i965
>> handles this case correctly, in which it adds both front and back buffer
>> attachments. Do the same thing in st.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99116
>> Signed-off-by: Boyan Ding 
>> ---
>>   src/mesa/state_tracker/st_manager.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_manager.c
>> b/src/mesa/state_tracker/st_manager.c
>> index c3d8286b5a..aa53edfecb 100644
>> --- a/src/mesa/state_tracker/st_manager.c
>> +++ b/src/mesa/state_tracker/st_manager.c
>> @@ -417,7 +417,6 @@ st_framebuffer_create(struct st_context *st,
>>   {
>>  struct st_framebuffer *stfb;
>>  struct gl_config mode;
>> -   gl_buffer_index idx;
>>
>>  if (!stfbi)
>> return NULL;
>> @@ -466,11 +465,12 @@ st_framebuffer_create(struct st_context *st,
>>  stfb->iface_stamp = p_atomic_read(>stamp) - 1;
>>
>>  /* add the color buffer */
>> -   idx = stfb->Base._ColorDrawBufferIndexes[0];
>> -   if (!st_framebuffer_add_renderbuffer(stfb, idx)) {
>> +   if (!st_framebuffer_add_renderbuffer(stfb, BUFFER_FRONT_LEFT)) {
>> free(stfb);
>> return NULL;
>>  }
>> +   if (mode.doubleBufferMode)
>> +  st_framebuffer_add_renderbuffer(stfb, BUFFER_BACK_LEFT);
>>
>>  st_framebuffer_add_renderbuffer(stfb, BUFFER_DEPTH);
>>  st_framebuffer_add_renderbuffer(stfb, BUFFER_ACCUM);
>>
>
>
> I believe we have code in the state tracker to allocate the front buffer on
> demand when the app actually tries to draw to it.  Can you look into that?
>
> -Brian
>

Hi,

I don't think I find that. The only place in state tracker where
_mesa_add_renderbuffer is called is st_framebuffer_create and only one
of the front and back buffers is allocated based on the default
behavior. When update_framebuffer_state() in state tracker is called
after draw buffer changes to front, state tracker will find it refers
to a null renderbuffer and remove it afterwards.

So the reasonable way is to allocate the front buffer in
update_framebuffer_state when front buffer is the draw buffer but not
allocated?

Regards,
Boyan Ding
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC] st/mesa: Add both front and back buffers for double-buffer visuals

2017-01-17 Thread Brian Paul

On 01/17/2017 06:44 AM, Boyan Ding wrote:

st will only add back buffer attachment to window framebuffer when
visual is in double-buffer mode. However, some applications may
render to front buffer even if they have chosen a double-buffer visual.
In this case, no color buffer will be attached when rendering. i965
handles this case correctly, in which it adds both front and back buffer
attachments. Do the same thing in st.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99116
Signed-off-by: Boyan Ding 
---
  src/mesa/state_tracker/st_manager.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/state_tracker/st_manager.c 
b/src/mesa/state_tracker/st_manager.c
index c3d8286b5a..aa53edfecb 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -417,7 +417,6 @@ st_framebuffer_create(struct st_context *st,
  {
 struct st_framebuffer *stfb;
 struct gl_config mode;
-   gl_buffer_index idx;

 if (!stfbi)
return NULL;
@@ -466,11 +465,12 @@ st_framebuffer_create(struct st_context *st,
 stfb->iface_stamp = p_atomic_read(>stamp) - 1;

 /* add the color buffer */
-   idx = stfb->Base._ColorDrawBufferIndexes[0];
-   if (!st_framebuffer_add_renderbuffer(stfb, idx)) {
+   if (!st_framebuffer_add_renderbuffer(stfb, BUFFER_FRONT_LEFT)) {
free(stfb);
return NULL;
 }
+   if (mode.doubleBufferMode)
+  st_framebuffer_add_renderbuffer(stfb, BUFFER_BACK_LEFT);

 st_framebuffer_add_renderbuffer(stfb, BUFFER_DEPTH);
 st_framebuffer_add_renderbuffer(stfb, BUFFER_ACCUM);




I believe we have code in the state tracker to allocate the front buffer 
on demand when the app actually tries to draw to it.  Can you look into 
that?


-Brian

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev