<div dir="ltr">Thanks for the feedback.<div> </div><div>I've updated the patch to integrate Ian's comments.</div><div> </div><div>Courtney</div></div><div class="gmail_extra"> <div class="gmail_quote"> On Fri, Feb 14, 2014 at 2:00 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On 02/14/2014 07:52 AM, Courtney Goeltzenleuchter wrote: > Decompressing ETC2 textures was causing intermitent segfault > by copying resulting 4x4 texel block to the destination texture > regardless of the size of the destination texture. Issue found > via application crash in GLBenchmark 3.0's Manhattan test. </div>So... the problem is that every ETC texture is (physically) a multiple of 4 width, but we may have allocated a decompression buffer that was the logical width of the texture. I think the code needs more of a comment explaining that. Otherwise, when someone comes back to it in a year (or uses it as a model for some other texture compression code), they won't understand why the code is the way it is. With that, this patch is Reviewed-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> One additional suggestion below that you can take or leave. > Signed-off-by: Courtney Goeltzenleuchter <courtney@LunarG.com> Also add: Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=74988" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=74988</a> Cc: "9.2 10.0 10.1" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>> <div class="im"> > --- > src/mesa/main/texcompress_etc.c | 49 +++++++++++++++++++++-------------------- > 1 file changed, 25 insertions(+), 24 deletions(-) > > diff --git a/src/mesa/main/texcompress_etc.c b/src/mesa/main/texcompress_etc.c > index e3862be..f9234b0 100644 > --- a/src/mesa/main/texcompress_etc.c > +++ b/src/mesa/main/texcompress_etc.c > @@ -684,9 +684,10 @@ etc2_unpack_rgb8(uint8_t *dst_row, > etc2_rgb8_parse_block(&block, src, > false /* punchthrough_alpha */); > </div>Alternately, the code below could be: const unsigned h = MIN2(bh, height - y); const unsigned w = MIN2(bw, width - x); ... for (j = 0; j < h; j++) { ... for (i = 0; i < w; i++) { <div class="HOEnZb"><div class="h5"> > - for (j = 0; j < bh; j++) { > + /* be sure to stay within the bounds of the texture */ > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgb8_fetch_texel(&block, i, j, dst, > false /* punchthrough_alpha */); > dst[3] = 255; > @@ -721,9 +722,9 @@ etc2_unpack_srgb8(uint8_t *dst_row, > etc2_rgb8_parse_block(&block, src, > false /* punchthrough_alpha */); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgb8_fetch_texel(&block, i, j, dst, > false /* punchthrough_alpha */); > /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */ > @@ -764,9 +765,9 @@ etc2_unpack_rgba8(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_rgba8_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgba8_fetch_texel(&block, i, j, dst); > dst += comps; > } > @@ -801,9 +802,9 @@ etc2_unpack_srgb8_alpha8(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_rgba8_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgba8_fetch_texel(&block, i, j, dst); > > /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */ > @@ -843,9 +844,9 @@ etc2_unpack_r11(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_r11_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_r11_fetch_texel(&block, i, j, dst); > dst += comps * comp_size; > } > @@ -879,10 +880,10 @@ etc2_unpack_rg11(uint8_t *dst_row, > /* red component */ > etc2_r11_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + > x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_r11_fetch_texel(&block, i, j, dst); > dst += comps * comp_size; > } > @@ -890,10 +891,10 @@ etc2_unpack_rg11(uint8_t *dst_row, > /* green component */ > etc2_r11_parse_block(&block, src + 8); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + > x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_r11_fetch_texel(&block, i, j, dst + comp_size); > dst += comps * comp_size; > } > @@ -926,10 +927,10 @@ etc2_unpack_signed_r11(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_r11_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + > x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_signed_r11_fetch_texel(&block, i, j, dst); > dst += comps * comp_size; > } > @@ -963,10 +964,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row, > /* red component */ > etc2_r11_parse_block(&block, src); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + > x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_signed_r11_fetch_texel(&block, i, j, dst); > dst += comps * comp_size; > } > @@ -974,10 +975,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row, > /* green component */ > etc2_r11_parse_block(&block, src + 8); > > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + > x * comps * comp_size; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_signed_r11_fetch_texel(&block, i, j, dst + comp_size); > dst += comps * comp_size; > } > @@ -1007,9 +1008,9 @@ etc2_unpack_rgb8_punchthrough_alpha1(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_rgb8_parse_block(&block, src, > true /* punchthrough_alpha */); > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgb8_fetch_texel(&block, i, j, dst, > true /* punchthrough_alpha */); > dst += comps; > @@ -1042,9 +1043,9 @@ etc2_unpack_srgb8_punchthrough_alpha1(uint8_t *dst_row, > for (x = 0; x < width; x+= bw) { > etc2_rgb8_parse_block(&block, src, > true /* punchthrough_alpha */); > - for (j = 0; j < bh; j++) { > + for (j = 0; j < bh && (j+y) < height; j++) { > uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps; > - for (i = 0; i < bw; i++) { > + for (i = 0; i < bw && (i+x) < width; i++) { > etc2_rgb8_fetch_texel(&block, i, j, dst, > true /* punchthrough_alpha */); > /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */ > </div></div></blockquote></div> <div> </div>-- <div dir="ltr">Courtney Goeltzenleuchter <div>LunarG</div><div><img src="http://media.lunarg.com/wp-content/themes/LunarG/images/logo.png" width="96" height="65"> </div></div> </div>