Re: [Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.
On Mon, 01 Aug 2011 13:15:45 -0700, Kenneth Graunke wrote: > On 07/31/2011 07:25 PM, Eric Anholt wrote: > > On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke > > wrote: > >> For power-of-two sizes, h0 == mt->height0 since it's already a multiple > >> of two. However, for NPOT, they're different; h1 should be computed > >> based on the original size. > >> > >> Fixes piglit test "cubemap npot" and oglconform_31 test "textureNPOT". > >> > >> NOTE: This is a candidate for stable release branches. > >> > >> Signed-off-by: Kenneth Graunke > >> --- > >> src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> Note that the piglit test referenced isn't committed yet; I sent it to the > >> piglit mailing list. > >> > >> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c > >> b/src/mesa/drivers/dri/i965/brw_tex_layout.c > >> index f462f32..46a417a 100644 > >> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c > >> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c > >> @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel, > >> * given in Volume 1 of the BSpec. > >> */ > >> h0 = ALIGN(mt->height0, align_h); > >> -h1 = ALIGN(minify(h0), align_h); > >> +h1 = ALIGN(minify(mt->height0), align_h); > >> qpitch = (h0 + h1 + (intel->gen >= 7 ? 12 : 11) * align_h); > >>if (mt->compressed) > >> qpitch /= 4; > > > > > > This looks wrong to me. The height of a level L is ALIGN(height0 >> L, > > j) according to SNB PRM vol1, 6.17.3.1 "Computing MIP level sizes". > > Note that our calculation of j is wrong for a bunch of hardware/format > > combos -- I wonder if that was the issue you were looking at? > > Just for the record, I talked with Eric about this in person, and he > agreed that my patch is correct. mt->height0 is H_0 (actual height) in > the PRM, while h0 is h_0 (aligned). We were using the wrong one. Also, > for the case I was looking at, j == 2, so that wasn't the issue. More to the point, I was reading the patch as exactly reversed of what it did. So, I think that's a Reviewed-by :) pgp76RveSgnmE.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.
On 07/31/2011 07:25 PM, Eric Anholt wrote: > On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke > wrote: >> For power-of-two sizes, h0 == mt->height0 since it's already a multiple >> of two. However, for NPOT, they're different; h1 should be computed >> based on the original size. >> >> Fixes piglit test "cubemap npot" and oglconform_31 test "textureNPOT". >> >> NOTE: This is a candidate for stable release branches. >> >> Signed-off-by: Kenneth Graunke >> --- >> src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> Note that the piglit test referenced isn't committed yet; I sent it to the >> piglit mailing list. >> >> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c >> b/src/mesa/drivers/dri/i965/brw_tex_layout.c >> index f462f32..46a417a 100644 >> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c >> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c >> @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel, >> * given in Volume 1 of the BSpec. >> */ >>h0 = ALIGN(mt->height0, align_h); >> - h1 = ALIGN(minify(h0), align_h); >> + h1 = ALIGN(minify(mt->height0), align_h); >>qpitch = (h0 + h1 + (intel->gen >= 7 ? 12 : 11) * align_h); >>if (mt->compressed) >> qpitch /= 4; > > > This looks wrong to me. The height of a level L is ALIGN(height0 >> L, > j) according to SNB PRM vol1, 6.17.3.1 "Computing MIP level sizes". > Note that our calculation of j is wrong for a bunch of hardware/format > combos -- I wonder if that was the issue you were looking at? Just for the record, I talked with Eric about this in person, and he agreed that my patch is correct. mt->height0 is H_0 (actual height) in the PRM, while h0 is h_0 (aligned). We were using the wrong one. Also, for the case I was looking at, j == 2, so that wasn't the issue. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.
On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke wrote: > For power-of-two sizes, h0 == mt->height0 since it's already a multiple > of two. However, for NPOT, they're different; h1 should be computed > based on the original size. > > Fixes piglit test "cubemap npot" and oglconform_31 test "textureNPOT". > > NOTE: This is a candidate for stable release branches. > > Signed-off-by: Kenneth Graunke > --- > src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > Note that the piglit test referenced isn't committed yet; I sent it to the > piglit mailing list. > > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c > b/src/mesa/drivers/dri/i965/brw_tex_layout.c > index f462f32..46a417a 100644 > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c > @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel, > * given in Volume 1 of the BSpec. > */ > h0 = ALIGN(mt->height0, align_h); > - h1 = ALIGN(minify(h0), align_h); > + h1 = ALIGN(minify(mt->height0), align_h); > qpitch = (h0 + h1 + (intel->gen >= 7 ? 12 : 11) * align_h); >if (mt->compressed) >qpitch /= 4; This looks wrong to me. The height of a level L is ALIGN(height0 >> L, j) according to SNB PRM vol1, 6.17.3.1 "Computing MIP level sizes". Note that our calculation of j is wrong for a bunch of hardware/format combos -- I wonder if that was the issue you were looking at? pgpW1Mo6odvzG.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.
For power-of-two sizes, h0 == mt->height0 since it's already a multiple of two. However, for NPOT, they're different; h1 should be computed based on the original size. Fixes piglit test "cubemap npot" and oglconform_31 test "textureNPOT". NOTE: This is a candidate for stable release branches. Signed-off-by: Kenneth Graunke --- src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Note that the piglit test referenced isn't committed yet; I sent it to the piglit mailing list. diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c index f462f32..46a417a 100644 --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel, * given in Volume 1 of the BSpec. */ h0 = ALIGN(mt->height0, align_h); - h1 = ALIGN(minify(h0), align_h); + h1 = ALIGN(minify(mt->height0), align_h); qpitch = (h0 + h1 + (intel->gen >= 7 ? 12 : 11) * align_h); if (mt->compressed) qpitch /= 4; -- 1.7.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
