On Tue, Feb 09, 2016 at 12:10:18PM -0800, Ian Romanick wrote: > On 02/09/2016 11:58 AM, Ben Widawsky wrote: > > 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. > > I think it's a good candidate for stable. It's pretty low risk, and it > fixes something real. > > Is there a piglit test? :) >
Anuj is working on it. > > 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
