Re: [Mesa-dev] [PATCH 1/4] common: Correct texture initialization in create_texture_for_pbo.
On Friday, February 20, 2015 01:30:53 PM Laura Ekstrand wrote: Solves bugs related to the driver not setting up the texture miptree correctly, leading to faulty PBO uploads and downloads. --- src/mesa/drivers/common/meta_tex_subimage.c | 13 + src/mesa/drivers/dri/i965/intel_tex.c | 3 ++- src/mesa/main/dd.h | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) Hi Laura, It looks like you're fixing several different bugs in this one patch - and from your commit message, it's not really obvious what those bugs are or how they manifest. Would you mind splitting this into a couple patches and explaining each fix in the respective commit message? diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 68c8273..f4f7716 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -51,7 +51,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, { uint32_t pbo_format; GLenum internal_format; - unsigned row_stride; + unsigned row_stride, image_stride; struct gl_buffer_object *buffer_obj; struct gl_texture_object *tex_obj; struct gl_texture_image *tex_image; @@ -74,6 +74,8 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, pixels = _mesa_image_address3d(packing, pixels, width, height, format, type, 0, 0, 0); row_stride = _mesa_image_row_stride(packing, width, format, type); + image_stride = _mesa_image_image_stride(packing, width, height, format, + type); For example, I'm not really sure what this is attempting to solve or why it's necessary. It would be great to briefly explain that in the commit message (i.e. what problem did you encounter, and why does this fix it?) if (_mesa_is_bufferobj(packing-BufferObj)) { *tmp_pbo = 0; @@ -89,7 +91,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, */ _mesa_BindBuffer(pbo_target, *tmp_pbo); - _mesa_BufferData(pbo_target, row_stride * height, pixels, GL_STREAM_DRAW); + _mesa_BufferData(pbo_target, image_stride * depth, pixels, GL_STREAM_DRAW); buffer_obj = ctx-Unpack.BufferObj; pixels = NULL; @@ -99,9 +101,11 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, _mesa_GenTextures(1, tmp_tex); tex_obj = _mesa_lookup_texture(ctx, *tmp_tex); - tex_obj-Target = depth 1 ? GL_TEXTURE_2D_ARRAY : GL_TEXTURE_2D; - tex_obj-Immutable = GL_TRUE; _mesa_initialize_texture_object(ctx, tex_obj, *tmp_tex, GL_TEXTURE_2D); + /* This must be set after _mesa_initialize_texture_object, not before. */ + tex_obj-Immutable = GL_TRUE; + /* This is required for interactions with ARB_texture_view. */ + tex_obj-NumLayers = 1; This seems separate from the other change - gl_texture_objects with Immutable set must also have NumLayers set to value 0. Otherwise, we use (0-1)=0x as the depth when setting up SURFACE_STATE, triggering assertions. Thanks, Laura! internal_format = _mesa_get_format_base_format(pbo_format); @@ -114,6 +118,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, buffer_obj, (intptr_t)pixels, row_stride, + image_stride, read_only)) { _mesa_DeleteTextures(1, tmp_tex); _mesa_DeleteBuffers(1, tmp_pbo); diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c index 2d3009a..3a0c09a 100644 --- a/src/mesa/drivers/dri/i965/intel_tex.c +++ b/src/mesa/drivers/dri/i965/intel_tex.c @@ -305,6 +305,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, struct gl_buffer_object *buffer_obj, uint32_t buffer_offset, uint32_t row_stride, +uint32_t image_stride, bool read_only) { struct brw_context *brw = brw_context(ctx); @@ -334,7 +335,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, drm_intel_bo *bo = intel_bufferobj_buffer(brw, intel_buffer_obj, buffer_offset, - row_stride * image-Height); + image_stride * image-Depth); intel_texobj-mt = intel_miptree_create_for_bo(brw, bo, image-TexFormat, diff
[Mesa-dev] [PATCH 1/4] common: Correct texture initialization in create_texture_for_pbo.
Solves bugs related to the driver not setting up the texture miptree correctly, leading to faulty PBO uploads and downloads. --- src/mesa/drivers/common/meta_tex_subimage.c | 13 + src/mesa/drivers/dri/i965/intel_tex.c | 3 ++- src/mesa/main/dd.h | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 68c8273..f4f7716 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -51,7 +51,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, { uint32_t pbo_format; GLenum internal_format; - unsigned row_stride; + unsigned row_stride, image_stride; struct gl_buffer_object *buffer_obj; struct gl_texture_object *tex_obj; struct gl_texture_image *tex_image; @@ -74,6 +74,8 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, pixels = _mesa_image_address3d(packing, pixels, width, height, format, type, 0, 0, 0); row_stride = _mesa_image_row_stride(packing, width, format, type); + image_stride = _mesa_image_image_stride(packing, width, height, format, + type); if (_mesa_is_bufferobj(packing-BufferObj)) { *tmp_pbo = 0; @@ -89,7 +91,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, */ _mesa_BindBuffer(pbo_target, *tmp_pbo); - _mesa_BufferData(pbo_target, row_stride * height, pixels, GL_STREAM_DRAW); + _mesa_BufferData(pbo_target, image_stride * depth, pixels, GL_STREAM_DRAW); buffer_obj = ctx-Unpack.BufferObj; pixels = NULL; @@ -99,9 +101,11 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, _mesa_GenTextures(1, tmp_tex); tex_obj = _mesa_lookup_texture(ctx, *tmp_tex); - tex_obj-Target = depth 1 ? GL_TEXTURE_2D_ARRAY : GL_TEXTURE_2D; - tex_obj-Immutable = GL_TRUE; _mesa_initialize_texture_object(ctx, tex_obj, *tmp_tex, GL_TEXTURE_2D); + /* This must be set after _mesa_initialize_texture_object, not before. */ + tex_obj-Immutable = GL_TRUE; + /* This is required for interactions with ARB_texture_view. */ + tex_obj-NumLayers = 1; internal_format = _mesa_get_format_base_format(pbo_format); @@ -114,6 +118,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, buffer_obj, (intptr_t)pixels, row_stride, + image_stride, read_only)) { _mesa_DeleteTextures(1, tmp_tex); _mesa_DeleteBuffers(1, tmp_pbo); diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c index 2d3009a..3a0c09a 100644 --- a/src/mesa/drivers/dri/i965/intel_tex.c +++ b/src/mesa/drivers/dri/i965/intel_tex.c @@ -305,6 +305,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, struct gl_buffer_object *buffer_obj, uint32_t buffer_offset, uint32_t row_stride, +uint32_t image_stride, bool read_only) { struct brw_context *brw = brw_context(ctx); @@ -334,7 +335,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, drm_intel_bo *bo = intel_bufferobj_buffer(brw, intel_buffer_obj, buffer_offset, - row_stride * image-Height); + image_stride * image-Depth); intel_texobj-mt = intel_miptree_create_for_bo(brw, bo, image-TexFormat, diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h index ec8662b..9de08e2 100644 --- a/src/mesa/main/dd.h +++ b/src/mesa/main/dd.h @@ -429,6 +429,7 @@ struct dd_function_table { struct gl_buffer_object *bufferObj, uint32_t buffer_offset, uint32_t row_stride, +uint32_t image_stride, bool read_only); /** -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] common: Correct texture initialization in create_texture_for_pbo.
On Fri, Feb 20, 2015 at 4:30 PM, Laura Ekstrand la...@jlekstrand.net wrote: Solves bugs related to the driver not setting up the texture miptree correctly, leading to faulty PBO uploads and downloads. --- src/mesa/drivers/common/meta_tex_subimage.c | 13 + src/mesa/drivers/dri/i965/intel_tex.c | 3 ++- src/mesa/main/dd.h | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 68c8273..f4f7716 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -51,7 +51,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, { uint32_t pbo_format; GLenum internal_format; - unsigned row_stride; + unsigned row_stride, image_stride; struct gl_buffer_object *buffer_obj; struct gl_texture_object *tex_obj; struct gl_texture_image *tex_image; @@ -74,6 +74,8 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, pixels = _mesa_image_address3d(packing, pixels, width, height, format, type, 0, 0, 0); row_stride = _mesa_image_row_stride(packing, width, format, type); + image_stride = _mesa_image_image_stride(packing, width, height, format, + type); if (_mesa_is_bufferobj(packing-BufferObj)) { *tmp_pbo = 0; @@ -89,7 +91,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, */ _mesa_BindBuffer(pbo_target, *tmp_pbo); - _mesa_BufferData(pbo_target, row_stride * height, pixels, GL_STREAM_DRAW); + _mesa_BufferData(pbo_target, image_stride * depth, pixels, GL_STREAM_DRAW); buffer_obj = ctx-Unpack.BufferObj; pixels = NULL; @@ -99,9 +101,11 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, _mesa_GenTextures(1, tmp_tex); tex_obj = _mesa_lookup_texture(ctx, *tmp_tex); - tex_obj-Target = depth 1 ? GL_TEXTURE_2D_ARRAY : GL_TEXTURE_2D; - tex_obj-Immutable = GL_TRUE; _mesa_initialize_texture_object(ctx, tex_obj, *tmp_tex, GL_TEXTURE_2D); + /* This must be set after _mesa_initialize_texture_object, not before. */ + tex_obj-Immutable = GL_TRUE; + /* This is required for interactions with ARB_texture_view. */ + tex_obj-NumLayers = 1; internal_format = _mesa_get_format_base_format(pbo_format); @@ -114,6 +118,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, buffer_obj, (intptr_t)pixels, row_stride, + image_stride, read_only)) { _mesa_DeleteTextures(1, tmp_tex); _mesa_DeleteBuffers(1, tmp_pbo); diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c index 2d3009a..3a0c09a 100644 --- a/src/mesa/drivers/dri/i965/intel_tex.c +++ b/src/mesa/drivers/dri/i965/intel_tex.c @@ -305,6 +305,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, struct gl_buffer_object *buffer_obj, uint32_t buffer_offset, uint32_t row_stride, +uint32_t image_stride, bool read_only) { struct brw_context *brw = brw_context(ctx); @@ -334,7 +335,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx, drm_intel_bo *bo = intel_bufferobj_buffer(brw, intel_buffer_obj, buffer_offset, - row_stride * image-Height); + image_stride * image-Depth); intel_texobj-mt = intel_miptree_create_for_bo(brw, bo, image-TexFormat, diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h index ec8662b..9de08e2 100644 --- a/src/mesa/main/dd.h +++ b/src/mesa/main/dd.h @@ -429,6 +429,7 @@ struct dd_function_table { struct gl_buffer_object *bufferObj, uint32_t buffer_offset, uint32_t row_stride, +uint32_t image_stride, bool read_only); Given that the next patch fixes it to only use 2D textures, we shouldn't need the changes to the DD table. In fact, we should take all the stuff about array textures out of the DD table entry, comment, and implementation. However,