On Mon, Oct 19, 2015 at 02:29:04PM -0700, Anuj Phogat wrote:
> On Thu, Aug 13, 2015 at 2:51 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> > Vertical alignment is not applicable to 1D textures.
> >
> > Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_tex_layout.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > index 4e44b15..edd7518 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > @@ -277,7 +277,9 @@ intel_vertical_texture_alignment_unit(struct
> > brw_context *brw,
> > if (mt->format == MESA_FORMAT_S_UINT8)
> > return brw->gen >= 7 ? 8 : 4;
> >
> > - if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
> > + if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE &&
> > + mt->target != GL_TEXTURE_1D &&
> > + mt->target != GL_TEXTURE_1D_ARRAY) {
There is the exact same assertion in tr_mode_vertical_texture_alignment(),
and therefore this makes sense.
> > uint32_t align = tr_mode_vertical_texture_alignment(brw, mt);
> > /* XY_FAST_COPY_BLT doesn't support vertical alignment < 64 */
> > return align < 64 ? 64 : align;
> > --
> > 2.4.3
> >
>
> Patches 2, 4, 5-7 of this series are waiting for review. These patches are
> doing
> simple changes and should be easy to review. Here is a patchwork link to the
> list of patches:
> http://patchwork.freedesktop.org/project/mesa/patches/?submitter=10862
Along with patch 2, number 5 is clear improvment also. The replacament of the
static alignment table with a multiplier (targeting another alignment table)
in patch 4 wasn't at first a clear improvement to me. But combined with
patch 6 I can see them reducing the number of lines of code and therefore I'm
in favor of commiting them as well.
In patch 4 you could call "multiplier" as "multiplier_ys" as it is
specifically used to derive YS alignment from YF alignment.
In patch 6 (or later in patch 7) you could declare the variable "i" as const
and initialize it right away with the correct value.
Using "mt->cpp" instead of "_mesa_get_format_bytes(mt->format) * 8" in patch
7 looks better to me also.
So 2 and 4-7 are:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
