Re: [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-sta...@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 

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

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

[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-sta...@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 );
+   }
 }
 
 
-- 
1.9.1

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