[Mesa-dev] [PATCH] intel: Fix size of temporary etc1 buffer

[Tapani Pälli]

Hello;

On 09/20/2012 11:01 PM, Chad Versace wrote:
> Adding Tapani to the CC list. Removing Oliver.
> 
> On 09/20/2012 09:55 PM, Chad Versace wrote:
>> Fixes valgrind errors in piglit test
>> oes_compressed_etc1_rgb8_texture-miptree: an invalid write in
>> _mesa_store_compressed_store_texsubimage() at line 4406 and invalid reads
>> in texcompress_etc_tmp.h:etc1_parse_block().
>>
>> The calculation of the size of the temporary etc1 buffer allocated by
>> intel_miptree_map_etc1() was incorrect. Sometimes the allocated buffer was
>> too small, sometimes too large.  This patch corrects the size to that
>> expected by _mesa_store_compressed_store_texsubimage().

With GLBenchmark2.5 on Android including the patch I'm still having some
issues within etc1 unpack and I think these are related to the size of
the buffer which is eventually given for '_mesa_etc1_unpack_rgba8888',
I'll try to come up with a smaller test case for this if possible and
step with gdb for some more details. This patch looks proper and correct
for me, maybe these issues left originate from some place else.

>> Note: This is candidate for the 9.0 branch.
>> CC: Oliver McFadden <oliver.mcfadden at linux.intel.com>
>> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
>> ---
>>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>> index 556a82f..c2266a4 100644
>> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>> @@ -1273,6 +1273,14 @@ intel_miptree_map_etc1(struct intel_context *intel,
>>                         unsigned int level,
>>                         unsigned int slice)
>>  {
>> +   const int block_width = 4; /*in pixels*/
>> +   const int block_height = 4; /*in pixels*/
>> +   const int block_size = 8; /*in bytes*/
>> +
>> +   /* Size of a row of blocks, in bytes. */
>> +   const int row_size = block_size * ALIGN(map->w, block_width) / block_width;
>> +   const int num_rows = ALIGN(map->h, block_height) / block_height;
>> +
>>     /* For justification of these invariants,
>>      * see intel_mipmap_tree:wraps_etc1.
>>      */
>> @@ -1292,9 +1300,8 @@ intel_miptree_map_etc1(struct intel_context *intel,
>>     assert(map->x == 0);
>>     assert(map->y == 0);
>>  
>> -   /* Each ETC1 block contains 4x4 pixels in 8 bytes. */
>> -   map->stride = 2 * map->w;
>> -   map->buffer = map->ptr = malloc(map->stride * map->h);
>> +   map->stride = row_size;
>> +   map->buffer = map->ptr = malloc(row_size * num_rows);
>>  }
>>  
>>  static void
>>
> 


-- 

// Tapani


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 551 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120921/8670fe6b/attachment.pgp>