<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 18, 2017 at 9:46 PM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_-4127542766644534891HOEnZb"><div class="m_-4127542766644534891h5">On Thursday, August 17, 2017 10:26:44 PM PDT Jason Ekstrand wrote:<br>
> On August 17, 2017 4:36:42 PM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote:<br>
><br>
> > ISL already offers functions to fill out most kinds of SURFACE_STATE,<br>
> > so why not handle null surfaces too?<br>
> ><br>
> > Null surfaces are simple, so we can just take the dimensions, rather<br>
> > than an entirte fill structure.<br>
> > ---<br>
> >  src/intel/isl/isl.c               |  7 +++++++<br>
> >  src/intel/isl/isl.h               |  4 ++++<br>
> >  src/intel/isl/isl_genX_priv.h     |  3 +++<br>
> >  src/intel/isl/isl_surface_stat<wbr>e.c | 26 ++++++++++++++++++++++++++<br>
> >  4 files changed, 40 insertions(+)<br>
> ><br>
> >  Applies on top of Jason's patches:<br>
> >  <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-August/166628.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2017-August/<wbr>166628.html</a><br>
> ><br>
> > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c<br>
> > index 3788f9c2ead..59f512fc050 100644<br>
> > --- a/src/intel/isl/isl.c<br>
> > +++ b/src/intel/isl/isl.c<br>
> > @@ -1811,6 +1811,13 @@ isl_buffer_fill_state_s(const struct isl_device<br>
> > *dev, void *state,<br>
> >     isl_genX_call(dev, buffer_fill_state_s, state, info);<br>
> >  }<br>
> ><br>
> > +void<br>
> > +isl_null_fill_state(const struct isl_device *dev, void *state,<br>
> > +                    struct isl_extent3d size)<br>
><br>
> I might be inclined to make this an extent4d, assert that one off depth and<br>
> array_length is zero, and take the maximum of the two as the depth.  Thoughts?<br>
<br>
</div></div>I suppose if you wanted a null surface with a different depth and render<br>
target view extent, then an isl_extent4d could be useful.  But...neither<br>
driver actually wants to do that today.  So it seems simpler to keep them<br>
the same, and make the caller pass the width/height/depth they want the<br>
surface to have.  Seems like less magic.  That's my preference, anyway.<span><br></span></blockquote><div><br></div><div>My thought was more that layered rendering is more of a 2d array thing so you want width, height, and array_len and not depth.  ISL tries very hard to distinguish between depth and array_len.  To be honest, it's one of the more annoying parts of the API though it is useful for clerity at some points.  I think 3D is probably fine in this case.</div><div><br></div><div>All three are</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> > +{<br>
> > +   isl_genX_call(dev, null_fill_state, state, size);<br>
><br>
> This is so much nicer.  Thanks for complaining.<br>
<br>
</span>Thanks for tidying it up :)</blockquote></div><br></div></div>