[Mesa-dev] [PATCH] i965: Make sure we blit a full compressed block

[Ilia Mirkin]

On Sat, Feb 6, 2016 at 2:50 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Is this fixing the same problem as this bug was reporting?
> https://bugs.freedesktop.org/show_bug.cgi?id=93358
>
> If so, perhaps mention it with a Bugzilla: tag.
>
> On Sat, Feb 6, 2016 at 1:41 PM, Ben Widawsky
> <benjamin.widawsky at intel.com> wrote:
>> This fixes an assertion failure in [at least] one of the Unreal Engine Linux
>> demo/games that uses DXT1 compression. Specifically, the "Vehicle Game". It
>> probably fixes other things as well, and if I had more time, I'd figure out how
>> piglit isn't exercising this.
>>
>> At some point, the game ends up trying to blit mip level whose size is 2x2,
>> which is smaller than a DXT1 block. As a result, the assertion in the blit path
>> is triggered. It should be safe to simply make sure we align the width and
>> height, which is sadly an example of compression being less efficient.
>>
>> NOTE: The demo seems to work fine without the assert, and therefore release
>> builds of mesa wouldn't stumble over this. Perhaps there is some unnoticeable
>> corruption, but I had trouble spotting it.
>>
>> Thanks to Jason for looking at my backtrace and figuring out what was going on.
>>
>> Cc: Jason Ekstrand <jason.ekstrand at intel.com>
>> Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/intel_copy_image.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c b/src/mesa/drivers/dri/i965/intel_copy_image.c
>> index 0a3337e..a4d897d 100644
>> --- a/src/mesa/drivers/dri/i965/intel_copy_image.c
>> +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
>> @@ -212,6 +212,7 @@ intel_copy_image_sub_data(struct gl_context *ctx,
>>     struct brw_context *brw = brw_context(ctx);
>>     struct intel_mipmap_tree *src_mt, *dst_mt;
>>     unsigned src_level, dst_level;
>> +   GLuint bw, bh;
>>
>>     if (_mesa_meta_CopyImageSubData_uncompressed(ctx,
>>                                                  src_image, src_renderbuffer,
>> @@ -275,6 +276,18 @@ intel_copy_image_sub_data(struct gl_context *ctx,
>>     intel_miptree_all_slices_resolve_depth(brw, dst_mt);
>>     intel_miptree_resolve_color(brw, dst_mt);
>>
>> +   _mesa_get_format_block_size(src_mt->format, &bw, &bh);
>> +   /* It's legal to have a WxH that's smaller than a compressed block. This
>> +    * happens for example when you are using a higher level LOD. For this case,
>> +    * we still want to copy the entire block, or else the decompression will be
>> +    * incorrect.
>> +    */
>> +   if (src_width < bw)
>> +      src_width = ALIGN(src_width, bw);
>> +
>> +   if (src_height < bh)
>> +      src_height = ALIGN(src_height, bh);

Separately,

ALIGN(uintptr_t value, int32_t alignment)
{
   assert((alignment > 0) && _mesa_is_pow_two(alignment));

This might cause sadness for the oddly-sized ASTC formats. There's a
ALIGN_NPOT() macro that may be more appropriate?

  -ilia

>> +
>>     if (copy_image_with_blitter(brw, src_mt, src_level,
>>                                 src_x, src_y, src_z,
>>                                 dst_mt, dst_level,
>> --
>> 2.7.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev