[Mesa-dev] [PATCH] st/mesa: fix glReadBuffer() assertion failure

[Brian Paul]

On 04/07/2016 06:17 PM, Roland Scheidegger wrote:
> Am 08.04.2016 um 01:45 schrieb Brian Paul:
>> If the first call in a GL app is glReadPixels(GL_FRONT) we'd fail the
>> assert(st->ctx->FragmentProgram._Current) at st_atom_shader.c:114 in
>> update_fp().
>>
>> This is because we were calling st_validate_state() without first
>> updating Mesa state with _mesa_update_state().
>>
>> The regression came from commit 83b589301f4a150f4 "st/mesa: fix
>> frontbuffer glReadPixels regressions".
>>
>> The new piglit gl-1.0-simple-readbuffer test exercises this.
>>
>> Cc: "11.1 11.2" <mesa-stable at lists.freedesktop.org>
>> ---
>>   src/mesa/state_tracker/st_cb_fbo.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
>> index ff570e0..456ad83 100644
>> --- a/src/mesa/state_tracker/st_cb_fbo.c
>> +++ b/src/mesa/state_tracker/st_cb_fbo.c
>> @@ -40,6 +40,7 @@
>>   #include "main/glformats.h"
>>   #include "main/macros.h"
>>   #include "main/renderbuffer.h"
>> +#include "main/state.h"
>>
>>   #include "pipe/p_context.h"
>>   #include "pipe/p_defines.h"
>> @@ -729,6 +730,7 @@ st_ReadBuffer(struct gl_context *ctx, GLenum buffer)
>>          fb->Attachment[fb->_ColorReadBufferIndex].Type == GL_NONE) {
>>         /* add the buffer */
>>         st_manager_add_color_renderbuffer(st, fb, fb->_ColorReadBufferIndex);
>> +      _mesa_update_state(ctx);
>>         st_validate_state(st, ST_PIPELINE_RENDER);
>>      }
>>   }
>>
>
> Looks ok to me, though it looks bugs come and go in that area. But not
> sure if the mechanism could be made more robust...

Generally, we call _mesa_update() state as needed in the main Mesa code 
so the Mesa state is validated before st_validate_state() is called. 
This was an exception to that.

Thanks for the reviews.

-Brian