[Mesa-dev] GL_NV_texture_env_combine4, Mesa and TNT2

Francisco Jerez currojerez at riseup.net
Wed Sep 8 07:41:10 PDT 2010


randrianasulu at gmail.com writes:

> Very hackish patch series for nv0x hardware, please take look and comment on 
> changes, blame me for errors etc.
>
> Francisco, might be nouveau list is better for this stuff?

It's fine either way, do it as you prefer.

> From 9bf16a26361ab4a44b6568deae30c523488cde1b Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Mon, 6 Sep 2010 18:17:48 +0400
> Subject: [PATCH 1/6] nv0x: use multitexturing engine for A8 textures, fixes teapot demo broken
>  probably since we started to use _mesa_meta_Bitmap
> 
Can you please split your commit messages? It goes way over the 80
column limit.

> ---
>  src/mesa/drivers/dri/nouveau/nv04_context.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/nouveau/nv04_context.c b/src/mesa/drivers/dri/nouveau/nv04_context.c
> index 6834f7c..af46ce2 100644
> --- a/src/mesa/drivers/dri/nouveau/nv04_context.c
> +++ b/src/mesa/drivers/dri/nouveau/nv04_context.c
> @@ -38,6 +38,7 @@ nv04_context_engine(GLcontext *ctx)
>  	struct nouveau_hw_state *hw = &to_nouveau_context(ctx)->hw;
>  	struct nouveau_grobj *fahrenheit;
>  
> +
This is useless whitespace.

