Bug 21691

Summary:	i945 cubemap texture layout code is buggy
Product:	Mesa	Reporter:	Steinar H. Gunderson <sgunderson>
Component:	Drivers/DRI/i915	Assignee:	Eric Anholt <eric>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Patch that fixes the issue Reference rendering of test case i945 rendering of test case before patch i945 rendering of test case after patch Test case source code

Description Steinar H. Gunderson 2009-05-11 14:40:34 UTC

Created attachment 25752 [details] [review]
Patch that fixes the issue

The i945 cubemap layout code is buggy for the lower mip levels; it puts the 4x4 and 2x2 mipmaps on top of each other due to a missing break, and the 2x2 mipmap levels are in the wrong order (+X -X +Y -Y +Z -Z instead of +X +Y +Z -X -Y -Z). The attached patch fixes both issues. It also fixes an inconsistency in the comment -- both the code, the figure and the rest of the comment assumes 8x8 alignment, not 4x4 as the comment says.

I've attached a small conformance test that makes a 64x64 cubemap and draws all faces and mip levels (with reference colors on the left, textured rendering on the right). reference-cubemap.png is how it's rendered on all ATI and nVidia cards I've been able to find. i945-cubemap-pre-fix.png is how it looks in Mesa 7.4.1, i945-cubemap-post-fix.png is how it looks after the attached patch.

As you can see, it's still not right on my i945, but that's a separate bug (filing shortly).

Comment 1 Steinar H. Gunderson 2009-05-11 14:41:11 UTC

Created attachment 25753 [details]
Reference rendering of test case

Comment 2 Steinar H. Gunderson 2009-05-11 14:41:42 UTC

Created attachment 25754 [details]
i945 rendering of test case before patch

Comment 3 Steinar H. Gunderson 2009-05-11 14:42:04 UTC

Created attachment 25755 [details]
i945 rendering of test case after patch

Comment 4 Steinar H. Gunderson 2009-05-11 14:42:22 UTC

Created attachment 25756 [details]
Test case source code

Comment 5 Eric Anholt 2009-05-12 11:34:31 UTC

This definitely fixes a couple of failures in the piglit cubemap test, but also changes a bunch of wrong results to other wrong results.  I think it's going to need a little more looking into.

Comment 6 Steinar H. Gunderson 2009-05-12 15:16:49 UTC

Well, the(In reply to comment #5)
> This definitely fixes a couple of failures in the piglit cubemap test, but also
> changes a bunch of wrong results to other wrong results.  I think it's going to
> need a little more looking into.

Well, I guess #21692 is a good candidate to fix first :-) Optionally, I guess someone with access to the docs should look into whether the alignment really is 8x8, or 4x4 as half the comment says.

Comment 7 Eric Anholt 2009-05-15 12:52:12 UTC

OK, pushed your fix, since everything in it looked correct.

commit b197a8ade3e1e6c67743111f12f27e0a4a985cd9
Author: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date:   Tue May 12 11:32:03 2009 -0700

    i915: Fix 945 cube map layout for the small mipmaps along the bottom.
    
    Bug #21691.

Note that the 2x2 handling still appears to be wrong -- when the base level is 2x2, there aren't the 2 4x4s at the start of the row, so the offset by 16 is wrong.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.