[Mesa-dev] GL_NV_texture_env_combine4, Mesa and TNT2
Ian Romanick
idr at freedesktop.org
Wed Sep 8 12:25:17 PDT 2010
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Francisco Jerez wrote:
> 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.
Patches to core Mesa should go to mesa-dev for review.
>> 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);
I have a pair patches that will fix this. I'm trying to track down an
unrelated regression that occurred last night first.
>> 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?
This looks like spurious punctuation changes that deviate from any
coding standards that I've ever seen in Mesa or DRI drivers.
>>
>> /* 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 }
>> };
At some point it may be worth adding a dri-conf option to enable fake
ARB_texture_env_combine support. There are a few apps that check for
that but not for the EXT. I'd swear that NV drivers used to support the
ARB extension on TNT2 hardware. They did some hack that let them do
some cases of GL_SUBTRACT using the combine4 hardware. I may just be
misremembering, though.
>> 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.
Agreed. The convention in Mesa has been:
- Use the most recent names available for enums.
- Don't change the enums used unless there is good reason.
This particular patch fails both.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkyH4xwACgkQX1gOwKyEAw+JiQCfVAhmsm3Y66GI0lKUrxZlupmh
jL8AnRWL5ysfHtEO30D3GpQ/rq45+5nq
=uIUJ
-----END PGP SIGNATURE-----
More information about the mesa-dev
mailing list