[Mesa-dev] GL_NV_texture_env_combine4, Mesa and TNT2
randrianasulu at gmail.com
randrianasulu at gmail.com
Wed Sep 8 16:06:34 PDT 2010
sorry, i come to not directly unrelated question:
looking at commit "mesa: initial changes for GL_NV_texture_env_combine4"
http://lists.freedesktop.org/archives/mesa-commit/2009-January/006090.html
i see struct gl_tex_env_combine_state , with some arrays holding texenv
state/parameters.
in nv04_state_frag.c nv0x DRI driver pulls in info from this struct,
but ....not into arrays!
----------------
/* Initialize a combiner_state struct from the texture unit
* context. */
#define INIT_COMBINER(chan, ctx, rc, i) do { \
struct gl_tex_env_combine_state *c = \
ctx->Texture.Unit[i]._CurrentCombine; \
(rc)->ctx = ctx; \
(rc)->unit = i; \
(rc)->alpha = __INIT_COMBINER_ALPHA_##chan; \
(rc)->mode = c->Mode##chan; \
(rc)->source = c->Source##chan; \
(rc)->operand = c->Operand##chan; \
(rc)->logscale = c->ScaleShift##chan; \
(rc)->hw = 0; \
} while (0)
------
I fear doing it in this way will lead to losing state. I'm wrong ? I'm talking
about GL_NV_texture_env_combine4 case!
Sorry, Francisco, i really missed something very important here, excuse me for
moving this off IRC.
В сообщении от Wednesday 08 September 2010 23:25:17 Ian Romanick написал(а):
> 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.
More information about the mesa-dev
mailing list