Re: [Mesa-dev] [PATCH] st/mesa: fix sampler view reference counting bug in glDraw/CopyPixels

2015-02-19 Thread Ilia Mirkin 
On Wed, Feb 18, 2015 at 1:20 PM, Brian Paul bri...@vmware.com wrote:
 Use pipe_sampler_view_reference() instead of ordinary assignment.
 Also add a new sanity check assertion.

 Fixes piglit gl-1.0-drawpixels-color-index test crash.  But note
 that the test still fails.

Fails on nvc0 as well, for the record.


 Cc: 10.4, 10.5 mesa-sta...@lists.freedesktop.org
 ---
  src/mesa/state_tracker/st_cb_drawpixels.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

 diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c 
 b/src/mesa/state_tracker/st_cb_drawpixels.c
 index 939fc20..3d13b5c 100644
 --- a/src/mesa/state_tracker/st_cb_drawpixels.c
 +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
 @@ -1154,8 +1154,10 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,

color = NULL;
if (st-pixel_xfer.pixelmap_enabled) {
 - sv[1] = st-pixel_xfer.pixelmap_sampler_view;
 - num_sampler_view++;
 + sv[1] = NULL;
 + pipe_sampler_view_reference(sv[1],
 + st-pixel_xfer.pixelmap_sampler_view);

I would *much* prefer a = {NULL} when sv is declared instead of
awkwardly before the reference (both here and in st_CopyPixels).

With that changed, Reviewed-by: Ilia Mirkin imir...@alum.mit.edu

 + num_sampler_view++;
}
 }

 @@ -1176,7 +1178,8 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
  if (write_stencil) {
 enum pipe_format stencil_format =
   util_format_stencil_only(pt-format);
 -
 +   /* we should not be doing pixel map/transfer (see above) */
 +   assert(num_sampler_view == 1);
 sv[1] = st_create_texture_sampler_view_format(st-pipe, pt,
   stencil_format);
 num_sampler_view++;
 @@ -1516,7 +1519,9 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint 
 srcy,
driver_vp = make_passthrough_vertex_shader(st, GL_FALSE);

if (st-pixel_xfer.pixelmap_enabled) {
 - sv[1] = st-pixel_xfer.pixelmap_sampler_view;
 + sv[1] = NULL;
 + pipe_sampler_view_reference(sv[1],
 + st-pixel_xfer.pixelmap_sampler_view);
   num_sampler_view++;
}
 }
 --
 1.9.1

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

Re: [Mesa-dev] [PATCH] st/mesa: fix sampler view reference counting bug in glDraw/CopyPixels

2015-02-19 Thread Brian Paul 

Ping.

On 02/18/2015 11:20 AM, Brian Paul wrote:

Use pipe_sampler_view_reference() instead of ordinary assignment.
Also add a new sanity check assertion.

Fixes piglit gl-1.0-drawpixels-color-index test crash.  But note
that the test still fails.

Cc: 10.4, 10.5 mesa-sta...@lists.freedesktop.org
---
  src/mesa/state_tracker/st_cb_drawpixels.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c 
b/src/mesa/state_tracker/st_cb_drawpixels.c
index 939fc20..3d13b5c 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -1154,8 +1154,10 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,

color = NULL;
if (st-pixel_xfer.pixelmap_enabled) {
- sv[1] = st-pixel_xfer.pixelmap_sampler_view;
- num_sampler_view++;
+ sv[1] = NULL;
+ pipe_sampler_view_reference(sv[1],
+ st-pixel_xfer.pixelmap_sampler_view);
+ num_sampler_view++;
}
 }

