On Mon, Feb 08, 2016 at 09:48:54PM -0800, Jason Ekstrand wrote: > On Sat, Feb 6, 2016 at 6:11 PM, Ben Widawsky <benjamin.widaw...@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". > > > > 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. > > > > v2: Use NPOT alignment to make sure ASTC is handled properly (Ilia) > > Remove comment about how this doesn't fix other bugs, because it does. > > > > Cc: Jason Ekstrand <jason.ekstr...@intel.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93358 > > Signed-off-by: Ben Widawsky <benjamin.widaw...@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..42bd7ff 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_NPOT(src_width, bw); > > + > > + if (src_height < bh) > > + src_height = ALIGN_NPOT(src_height, bh); > > > > I've been going back-and-forth as to whether or not this is the right place > to do this or if we wanted it further up or down the stack. At the end of > the day, I concluded that it's as good a place as any. > > Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com> > Cc "11.0 11.1" <mesa-sta...@lists.freedesktop.org>
I left out stable intentionally because we will only hit the assert in debug builds, and AFAICT, we'll only get minor corruption by this not being there - but it's up to you. I just figured I'd share my thoughts in case they weren't clear. I can add stable if you still think it wise. Thanks for review. > > --Jason > > > > + > > 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev