[Mesa-dev] [PATCH 2/2] intel: make sure hardware choose the right filter mode

[Yuanhan Liu]

On Fri, Nov 11, 2011 at 08:53:13AM -0800, Eric Anholt wrote:
> On Thu, 3 Nov 2011 10:16:06 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> > On Wed, Nov 02, 2011 at 02:18:46PM -0700, Eric Anholt wrote:
> > > On Wed, 2 Nov 2011 11:12:07 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> > > > On Tue, Nov 01, 2011 at 05:57:36PM +0800, Yuanhan Liu wrote:
> > > > > According to bspec, MIPCnt(was set to intelObj->_MaxLevel) was used for
> > > > > min/mag filter mode determination. For a normal case with no mipmap like
> > > > > this:
> > > > > 
> > > > >   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> > > > >   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> > > > > 
> > > > > hardware would always choose mag filter(GL_LINEAR) here since MIPCnt was
> > > > > set to 0 zero, then would make LOD be zero. Then according the formula:
> > > > >   MagMode = (LOD - Base <= 0)
> > > > 
> > > > Here are some more comments about this: LOD is a pre-computed value. if
> > > > preClamp is enabled, then LOD would be:
> > > >   LOD = min(min(MIPCnt, MAX_LOD), LOD);
> > > > 
> > > > So, if MIPCnt was set to 0, and preClamp was enabled, then the _final_
> > > > LOD would be 0, and hardware choose mag filter.
> > > > 
> > > > Thoughts?
> > > 
> > > It took me a long time to understand what you were trying to fix here.
> > 
> > Sorry for that :(
> > 
> > > The story I've worked out is:
> > > 
> > > "We can't optimize out uploading the other mipmap levels for
> > > non-mipmapped filtering modes if the min filter is not the same as the
> > > mag filter.  This is because the min/mag decision is based on the
> > > computed LOD of the sample, and if we clamp the LOD to BaseLevel by not
> > > including the other levels, it will always choose the mag filter even if
> > > minification should have occurred. 
> > 
> > Yes.
> > 
> > > See page FINISHME: citation of PRM."
> > 
> > Sorry, what does this mean?
> 
> If you're going to make a reference to the spec, if possible please
> quote the exact section from the public PRM, so the next person to find
> it has a nice reference to read up on.
> 
> > Yeah, I saw that. While thinking about this issue, I thought another
> > patch to fix this issue. It's somehow against with the Bspec: preclamp
> > should be enabled for OpenGL driver.
> > 
> > Anyway, here is the patch, and I'd like to know what's your thoughts.
> 
> I think I liked the previous one more.  While I couldn't come up with a
> testcase for how this one would break, 

Here I attached a simple test case I wrote. The result is somehow
obvious: run it with i965 driver, you will find it use MAG
filter(linear), and run it with swrast driver, you will find it use MIN
filter(nearest, and that's what we want).


> I still think just disabling the
> minor optimization in this weird case is more obviously correct than
> this change.

It's OK to me.

Thanks,
Yuanhan Liu
-------------- next part --------------
#include <GL/glut.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


#define WIN_WIDTH	100
#define WIN_HEIGHT	100
#define TEX_WIDTH	256
#define TEX_HEIGHT	256

void Initial(void)
{
    glClearColor(0.0,0.0,0.0,1.0);
}


static GLfloat * make_tex_data(void)
{
	int i, j;
	int index;
	float color_table[][4] = {
		0, 0, 0, 1,
		1, 1, 1, 1,
	};
	float *tex_data = malloc(TEX_WIDTH * TEX_HEIGHT * 4 * sizeof(GLfloat));
	float *p = tex_data;

	for (i = 0; i < TEX_HEIGHT; i++) {
		index = i % 2;
		for (j = 0; j < TEX_WIDTH; j++) {
				p[0] = color_table[index][0];
				p[1] = color_table[index][1];
				p[2] = color_table[index][2];
				p[3] = color_table[index][3];

				p += 4;
				index = !index;
		}
	}

	if (0) {
		glDrawPixels(TEX_WIDTH, TEX_HEIGHT, GL_RGBA, GL_FLOAT, tex_data);
		glutSwapBuffers();
		sleep(3);
	}

	return tex_data;
}

void myDisplay(void)
{							
	/* Texture stuff */
	GLfloat *tex_data = make_tex_data();

	glEnable(GL_TEXTURE_2D);
	glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, TEX_WIDTH, TEX_HEIGHT, 0, GL_RGBA, GL_FLOAT, tex_data);
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

	glClear(GL_COLOR_BUFFER_BIT);

	glBegin(GL_QUADS);
		glTexCoord2f(0.0f, 0.0f);
		glVertex2f(-1, -1);
		glTexCoord2f(1.0f, 0.0f);
		glVertex2f(1, -1);
		glTexCoord2f(1.0f, 1.0f);
		glVertex2f(1, 1);
		glTexCoord2f(0.0f, 1.0f);
		glVertex2f(-1, 1);
	glEnd();

	glutSwapBuffers();
}

int main(int argc,char** argv)
{
	glutInit(&argc,argv);
	glutInitDisplayMode(GLUT_DOUBLE|GLUT_RGB);
	glutInitWindowSize(WIN_WIDTH, WIN_HEIGHT);
	glutCreateWindow("tex-near-min");
	glutDisplayFunc(myDisplay);
	Initial();
	glutMainLoop();
	return 0;
}