On Mon, 01 Aug 2011 13:15:45 -0700, Kenneth Graunke <kenn...@whitecape.org> 
wrote:
> On 07/31/2011 07:25 PM, Eric Anholt wrote:
> > On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke <kenn...@whitecape.org> 
> > 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 <kenn...@whitecape.org>
> >> ---
> >>  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 :)

Attachment: pgp76RveSgnmE.pgp
Description: PGP signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to