meta: Compute correct buffer size with SkipRows/SkipPixels
authorChris Wilson <chris@chris-wilson.co.uk>
Tue, 1 Sep 2015 08:31:15 +0000 (09:31 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Wed, 2 Sep 2015 09:08:39 +0000 (10:08 +0100)
If the user is specifying a subregion of a buffer using SKIP_ROWS and
SKIP_PIXELS, we must compute the buffer size carefully as the end of the
last row may be much shorter than stride*image_height*depth. The current
code tries to memcpy from beyond the end of the user data, for example
causing:

==28136== Invalid read of size 8
==28136==    at 0x4C2D94E: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915)
==28136==    by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856)
==28136==    by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208)
==28136==    by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600)
==28136==    by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631)
==28136==    by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103)
==28136==    by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176)
==28136==    by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195)
==28136==    by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654)
==28136==    by 0xB254C9F: texsubimage (teximage.c:3712)
==28136==    by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853)
==28136==    by 0x401CA0: UploadTexSubImage2D (teximage.c:171)
==28136==  Address 0xd8bfbe0 is 0 bytes after a block of size 1,024 alloc'd
==28136==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==28136==    by 0x402014: PerfDraw (teximage.c:270)
==28136==    by 0x402648: Draw (glmain.c:182)
==28136==    by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x4019C1: main (glmain.c:262)
==28136==
==28136== Invalid read of size 8
==28136==    at 0x4C2D940: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915)
==28136==    by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856)
==28136==    by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208)
==28136==    by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600)
==28136==    by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631)
==28136==    by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103)
==28136==    by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176)
==28136==    by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195)
==28136==    by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654)
==28136==    by 0xB254C9F: texsubimage (teximage.c:3712)
==28136==    by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853)
==28136==    by 0x401CA0: UploadTexSubImage2D (teximage.c:171)
==28136==  Address 0xd8bfbe8 is 8 bytes after a block of size 1,024 alloc'd
==28136==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==28136==    by 0x402014: PerfDraw (teximage.c:270)
==28136==    by 0x402648: Draw (glmain.c:182)
==28136==    by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0)
==28136==    by 0x4019C1: main (glmain.c:262)
==28136==

Fixes regression from commit 7f396189f073d626c5f7a2c232dac92b65f5a23f
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Mon Jan 5 18:17:04 2015 -0800

    meta: Add a BlitFramebuffers-based implementation of TexSubImage

v2: However, the teximage we create does need to be width x full_height x 1

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jason Ekstrand <jason.ekstrand@intel.com>
Cc: Neil Roberts <neil@linux.intel.com>
Reviewed-by Neil Roberts <neil@linux.intel.com>

src/mesa/drivers/common/meta_tex_subimage.c

index 16d8f5d4747b765f16dd32b7557412b1babd4e56..33c22aa139d28f9be295067fc599881542e82d94 100644 (file)
@@ -46,8 +46,9 @@
 #include "varray.h"
 
 static struct gl_texture_image *
-create_texture_for_pbo(struct gl_context *ctx, bool create_pbo,
-                       GLenum pbo_target, int width, int height,
+create_texture_for_pbo(struct gl_context *ctx,
+                       bool create_pbo, GLenum pbo_target,
+                       int dims, int width, int height, int depth,
                        GLenum format, GLenum type, const void *pixels,
                        const struct gl_pixelstore_attrib *packing,
                        GLuint *tmp_pbo, GLuint *tmp_tex)
@@ -73,13 +74,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo,
       return NULL;
 
    /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */
-   pixels = _mesa_image_address3d(packing, pixels,
-                                  width, height, format, type, 0, 0, 0);
+   uint32_t first_pixel = _mesa_image_offset(dims, packing, width, height,
+                                             format, type,
+                                             0, 0, 0);
+   uint32_t last_pixel =  _mesa_image_offset(dims, packing, width, height,
+                                             format, type,
+                                             depth-1, height-1, width);
    row_stride = _mesa_image_row_stride(packing, width, format, type);
 
    if (_mesa_is_bufferobj(packing->BufferObj)) {
       *tmp_pbo = 0;
       buffer_obj = packing->BufferObj;
+      first_pixel += (intptr_t)pixels;
    } else {
       bool is_pixel_pack = pbo_target == GL_PIXEL_PACK_BUFFER;
 
@@ -97,14 +103,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo,
        * data to avoid unnecessary data copying in _mesa_BufferData().
        */
       if (is_pixel_pack)
-         _mesa_BufferData(pbo_target, row_stride * height, NULL,
+         _mesa_BufferData(pbo_target,
+                          last_pixel - first_pixel,
+                          NULL,
                           GL_STREAM_READ);
       else
-         _mesa_BufferData(pbo_target, row_stride * height, pixels,
+         _mesa_BufferData(pbo_target,
+                          last_pixel - first_pixel,
+                          (char *)pixels + first_pixel,
                           GL_STREAM_DRAW);
 
       buffer_obj = packing->BufferObj;
-      pixels = NULL;
+      first_pixel = 0;
 
       _mesa_BindBuffer(pbo_target, 0);
    }
@@ -119,14 +129,21 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo,
 
    internal_format = _mesa_get_format_base_format(pbo_format);
 
+   /* The texture is addressed as a single very-tall image, so we
+    * need to pack the multiple image depths together taking the
+    * inter-image padding into account.
+    */
+   int image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight;
+   int full_height = image_height * (depth - 1) + height;
+
    tex_image = _mesa_get_tex_image(ctx, tex_obj, tex_obj->Target, 0);
-   _mesa_init_teximage_fields(ctx, tex_image, width, height, 1,
+   _mesa_init_teximage_fields(ctx, tex_image, width, full_height, 1,
                               0, internal_format, pbo_format);
 
    read_only = pbo_target == GL_PIXEL_UNPACK_BUFFER;
    if (!ctx->Driver.SetTextureStorageForBufferObject(ctx, tex_obj,
                                                      buffer_obj,
-                                                     (intptr_t)pixels,
+                                                     first_pixel,
                                                      row_stride,
                                                      read_only)) {
       _mesa_DeleteTextures(1, tmp_tex);
@@ -147,7 +164,7 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
                            const struct gl_pixelstore_attrib *packing)
 {
    GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
-   int full_height, image_height;
+   int image_height;
    struct gl_texture_image *pbo_tex_image;
    GLenum status;
    bool success = false;
@@ -171,11 +188,10 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
     * property.
     */
    image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight;
-   full_height = image_height * (depth - 1) + height;
 
    pbo_tex_image = create_texture_for_pbo(ctx, create_pbo,
                                           GL_PIXEL_UNPACK_BUFFER,
-                                          width, full_height,
+                                          dims, width, height, depth,
                                           format, type, pixels, packing,
                                           &pbo, &pbo_tex);
    if (!pbo_tex_image)
@@ -277,7 +293,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
                               const struct gl_pixelstore_attrib *packing)
 {
    GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
-   int full_height, image_height;
+   int image_height;
    struct gl_texture_image *pbo_tex_image;
    struct gl_renderbuffer *rb = NULL;
    GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format);
@@ -324,10 +340,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
     * property.
     */
    image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight;
-   full_height = image_height * (depth - 1) + height;
 
    pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER,
-                                          width, full_height * depth,
+                                          dims, width, height, depth,
                                           format, type, pixels, packing,
                                           &pbo, &pbo_tex);
    if (!pbo_tex_image)