[PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

View: New views
4 Messages — Rating Filter:   Alert me  

[PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

by Marek Olšák :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi,

the attached patch fixes a possible crash in function draw_stencil_pixels in mesa/state_tracker. I've also updated the list of formats we read from to prevent an assertion failing later in the code.

This makes glean/depthStencil work on r300g and softpipe.

Best regards
Marek Olšák

[0001-mesa-st-fix-crash-when-drawing-stencil-pixels-outsid.patch]

From 2b79c8adc5a92410cdfe4ae8c15880a73f839159 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Marek=20Ol=C5=A1=C3=A1k?= <maraeo@...>
Date: Sat, 31 Oct 2009 19:38:29 +0100
Subject: [PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

Also, add a missing depth-stencil format to the list of formats we want
to read from.
---
 src/mesa/state_tracker/st_cb_drawpixels.c |    6 ++++++
 src/mesa/state_tracker/st_cb_texture.c    |    3 ++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
index 0a6bfcd..0699404 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -669,6 +669,12 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
    strb = st_renderbuffer(ctx->DrawBuffer->
                           Attachment[BUFFER_STENCIL].Renderbuffer);
 
+   /* Do not draw outside the drawbuffer */
+   if (x + width > ctx->DrawBuffer->Width)
+      width = ctx->DrawBuffer->Width - x;
+   if (y + height > ctx->DrawBuffer->Height)
+      height = ctx->DrawBuffer->Height - y;
+
    if (st_fb_orientation(ctx->DrawBuffer) == Y_0_TOP) {
       y = ctx->DrawBuffer->Height - y - height;
    }
diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index 878a40f..6f287ff 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1309,7 +1309,8 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
        srcX, srcY,
        width, height);
 
-   if (baseFormat == GL_DEPTH_COMPONENT &&
+   if ((baseFormat == GL_DEPTH_COMPONENT ||
+        baseFormat == GL_DEPTH24_STENCIL8) &&
        pf_is_depth_and_stencil(stImage->pt->format))
       transfer_usage = PIPE_TRANSFER_READ_WRITE;
    else
--
1.6.3.3



------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

by Brian Paul-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Marek Olšák wrote:

> Hi,
>
> the attached patch fixes a possible crash in function
> draw_stencil_pixels in mesa/state_tracker. I've also updated the list of
> formats we read from to prevent an assertion failing later in the code.
>
> This makes glean/depthStencil work on r300g and softpipe.
>
> Best regards
> Marek Olšák
>
diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
b/src/mesa/state_tracker/st_cb_drawpixels.c
index 0a6bfcd..0699404 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -669,6 +669,12 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
     strb = st_renderbuffer(ctx->DrawBuffer->
                            Attachment[BUFFER_STENCIL].Renderbuffer);

+   /* Do not draw outside the drawbuffer */
+   if (x + width > ctx->DrawBuffer->Width)
+      width = ctx->DrawBuffer->Width - x;
+   if (y + height > ctx->DrawBuffer->Height)
+      height = ctx->DrawBuffer->Height - y;

I think you can use _mesa_clip_drawpixels() here instead.  See swrast
or other drivers for example usage.


The second change should be a totally separate patch since it's
independent of the drawpixels clipping issue.


diff --git a/src/mesa/state_tracker/st_cb_texture.c
b/src/mesa/state_tracker/st_cb_texture.c
index 878a40f..6f287ff 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1309,7 +1309,8 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum
target, GLint level,
        srcX, srcY,
        width, height);

-   if (baseFormat == GL_DEPTH_COMPONENT &&
+   if ((baseFormat == GL_DEPTH_COMPONENT ||
+        baseFormat == GL_DEPTH24_STENCIL8) &&
         pf_is_depth_and_stencil(stImage->pt->format))
        transfer_usage = PIPE_TRANSFER_READ_WRITE;
     else


I think there's a few things wrong with the depth/stencil path in this
code.  Can you try the attached patch?

I also suspec that util_blit_pixels_writemask() isn't working properly
when blitting depth/stencil pixels.

-Brian

diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index 878a40f..00a0e4c 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1309,7 +1309,8 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
        srcX, srcY,
        width, height);
 
-   if (baseFormat == GL_DEPTH_COMPONENT &&
+   if ((baseFormat == GL_DEPTH_COMPONENT ||
+        baseFormat == GL_DEPTH_STENCIL) &&
        pf_is_depth_and_stencil(stImage->pt->format))
       transfer_usage = PIPE_TRANSFER_READ_WRITE;
    else
@@ -1322,7 +1323,7 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
                                   destX, destY, width, height);
 
    if (baseFormat == GL_DEPTH_COMPONENT ||
-       baseFormat == GL_DEPTH24_STENCIL8) {
+       baseFormat == GL_DEPTH_STENCIL) {
       const GLboolean scaleOrBias = (ctx->Pixel.DepthScale != 1.0F ||
                                      ctx->Pixel.DepthBias != 0.0F);
       GLint row, yStep;
@@ -1455,7 +1456,7 @@ st_copy_texsubimage(GLcontext *ctx,
    struct gl_texture_image *texImage =
       _mesa_select_tex_image(ctx, texObj, target, level);
    struct st_texture_image *stImage = st_texture_image(texImage);
-   const GLenum texBaseFormat = texImage->InternalFormat;
+   const GLenum texBaseFormat = texImage->_BaseFormat;
    struct gl_framebuffer *fb = ctx->ReadBuffer;
    struct st_renderbuffer *strb;
    struct pipe_context *pipe = ctx->st->pipe;
@@ -1476,12 +1477,9 @@ st_copy_texsubimage(GLcontext *ctx,
 
    /* determine if copying depth or color data */
    if (texBaseFormat == GL_DEPTH_COMPONENT ||
-       texBaseFormat == GL_DEPTH24_STENCIL8) {
+       texBaseFormat == GL_DEPTH_STENCIL) {
       strb = st_renderbuffer(fb->_DepthBuffer);
    }
-   else if (texBaseFormat == GL_DEPTH_STENCIL_EXT) {
-      strb = st_renderbuffer(fb->_StencilBuffer);
-   }
    else {
       /* texBaseFormat == GL_RGB, GL_RGBA, GL_ALPHA, etc */
       strb = st_renderbuffer(fb->_ColorReadBuffer);

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

by Marek Olšák :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Brian,

I tried your patch and it solves the buffer-mapping issue.

I attached a new patch that fixes the clipping issue. Please, review/push.

Best regards
Marek Olšák


On Mon, Nov 2, 2009 at 6:12 PM, Brian Paul <brianp@...> wrote:
Marek Olšák wrote:
Hi,

the attached patch fixes a possible crash in function draw_stencil_pixels in mesa/state_tracker. I've also updated the list of formats we read from to prevent an assertion failing later in the code.

This makes glean/depthStencil work on r300g and softpipe.

Best regards
Marek Olšák


diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
index 0a6bfcd..0699404 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -669,6 +669,12 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
   strb = st_renderbuffer(ctx->DrawBuffer->
                          Attachment[BUFFER_STENCIL].Renderbuffer);

+   /* Do not draw outside the drawbuffer */
+   if (x + width > ctx->DrawBuffer->Width)
+      width = ctx->DrawBuffer->Width - x;
+   if (y + height > ctx->DrawBuffer->Height)
+      height = ctx->DrawBuffer->Height - y;

I think you can use _mesa_clip_drawpixels() here instead.  See swrast or other drivers for example usage.


The second change should be a totally separate patch since it's independent of the drawpixels clipping issue.


diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index 878a40f..6f287ff 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1309,7 +1309,8 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
                                              srcX, srcY,
                                              width, height);

-   if (baseFormat == GL_DEPTH_COMPONENT &&
+   if ((baseFormat == GL_DEPTH_COMPONENT ||
+        baseFormat == GL_DEPTH24_STENCIL8) &&
       pf_is_depth_and_stencil(stImage->pt->format))
      transfer_usage = PIPE_TRANSFER_READ_WRITE;
   else


I think there's a few things wrong with the depth/stencil path in this code.  Can you try the attached patch?

I also suspec that util_blit_pixels_writemask() isn't working properly when blitting depth/stencil pixels.

-Brian

diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index 878a40f..00a0e4c 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1309,7 +1309,8 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
                                              srcX, srcY,
                                              width, height);

-   if (baseFormat == GL_DEPTH_COMPONENT &&
+   if ((baseFormat == GL_DEPTH_COMPONENT ||
+        baseFormat == GL_DEPTH_STENCIL) &&
       pf_is_depth_and_stencil(stImage->pt->format))
      transfer_usage = PIPE_TRANSFER_READ_WRITE;
   else
@@ -1322,7 +1323,7 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
                                  destX, destY, width, height);

   if (baseFormat == GL_DEPTH_COMPONENT ||
-       baseFormat == GL_DEPTH24_STENCIL8) {
+       baseFormat == GL_DEPTH_STENCIL) {
      const GLboolean scaleOrBias = (ctx->Pixel.DepthScale != 1.0F ||
                                     ctx->Pixel.DepthBias != 0.0F);
      GLint row, yStep;
@@ -1455,7 +1456,7 @@ st_copy_texsubimage(GLcontext *ctx,
   struct gl_texture_image *texImage =
      _mesa_select_tex_image(ctx, texObj, target, level);
   struct st_texture_image *stImage = st_texture_image(texImage);
-   const GLenum texBaseFormat = texImage->InternalFormat;
+   const GLenum texBaseFormat = texImage->_BaseFormat;
   struct gl_framebuffer *fb = ctx->ReadBuffer;
   struct st_renderbuffer *strb;
   struct pipe_context *pipe = ctx->st->pipe;
@@ -1476,12 +1477,9 @@ st_copy_texsubimage(GLcontext *ctx,

   /* determine if copying depth or color data */
   if (texBaseFormat == GL_DEPTH_COMPONENT ||
-       texBaseFormat == GL_DEPTH24_STENCIL8) {
+       texBaseFormat == GL_DEPTH_STENCIL) {
      strb = st_renderbuffer(fb->_DepthBuffer);
   }
-   else if (texBaseFormat == GL_DEPTH_STENCIL_EXT) {
-      strb = st_renderbuffer(fb->_StencilBuffer);
-   }
   else {
      /* texBaseFormat == GL_RGB, GL_RGBA, GL_ALPHA, etc */
      strb = st_renderbuffer(fb->_ColorReadBuffer);



[0001-mesa-st-clip-pixels-in-draw_stencil_pixels-to-avoid-.patch]

From 23a8a4fdc2848be822c9884d2b758909287e9a6e Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Marek=20Ol=C5=A1=C3=A1k?= <maraeo@...>
Date: Tue, 3 Nov 2009 16:16:05 +0100
Subject: [PATCH] mesa/st: clip pixels in draw_stencil_pixels to avoid crash

---
 src/mesa/state_tracker/st_cb_drawpixels.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
index 0a6bfcd..1d33e81 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -665,6 +665,15 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
    const GLboolean zoom = ctx->Pixel.ZoomX != 1.0 || ctx->Pixel.ZoomY != 1.0;
    GLint skipPixels;
    ubyte *stmap;
+   struct gl_pixelstore_attrib clippedUnpack = *unpack;
+
+   if (!zoom) {
+      if (!_mesa_clip_drawpixels(ctx, &x, &y, &width, &height,
+                                 &clippedUnpack)) {
+         /* totally clipped */
+         return;
+      }
+   }
 
    strb = st_renderbuffer(ctx->DrawBuffer->
                           Attachment[BUFFER_STENCIL].Renderbuffer);
@@ -685,7 +694,7 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
 
    stmap = screen->transfer_map(screen, pt);
 
-   pixels = _mesa_map_pbo_source(ctx, unpack, pixels);
+   pixels = _mesa_map_pbo_source(ctx, &clippedUnpack, pixels);
    assert(pixels);
 
    /* if width > MAX_WIDTH, have to process image in chunks */
@@ -698,17 +707,18 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
          GLubyte sValues[MAX_WIDTH];
          GLuint zValues[MAX_WIDTH];
          GLenum destType = GL_UNSIGNED_BYTE;
-         const GLvoid *source = _mesa_image_address2d(unpack, pixels,
+         const GLvoid *source = _mesa_image_address2d(&clippedUnpack, pixels,
                                                       width, height,
                                                       format, type,
                                                       row, skipPixels);
          _mesa_unpack_stencil_span(ctx, spanWidth, destType, sValues,
-                                   type, source, unpack,
+                                   type, source, &clippedUnpack,
                                    ctx->_ImageTransferState);
 
          if (format == GL_DEPTH_STENCIL) {
             _mesa_unpack_depth_span(ctx, spanWidth, GL_UNSIGNED_INT, zValues,
-                                    (1 << 24) - 1, type, source, unpack);
+                                    (1 << 24) - 1, type, source,
+                                    &clippedUnpack);
          }
 
          if (zoom) {
@@ -779,7 +789,7 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y,
       skipPixels += spanWidth;
    }
 
-   _mesa_unmap_pbo_source(ctx, unpack);
+   _mesa_unmap_pbo_source(ctx, &clippedUnpack);
 
    /* unmap the stencil buffer */
    screen->transfer_unmap(screen, pt);
--
1.6.3.3



------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [PATCH] mesa/st: fix crash when drawing stencil pixels outside the drawbuffer

by Brian Paul-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Marek Olšák wrote:
> Hi Brian,
>
> I tried your patch and it solves the buffer-mapping issue.

OK, I've committed that, plus another depth/stencil glCopyTexImage bug.

> I attached a new patch that fixes the clipping issue. Please, review/push.

Committed.  Thanks.

-Brian


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev