<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 2, 2017 at 10:49 AM, Lionel Landwerlin <span dir="ltr"><<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.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 29/09/17 04:23, Matt Turner wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Sep 15, 2017 at 9:01 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
---<br>
src/intel/blorp/blorp_clear.c | 24 +++++++++++++++++-------<br>
1 file changed, 17 insertions(+), 7 deletions(-)<br>
<br>
diff --git a/src/intel/blorp/blorp_clear.<wbr>c b/src/intel/blorp/blorp_clear.<wbr>c<br>
index 0feebef..e8b1e32 100644<br>
--- a/src/intel/blorp/blorp_clear.<wbr>c<br>
+++ b/src/intel/blorp/blorp_clear.<wbr>c<br>
@@ -442,14 +442,24 @@ blorp_clear(struct blorp_batch *batch,<br>
if (batch->blorp->isl_dev->info-><wbr>gen == 4 &&<br>
(params.dst.surf.usage & ISL_SURF_USAGE_CUBE_BIT)) {<br>
blorp_surf_convert_to_single_<wbr>slice(batch->blorp->isl_dev, ¶ms.dst);<br>
+ }<br>
+<br>
+ if (isl_format_is_compressed(para<wbr>ms.dst.surf.format)) {<br>
+ blorp_surf_convert_to_uncompr<wbr>essed(batch->blorp->isl_dev, ¶ms.dst,<br>
+ NULL, NULL, NULL, NULL);<br>
+ //&dst_x, &dst_y, &dst_w, &dst_h);<br>
</blockquote>
Did you mean to leave this as is?<br>
<br>
The previous patch (commit f395d0abc) caused a Coverity warning<br>
because you began checking if x and y are non-NULL in one place after<br>
dereferencing them under different conditions earlier. This code being<br>
commented out makes me wonder what was really intended.<br>
<br>
</blockquote>
<br></div></div>
That commented line should be removed but all parameters to NULL is the intended behavior.<br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">That's correct. More to Matt's point, yes, we dereference them under different conditions. The implicit assumption (which is currently always valid) is that if you specify width and height then you also specified X and Y. Prior to this patch, every caller specified x and y but only some of them specified width/height. As of this patch, there is now a caller which does not specify x and y (and also leaves width/height NULL). I don't think there's any point in (nor would it be possible to) supporting width/height without x/y. I'd be happy if someone added some asserts to that effect.</div><div class="gmail_extra"><br></div><div class="gmail_extra">--Jason<br></div></div>