[Mesa-dev] GL_NV_texture_env_combine4, Mesa and TNT2
randrianasulu at gmail.com
randrianasulu at gmail.com
Wed Sep 8 10:53:19 PDT 2010
В сообщении от Wednesday 08 September 2010 18:41:10 Francisco Jerez
написал(а):
> 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.
Will rebase with edited message.
>
> > ---
> > 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.
OK
>
> > + 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.
Already in slightly update patch series, patch 7, i'll rebase/redo whole
series based on your comments.
>
> > + }
> >
> > 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);
> >
Yea, i misread spec, hopefully replacing
ctx->Extensions.EXT_texture_env_combine with
ctx->Extensions.NV_texture_env_combine4 in patch above will work OK?
> > 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?
I dropped this part, just enforce eng3dm under more correct conditions on
context, fixes assert in get_texenv_mode, same patch 7 in newer series.
>
> > /* 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.
Yes, realized this too! This part dropped/reverted.
Thanks _a lot_ for review!
>
> > --
> > 1.7.0.2
More information about the mesa-dev
mailing list