>  	if (ctx->Texture.Unit[0].EnvMode == GL_COMBINE ||
>  	    ctx->Texture.Unit[0].EnvMode == GL_BLEND ||
>  	    ctx->Texture.Unit[0].EnvMode == GL_ADD ||
> @@ -46,6 +47,13 @@ nv04_context_engine(GLcontext *ctx)
>  		fahrenheit = hw->eng3dm;
>  	else
>  		fahrenheit = hw->eng3d;
> +		
Same here.

> +	if (ctx->Texture.Unit[0]._ReallyEnabled) {
> +	struct gl_texture_object *t = ctx->Texture.Unit[0]._Current;
> +	struct gl_texture_image *ti = t->Image[0][t->BaseLevel];
Inconsistent indentation here.

> +		if (ti->_BaseFormat == GL_ALPHA)
> +			fahrenheit = hw->eng3dm;
This is the right thing to do, but we need to check for GL_LUMINANCE
too.

> +	}
>  
>  	if (fahrenheit != nctx->eng3d) {
>  		nctx->eng3d = fahrenheit;
> -- 
> 1.7.0.2

> From 8ca9fd5db720af498fcaf1365db19fbd2be57b57 Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Mon, 6 Sep 2010 18:26:05 +0400
> Subject: [PATCH 2/6] nv0x: add workaround for TNT2 and small sifm copies, shuts up errors but texenv demo still NOT fixed.
> 
> ---
>  src/mesa/drivers/dri/nouveau/nv04_surface.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/nouveau/nv04_surface.c b/src/mesa/drivers/dri/nouveau/nv04_surface.c
> index e3febf7..48078f9 100644
> --- a/src/mesa/drivers/dri/nouveau/nv04_surface.c
> +++ b/src/mesa/drivers/dri/nouveau/nv04_surface.c
> @@ -410,10 +410,19 @@ nv04_surface_copy(GLcontext *ctx,
>  		nv04_surface_copy_m2mf(ctx, dst, src, dx, dy, sx, sy, w, h);
>  		return;
>  	}
> +	
> +	/* TNT2 seems to be upset if we use sifm with small region,
> +	 add workaround */
> +	int sifm_allowed = 0;
> +	if (context_chipset(ctx) < 0x10) {
> +		if (w > 8 && h > 8)
> +			sifm_allowed = 1;
> +	} else { sifm_allowed = 1; }
> +
It's probably not about size, but about alignment, and most likely we
can keep using accelerated copies fixing the provided coordinates.
>  
>  	/* Swizzle using sifm+swzsurf. */
>          if (src->layout == LINEAR && dst->layout == SWIZZLED &&
> -	    dst->cpp != 1 && !(dst->offset & 63)) {
> +	    dst->cpp != 1 && !(dst->offset & 63) &&  (sifm_allowed == 1)) {
>  		nv04_surface_copy_swizzle(ctx, dst, src, dx, dy, sx, sy, w, h);
>  		return;
>  	}
> -- 
> 1.7.0.2
> 

> From c183d547ec41466e902248c856c7291dfac33726 Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Tue, 7 Sep 2010 19:00:31 +0400
> Subject: [PATCH 3/6] nouveau: Trivially move GL_ARB_texture_env_dot3 and GL_ARB_texture_env_combine
>   from common extension list into nv10 and nv20 specific extension lists, so they will not show up on nv0x.
> 
> ---
>  src/mesa/drivers/dri/nouveau/nouveau_context.c |    2 --
>  src/mesa/drivers/dri/nouveau/nv10_context.c    |    2 ++
>  src/mesa/drivers/dri/nouveau/nv20_context.c    |    2 ++
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_context.c b/src/mesa/drivers/dri/nouveau/nouveau_context.c
> index f481161..e9e7bbf 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_context.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_context.c
> @@ -50,8 +50,6 @@
>  static const struct dri_extension nouveau_extensions[] = {
>  	{ "GL_ARB_multitexture",	NULL },
>  	{ "GL_ARB_texture_env_add",	NULL },
> -	{ "GL_ARB_texture_env_combine",	NULL },
> -	{ "GL_ARB_texture_env_dot3",	NULL },
>  	{ "GL_ARB_texture_mirrored_repeat", NULL },
>  	{ "GL_EXT_fog_coord",		GL_EXT_fog_coord_functions },
>  	{ "GL_EXT_framebuffer_blit",	NULL },
> diff --git a/src/mesa/drivers/dri/nouveau/nv10_context.c b/src/mesa/drivers/dri/nouveau/nv10_context.c
> index b6d1036..7f00002 100644
> --- a/src/mesa/drivers/dri/nouveau/nv10_context.c
> +++ b/src/mesa/drivers/dri/nouveau/nv10_context.c
> @@ -34,6 +34,8 @@
>  
>  static const struct dri_extension nv10_extensions[] = {
>  	{ "GL_EXT_texture_rectangle",	NULL },
> +	{ "GL_ARB_texture_env_combine", NULL },
> +	{ "GL_ARB_texture_env_dot3",    NULL },
>  	{ NULL,				NULL }
>  };
>  
> diff --git a/src/mesa/drivers/dri/nouveau/nv20_context.c b/src/mesa/drivers/dri/nouveau/nv20_context.c
> index 789dcaa..03cb14a 100644
> --- a/src/mesa/drivers/dri/nouveau/nv20_context.c
> +++ b/src/mesa/drivers/dri/nouveau/nv20_context.c
> @@ -33,6 +33,8 @@
>  
>  static const struct dri_extension nv20_extensions[] = {
>  	{ "GL_EXT_texture_rectangle",	NULL },
> +	{ "GL_ARB_texture_env_combine", NULL },
> +	{ "GL_ARB_texture_env_dot3",    NULL },
>  	{ NULL,				NULL }
>  };
>  
> -- 
> 1.7.0.2

This one is OK, but please fix the commit message.

> From 3accecebe12d34e74feae8149615c379bea54915 Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Wed, 8 Sep 2010 15:27:24 +0400
> Subject: [PATCH 5/6] mesa: hack support for EXT_texture_env_combine in texenv.c, avoid gl errors if only this extension enabled in  driver
> 
> ---
>  src/mesa/main/texenv.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/texenv.c b/src/mesa/main/texenv.c
> index 4442fb8..d8402da 100644
> --- a/src/mesa/main/texenv.c
> +++ b/src/mesa/main/texenv.c
> @@ -320,7 +320,8 @@ set_combiner_operand(GLcontext *ctx,
>        alpha = GL_FALSE;
>        break;
>     case GL_OPERAND2_RGB:
> -      if (ctx->Extensions.ARB_texture_env_combine) {
> +      if (ctx->Extensions.ARB_texture_env_combine || 
> +    	    ctx->Extensions.EXT_texture_env_combine) {
>           term = 2;
>           alpha = GL_FALSE;
>        }
> @@ -348,7 +349,8 @@ set_combiner_operand(GLcontext *ctx,
>        alpha = GL_TRUE;
>        break;
>     case GL_OPERAND2_ALPHA:
> -      if (ctx->Extensions.ARB_texture_env_combine) {
> +      if (ctx->Extensions.ARB_texture_env_combine || 
> +    	    ctx->Extensions.EXT_texture_env_combine) {
>           term = 2;
>           alpha = GL_TRUE;
>        }
> -- 
> 1.7.0.2
> 

That's incorrect, it makes mesa pass through illegal combinations like:
> glTexEnvi(GL_TEXTURE_ENV, GL_OPERAND2_RGB, GL_ONE_MINUS_SRC_COLOR);

> From 6e3cbb059b96032bd1a800b802e15f0c72e291fd Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Wed, 8 Sep 2010 15:29:11 +0400
> Subject: [PATCH 6/6] nv0x: avoid assertion, just hack, we need properly enable multitexturing engine for advanced  texenv modes and/or emulated textures, may be by dirtying context on texenv changes?
> 
> ---
>  src/mesa/drivers/dri/nouveau/nv04_state_raster.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/nouveau/nv04_state_raster.c b/src/mesa/drivers/dri/nouveau/nv04_state_raster.c
> index c191571..a2544fb 100644
> --- a/src/mesa/drivers/dri/nouveau/nv04_state_raster.c
> +++ b/src/mesa/drivers/dri/nouveau/nv04_state_raster.c
> @@ -307,11 +307,17 @@ nv04_emit_blend(GLcontext *ctx, int emit)
>  		else
>  			blend |= NV04_TEXTURED_TRIANGLE_BLEND_SHADE_MODE_FLAT;
>  
> +
> +		
>  		/* Texture environment. */
> -		if (ctx->Texture._EnabledUnits)
> +		if (ctx->Texture._EnabledUnits) {
> +		    if ((ctx->Texture.Unit[0].EnvMode == GL_REPLACE) ||
> +			ctx->Texture.Unit[0].EnvMode == GL_MODULATE ||
> +			ctx->Texture.Unit[0].EnvMode == GL_DECAL) {
>  			blend |= get_texenv_mode(ctx->Texture.Unit[0].EnvMode);
> -		else
> +		} else { blend |= get_texenv_mode(GL_MODULATE); }
>  			blend |= get_texenv_mode(GL_MODULATE);
> +		}
What exactly are you trying to workaround here?

>  
>  		/* Secondary color */
>  		if (NEED_SECONDARY_COLOR(ctx))
> -- 
> 1.7.0.2

> From 8d361fd58f6c5543f398aeb2a8eaff596bf721d8 Mon Sep 17 00:00:00 2001
> From: Andrew Randrianasulu <randrianasulu at gmail.com>
> Date: Wed, 8 Sep 2010 06:50:04 +0400
> Subject: [PATCH 4/6] nouveau: Add GL_EXT_texture_env_combine and GL_NV_texture_env_combine4 into common extension list, currently fail piglit tests, need more testing!
> 
> ---
>  src/mesa/drivers/dri/nouveau/nouveau_context.c |    2 ++
>  src/mesa/drivers/dri/nouveau/nv04_state_frag.c |   10 +++++-----
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_context.c b/src/mesa/drivers/dri/nouveau/nouveau_context.c
> index e9e7bbf..0b735df 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_context.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_context.c
> @@ -56,8 +56,10 @@ static const struct dri_extension nouveau_extensions[] = {
>  	{ "GL_EXT_framebuffer_object",	GL_EXT_framebuffer_object_functions },
>  	{ "GL_EXT_secondary_color",	GL_EXT_secondary_color_functions },
>  	{ "GL_EXT_stencil_wrap",	NULL },
> +	{ "GL_EXT_texture_env_combine",     NULL },
>  	{ "GL_EXT_texture_lod_bias",	NULL },
>  	{ "GL_NV_blend_square",         NULL },
> +	{ "GL_NV_texture_env_combine4",    NULL },
The second field is aligned inconsistently.

>  	{ "GL_SGIS_generate_mipmap",	NULL },
>  	{ NULL,				NULL }
>  };
> diff --git a/src/mesa/drivers/dri/nouveau/nv04_state_frag.c b/src/mesa/drivers/dri/nouveau/nv04_state_frag.c
> index d7c86d4..d95ac97 100644
> --- a/src/mesa/drivers/dri/nouveau/nv04_state_frag.c
> +++ b/src/mesa/drivers/dri/nouveau/nv04_state_frag.c
> @@ -89,13 +89,13 @@ get_input_source(struct combiner_state *rc, int source)
>  	case GL_TEXTURE1:
>  		return COMBINER_SOURCE(TEXTURE1);
>  
> -	case GL_CONSTANT:
> +	case GL_CONSTANT_EXT:
>  		return COMBINER_SOURCE(CONSTANT);
>  
> -	case GL_PRIMARY_COLOR:
> +	case GL_PRIMARY_COLOR_EXT:
>  		return COMBINER_SOURCE(PRIMARY_COLOR);
>  
> -	case GL_PREVIOUS:
> +	case GL_PREVIOUS_EXT:
>  		return rc->unit ? COMBINER_SOURCE(PREVIOUS) :
>  			COMBINER_SOURCE(PRIMARY_COLOR);
>  
> @@ -202,7 +202,7 @@ setup_combiner(struct combiner_state *rc)
>  		UNSIGNED_OP(rc);
>  		break;
>  
> -	case GL_INTERPOLATE:
> +	case GL_INTERPOLATE_EXT:
>  		INPUT_ARG(rc, 0, 0, 0);
>  		INPUT_ARG(rc, 1, 2, 0);
>  		INPUT_ARG(rc, 2, 1, 0);
> @@ -210,7 +210,7 @@ setup_combiner(struct combiner_state *rc)
>  		UNSIGNED_OP(rc);
>  		break;
>  
> -	case GL_ADD_SIGNED:
> +	case GL_ADD_SIGNED_EXT:
>  		INPUT_ARG(rc, 0, 0, 0);
>  		INPUT_SRC(rc, 1, ZERO, INVERT);
>  		INPUT_ARG(rc, 2, 1, 0);

That's pointless, the _EXT definitions match the core ones, they're just
uglier.

> -- 
> 1.7.0.2
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20100908/2bd999b6/attachment.pgp>


More information about the mesa-dev mailing list