@@ -1176,7 +1178,8 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
  if (write_stencil) {
 enum pipe_format stencil_format =
   util_format_stencil_only(pt-format);
-
+   /* we should not be doing pixel map/transfer (see above) */
+   assert(num_sampler_view == 1);
 sv[1] = st_create_texture_sampler_view_format(st-pipe, pt,
   stencil_format);
 num_sampler_view++;
@@ -1516,7 +1519,9 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint 
srcy,
driver_vp = make_passthrough_vertex_shader(st, GL_FALSE);

if (st-pixel_xfer.pixelmap_enabled) {
- sv[1] = st-pixel_xfer.pixelmap_sampler_view;
+ sv[1] = NULL;
+ pipe_sampler_view_reference(sv[1],
+ st-pixel_xfer.pixelmap_sampler_view);
   num_sampler_view++;
}
 }



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

Re: [Mesa-dev] [PATCH] st/mesa: fix sampler view reference counting bug in glDraw/CopyPixels

2015-02-19 Thread Brian Paul 

On 02/19/2015 02:55 PM, Ilia Mirkin wrote:

On Wed, Feb 18, 2015 at 1:20 PM, Brian Paul bri...@vmware.com wrote:

Use pipe_sampler_view_reference() instead of ordinary assignment.
Also add a new sanity check assertion.

Fixes piglit gl-1.0-drawpixels-color-index test crash.  But note
that the test still fails.


Fails on nvc0 as well, for the record.


Yeah, it's a state tracker issue.  I started working on a fix but it's 
not high priority ATM.







Cc: 10.4, 10.5 mesa-sta...@lists.freedesktop.org
---
  src/mesa/state_tracker/st_cb_drawpixels.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c 
b/src/mesa/state_tracker/st_cb_drawpixels.c
index 939fc20..3d13b5c 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -1154,8 +1154,10 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,

color = NULL;
if (st-pixel_xfer.pixelmap_enabled) {
- sv[1] = st-pixel_xfer.pixelmap_sampler_view;
- num_sampler_view++;
+ sv[1] = NULL;
+ pipe_sampler_view_reference(sv[1],
+ st-pixel_xfer.pixelmap_sampler_view);


I would *much* prefer a = {NULL} when sv is declared instead of
awkwardly before the reference (both here and in st_CopyPixels).

With that changed, Reviewed-by: Ilia Mirkin imir...@alum.mit.edu


Will do. Thx.

-Brian


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

[Mesa-dev] [PATCH] st/mesa: fix sampler view reference counting bug in glDraw/CopyPixels

2015-02-18 Thread Brian Paul 
Use pipe_sampler_view_reference() instead of ordinary assignment.
Also add a new sanity check assertion.

Fixes piglit gl-1.0-drawpixels-color-index test crash.  But note
that the test still fails.

Cc: 10.4, 10.5 mesa-sta...@lists.freedesktop.org
---
 src/mesa/state_tracker/st_cb_drawpixels.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c 
b/src/mesa/state_tracker/st_cb_drawpixels.c
index 939fc20..3d13b5c 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -1154,8 +1154,10 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
 
   color = NULL;
   if (st-pixel_xfer.pixelmap_enabled) {
- sv[1] = st-pixel_xfer.pixelmap_sampler_view;
- num_sampler_view++;
+ sv[1] = NULL;
+ pipe_sampler_view_reference(sv[1],
+ st-pixel_xfer.pixelmap_sampler_view);
+ num_sampler_view++;
   }
}
 
@@ -1176,7 +1178,8 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
 if (write_stencil) {
enum pipe_format stencil_format =
  util_format_stencil_only(pt-format);
-
+   /* we should not be doing pixel map/transfer (see above) */
+   assert(num_sampler_view == 1);
sv[1] = st_create_texture_sampler_view_format(st-pipe, pt,
  stencil_format);
num_sampler_view++;
@@ -1516,7 +1519,9 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint 
srcy,
   driver_vp = make_passthrough_vertex_shader(st, GL_FALSE);
 
   if (st-pixel_xfer.pixelmap_enabled) {
- sv[1] = st-pixel_xfer.pixelmap_sampler_view;
+ sv[1] = NULL;
+ pipe_sampler_view_reference(sv[1],
+ st-pixel_xfer.pixelmap_sampler_view);
  num_sampler_view++;
   }
}
-- 
1.9.1

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