[Mesa-dev] [PATCH] st/mesa: fix frontbuffer glReadPixels regressions

[Roland Scheidegger]

Am 24.02.2016 um 00:40 schrieb Brian Paul:
> The change "mesa/readpix: Don't clip in _mesa_readpixels()" caused a
> few piglit regressions.  The failing tests use glReadPixels to read
> from the front color buffer.  The problem is we were trying to read
> from a non-existant front color buffer.  The front color buffer is
> created on demand in st/mesa.  Since the missing buffer bounds were
> effectively 0 x 0 the glReadPixels was totally clipped and returned
> early.
> 
> The fix involves creating the real front color buffer when we're about
> to try reading from it.
> 
> Tested with llvmpipe and VMware driver on Linux, Windows.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94253
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94254
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94257
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/state_tracker/st_cb_fbo.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
> index 2a2eb09..e17ef66 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -44,6 +44,7 @@
>  #include "pipe/p_context.h"
>  #include "pipe/p_defines.h"
>  #include "pipe/p_screen.h"
> +#include "st_atom.h"
>  #include "st_context.h"
>  #include "st_cb_fbo.h"
>  #include "st_cb_flush.h"
> @@ -711,9 +712,17 @@ st_ReadBuffer(struct gl_context *ctx, GLenum buffer)
>  
>     (void) buffer;
>  
> -   /* add the renderbuffer on demand */
> -   if (fb->_ColorReadBufferIndex >= 0)
> +   /* Check if we need to allocate a front color buffer.
> +    * Front buffers are often allocated on demand (other color buffers are
> +    * always allocated in advance).
> +    */
> +   if ((fb->_ColorReadBufferIndex == BUFFER_FRONT_LEFT ||
> +        fb->_ColorReadBufferIndex == BUFFER_FRONT_RIGHT) &&
> +       fb->Attachment[fb->_ColorReadBufferIndex].Type == GL_NONE) {
> +      /* add the buffer */
>        st_manager_add_color_renderbuffer(st, fb, fb->_ColorReadBufferIndex);
> +      st_validate_state( st, ST_PIPELINE_RENDER );
extra whitespace?


> +   }
>  }
>  
>  
> 

For the series:
Reviewed-by: Roland Scheidegger <sroland at vmware.com>