<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 25, 2016 at 11:52 PM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 2016-10-25 at 22:53 -0700, Jason Ekstrand wrote:<br>
> With dealing with rectangles in compressed images, you can have a<br>
> width or<br>
> height that isn't a multiple of the corresponding compression block<br>
> dimension but only if that edge of your rectangle is on the edge of<br>
> the<br>
> image.  When we call convert_to_single_slice, it creates an 2-D image<br>
> and a<br>
> set of tile offsets into that image.  When detecting the right-edge<br>
> and<br>
> bottom-edge cases, we weren't including the tile offsets so the<br>
> assert<br>
> would misfire.  This caused crashes in a few UE4 demos<br>
><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
> Reported-by: "Eero Tamminen" <<a href="mailto:eero.t.tamminen@intel.com">eero.t.tamminen@intel.com</a>><br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=98431" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=98431</a><br>
> Cc: "13.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.<wbr>freedesktop.org</a>><br>
> ---<br>
>  src/intel/blorp/blorp_blit.c | 8 ++++++--<br>
>  1 file changed, 6 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/intel/blorp/blorp_blit.c<br>
> b/src/intel/blorp/blorp_blit.c<br>
> index 0c3ee72..9357a72 100644<br>
> --- a/src/intel/blorp/blorp_blit.c<br>
> +++ b/src/intel/blorp/blorp_blit.c<br>
> @@ -1761,10 +1761,14 @@ surf_convert_to_uncompressed(<wbr>const struct<br>
> isl_device *isl_dev,<br>
>     surf_convert_to_single_<wbr>slice(isl_dev, info);<br>
>  <br>
>     if (width || height) {<br>
> +#ifndef NDEBUG<br>
> +      uint32_t right_edge_px = info->tile_x_sa + *x + *width;<br>
> +      uint32_t bottom_edge_px = info->tile_y_sa + *y + *height;<br>
>        assert(*width % fmtl->bw == 0 ||<br>
> -             *x + *width == info->surf.logical_level0_px.<wbr>width);<br>
> +             right_edge_px == info->surf.logical_level0_px.<wbr>width);<br>
>        assert(*height % fmtl->bh == 0 ||<br>
> -             *y + *height == info->surf.logical_level0_px.<wbr>height);<br>
> +             bottom_edge_px == info->surf.logical_level0_px.<wbr>height);<br>
> +#endif<br>
<br>
</div></div>Makes sense to me.<br>
<span class=""><br>
>        *width = DIV_ROUND_UP(*width, fmtl->bw);<br>
>        *height = DIV_ROUND_UP(*height, fmtl->bh);<br>
<br>
</span>In that case, we end up rounding up here. I suppose that is handled<br>
properly going forward from here, right? I ask because the previous<br>
asserts suggested that this might not have been considered during the<br>
implementation (although the DIV_ROUND_UP() here suggests that it was).<br></blockquote><div><br></div><div>Right.  The DIV_ROUND_UP is here exactly to handle the unaligned bottom-right edge case.  All of the other cases require alignment.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Assuming this is all fine, this patch is:<br>
Reviewed-by: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>><br>
<br>
>     }<br>
</blockquote></div><br></div></div>