On Thu 16 Jun 2016, Jason Ekstrand wrote: > On Thu, Jun 16, 2016 at 10:39 AM, Chad Versace <chad.vers...@intel.com> > wrote: > > > On Sat 11 Jun 2016, Jason Ekstrand wrote: > > > --- > > > src/intel/isl/isl_surface_state.c | 28 ++++++++-------------------- > > > 1 file changed, 8 insertions(+), 20 deletions(-) > > > > > > diff --git a/src/intel/isl/isl_surface_state.c > > b/src/intel/isl/isl_surface_state.c > > > index 50570aa..1e94e60 100644 > > > --- a/src/intel/isl/isl_surface_state.c > > > +++ b/src/intel/isl/isl_surface_state.c > > > @@ -110,9 +110,8 @@ get_surftype(enum isl_surf_dim dim, > > isl_surf_usage_flags_t usage) > > > /* > > > * Get the values to pack into > > RENDER_SUFFACE_STATE.SurfaceHorizontalAlignment > > > * and SurfaceVerticalAlignment. > > > */ > > > -static void > > > -get_halign_valign(const struct isl_surf *surf, > > > - uint32_t *halign, uint32_t *valign) > > > +static struct isl_extent3d > > > +get_image_alignment(const struct isl_surf *surf) > > > > > > The function comment is incorrect post-patch. It should say something to > > the tune of "Returns indices into isl_to_gen_halign, isl_to_gen_valign". > > Specifically, the function comment needs to clarify (with as few words > > as possible) that the units of the returned extent is neither samples, > > pixels, nor elements, but something entirely different--array indices-- > > because it's not really an extent at all. > > > > Right. It's the "logical" halign/valign values not the actual hardware > enums.
But even the term "logical values" is overly ambiguous. Logical values of *what*? On some gens, the returned value is in units of surface samples; other gens, surface elements. So, in effect, the returned value is a *unitless* array index. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
