On Thu, Jul 17, 2003 at 12:51:09PM -0700, Ian Romanick wrote:
> Ville Syrjälä wrote:
> > mga_texenv.patch:
> > - Fix all texenv modes for all texformats on G400.
> >
> > - Implement GL_DECAL using linear blend mode.
> > The G400 docs aren't very clear on this subject so I'm not entirely sure
> > if it will work correctly with multi texturing. Anyone have a good test?
> >
> > - GL_BLEND suffers from hardware limitations. All three components of Cc
> > must be equal. Additionally GL_INTENSITY requires that Ac is 0.0 or 1.0.
> > If Cc!=0.0 (or Ac!=0.0 for GL_INTENSITY) we need both units to get
> > things done in a single pass.
>
> Here's another thought that I've been toying with. All of the "classic"
> texture environments are a subset of the texture combine environment.
> That is, all of the equations for REPLACE, MODULATE, DECAL, BLEND, and
> ADD can be implemented using COMBINE. Why not implement the driver that
> way? Why not have a generic updateTexEnv routine that has a big switch
> based on the texture environment with inner-switches based on texture
> base format. Each case in the inner-switches would call a device
> specific UpdateTexEnvCombine routine with the right values.
>
> Doing this for chips like the Radeon, R200, and i830 that actually
> support all of ARB_texture_env_combine would be easy. Doing it for
> chips like the Rage128 and G400 would be more difficult. There would be
> a lot of software fallbacks, but those drivers have software fallbacks
> for the "classic" texture environments as it is.
I took a brief look at this stuff for G400 but came to the conclusion that
pretty much anything beyond the normal texenv modes would be a fallback.
So I doubt there's much point in doing this.
> > - Passes glean's texenv test (32bpp and 16bpp). And without fallbacks if I
> > change the envcolor to comply with the aforementioned restrictions. Also
> > Quake3 looks ok with the patch ;)
>
> Quake3 intentionally uses a very limited set of texture environments.
> This was done to work around driver / hardware limitations at the time
> it was released. Anymore, I don't think that makes it a very good test.
> A better test would be RtCW: Enemy Territory. The whole game is free
> (as in beer), and it builds on the RtCW engine (which builds on the
> Quake3 engine).
Thanks. I'll have to try that one too.
> > - I also removed the useless MGA_FALLBACK_BORDER_MODE thing. It didn't
> > print anything since the debug print stopped at the first enabled
> > fallback bit which was the texture fallback.
>
> That doesn't seem like it should be true. When FALLBACK is called in
> update_tex_common (mga_texstate.c, line 686) there shouldn't be any bits
> already set. Try modifing tests/texwrap.c to always set TEXTURE_WRAP_T
> to CLAMP_TO_EDGE. You will get the "Mixing GL_CLAMP_TO_EDGE and
> GL_CLAMP" fallback message. :)
I'll have to try this again. Certainly the code suggests that it should
work...
>
> > ------------------------------------------------------------------------
> >
> > diff -urN mga.orig/mgastate.c mga/mgastate.c
> > --- mga.orig/mgastate.c 2003-07-12 22:40:08.000000000 +0300
> > +++ mga/mgastate.c 2003-07-12 22:39:47.000000000 +0300
> > @@ -123,9 +123,11 @@
> > /* BlendEquation sets ColorLogicOpEnabled in an unexpected
> > * manner.
> > */
> > +#if !defined(ACCEL_ROP)
> > FALLBACK( ctx, MGA_FALLBACK_LOGICOP,
> > (ctx->Color.ColorLogicOpEnabled &&
> > ctx->Color.LogicOp != GL_COPY));
> > +#endif
> > }
> >
> > static void mgaDDBlendFunc(GLcontext *ctx, GLenum sfactor, GLenum dfactor)
> > @@ -900,9 +902,11 @@
> >
> > /* For some reason enable(GL_BLEND) affects ColorLogicOpEnabled.
> > */
> > +#if !defined(ACCEL_ROP)
> > FALLBACK( ctx, MGA_FALLBACK_LOGICOP,
> > (ctx->Color.ColorLogicOpEnabled &&
> > ctx->Color.LogicOp != GL_COPY));
> > +#endif
> > break;
> > case GL_DEPTH_TEST:
> > FLUSH_BATCH( mmesa );
>
> I'm not really familiar with the blend code for the MGA. Have you
> tested these changes? Clearly with ACCEL_ROP undefined they don't make
> any difference. Has any of this code ever been tested with ACCEL_ROP
> defined? What were the issues?
I have no idea. I'm running with ACCEL_ROP defined. Certainly glean
doesn't pass the logicOp test but then again I don't think it passes it
with ACCEL_ROP undefined either (as strange as it sounds). I'll have to do
some more testing...
>
> > ------------------------------------------------------------------------
> >
> > diff -urN mga.orig/mga_texstate.c mga/mga_texstate.c
> > --- mga.orig/mga_texstate.c 2003-07-12 22:40:08.000000000 +0300
> > +++ mga/mga_texstate.c 2003-07-12 22:42:30.000000000 +0300
> > @@ -106,14 +106,17 @@
> > * GL_TEXTURE_MAX_LOD, GL_TEXTURE_BASE_LEVEL, and GL_TEXTURE_MAX_LEVEL.
> > * Yes, this looks overly complicated, but it's all needed.
> > */
> > -
> > - firstLevel = tObj->BaseLevel + (GLint)(tObj->MinLod + 0.5);
> > - firstLevel = MAX2(firstLevel, tObj->BaseLevel);
> > - lastLevel = tObj->BaseLevel + (GLint)(tObj->MaxLod + 0.5);
> > - lastLevel = MAX2(lastLevel, tObj->BaseLevel);
> > - lastLevel = MIN2(lastLevel, tObj->BaseLevel + baseImage->MaxLog2);
> > - lastLevel = MIN2(lastLevel, tObj->MaxLevel);
> > - lastLevel = MAX2(firstLevel, lastLevel); /* need at least one level */
> > + if (tObj->MinFilter == GL_NEAREST || tObj->MinFilter == GL_LINEAR) {
> > + firstLevel = lastLevel = tObj->BaseLevel;
> > + } else {
> > + firstLevel = tObj->BaseLevel + (GLint)(tObj->MinLod + 0.5);
> > + firstLevel = MAX2(firstLevel, tObj->BaseLevel);
> > + lastLevel = tObj->BaseLevel + (GLint)(tObj->MaxLod + 0.5);
> > + lastLevel = MAX2(lastLevel, tObj->BaseLevel);
> > + lastLevel = MIN2(lastLevel, tObj->BaseLevel + baseImage->MaxLog2);
> > + lastLevel = MIN2(lastLevel, tObj->MaxLevel);
> > + lastLevel = MAX2(firstLevel, lastLevel); /* need at least one level */
> > + }
> > log2Width = tObj->Image[firstLevel]->WidthLog2;
> > log2Height = tObj->Image[firstLevel]->HeightLog2;
> > width = tObj->Image[firstLevel]->Width;
>
> This is not correct. Both the OpenGL spec (page 142 (155 in the pdf) of
> the 1.4 spec) and the SGIS_texture_lod extension
> (http://oss.sgi.com/projects/ogl-sample/registry/SGIS/texture_lod.txt)
> are quite clear on this. The value of TEXTURE_BASE_LEVEL, which
> defaults to zero, is the texture level used by the NEAREST and LINEAR
> filters.
Isn't that exactly what my patch does?
> > diff -urN mga.orig/mgatex.c mga/mgatex.c
> > --- mga.orig/mgatex.c 2003-07-12 22:40:08.000000000 +0300
> > +++ mga/mgatex.c 2003-07-12 22:42:45.000000000 +0300
> > @@ -449,6 +445,7 @@
> >
> > switch (pname) {
> > case GL_TEXTURE_MIN_FILTER:
> > + driSwapOutTextureObject( (driTextureObject *) t );
> > case GL_TEXTURE_MAG_FILTER:
> > FLUSH_BATCH(mmesa);
> > mgaSetTexFilter( t, tObj->MinFilter, tObj->MagFilter );
> >
>
> Why is this needed?
Because of the previous patch. When changing between LINEAR/NEAREST and
mipmapping firstLevel needs to be recalculated.
> > ------------------------------------------------------------------------
> >
> > diff -urN mga.orig/mga_texstate.c mga/mga_texstate.c
> > --- mga.orig/mga_texstate.c 2003-07-12 22:44:37.000000000 +0300
> > +++ mga/mga_texstate.c 2003-07-12 22:46:49.000000000 +0300
> > @@ -134,7 +134,7 @@
> >
> > totalSize = 0;
> > for ( i = 0 ; i < numLevels ; i++ ) {
> > - const struct gl_texture_image * const texImage = tObj->Image[i];
> > + const struct gl_texture_image * const texImage = tObj->Image[i+firstLevel];
>
> This is correct. Good catch.
>
> As for the rest of this patch, I think there is some confusion.
> Hopefully I can clear it up. The intention of the firstLevel and
> lastLevel values in classes (in a very loose, C-instead-of-C++ sense)
> derrived from driTextureObject is for them to refer to images in the
> Mesa controlled texture state. That is, firstLevel is the image in
> tObj->Image that corresponds to the base level (as set by
> TEXTURE_BASE_LEVEL) of the current texture environment. The idea is
> that this image is set to the level 0 image in the hardware.
But that isn't true for G200/G400 since the hw doesn't support LOD
clamping.
> As a result of that usage, the bits in the dirty_images array correspond
> to the hardware levels. That is 't->dirty_images[0] |= (1U << 0)' says
> that the base level needs to be uploaded. So the image at
> t->tObj->Images[ t->firstLevel ] gets uploaded to hardware texture lavel
> 0. This is the way most if not all of the other drivers that use the
> texmem interface operate.
>
> The one change that I would like to see is for firstLevel & lastLevel to
> be moved to driTextureObject. This change may even allow some
> additional code to be moved out of the individual drivers and into the
> common directory.
>
> > if ( (texImage == NULL)
> > || ((i != 0)
> > @@ -143,10 +143,10 @@
> > }
> >
> > t->offsets[i] = totalSize;
> > - t->base.dirty_images[0] |= (1<<i);
> > + t->base.dirty_images[0] |= (1<<(i+firstLevel));
> >
> > - totalSize += ((MAX2( texImage->Width, 8 ) *
> > - MAX2( texImage->Height, 8 ) *
> > + totalSize += ((texImage->Width *
> > + texImage->Height *
> > baseImage->TexFormat->TexelBytes) + 31) & ~31;
> > }
> >
> > @@ -171,7 +171,7 @@
> > */
> >
> > t->setup.texctl |= TMC_tpitchlin_enable;
> > - t->setup.texctl |= (baseImage->Width & (2048 - 1)) << TMC_tpitchext_SHIFT;
> > + t->setup.texctl |= (width & (2048 - 1)) << TMC_tpitchext_SHIFT;
> >
> >
> > /* G400 specifies the number of mip levels in a strange way. Since there
> > diff -urN mga.orig/mgatexmem.c mga/mgatexmem.c
> > --- mga.orig/mgatexmem.c 2003-07-12 22:44:37.000000000 +0300
> > +++ mga/mgatexmem.c 2003-07-12 22:47:41.000000000 +0300
> > @@ -89,13 +89,13 @@
> > * been hardware accelerated.
> > */
> > static void mgaUploadSubImage( mgaContextPtr mmesa,
> > - mgaTextureObjectPtr t, GLint hwlevel )
> > + mgaTextureObjectPtr t, GLint level )
> > {
> > struct gl_texture_image * texImage;
> > unsigned offset;
> > unsigned texelBytes;
> > unsigned length;
> > - const int level = hwlevel + t->firstLevel;
> > + const int hwlevel = level - t->firstLevel;
> >
> >
> > if ( (hwlevel < 0)
> > @@ -265,7 +265,7 @@
> > fprintf(stderr, "[%s:%d] dirty_images[0] = 0x%04x\n",
> > __FILE__, __LINE__, t->base.dirty_images[0] );
> >
> > - for (i = 0 ; i <= t->lastLevel ; i++) {
> > + for (i = t->firstLevel ; i <= t->lastLevel ; i++) {
> > if ( (t->base.dirty_images[0] & (1U << i)) != 0 ) {
> > mgaUploadSubImage( mmesa, t, i );
> > }
> >
> >
> > ------------------------------------------------------------------------
>
> There's a lot going on in this patch (which isn't 100% clear from just
> looking at the diff). I'm going to have to take a deeper look at it a
> little later.
Fine.
> > diff -urN mga.orig/mga_texstate.c mga/mga_texstate.c
> > --- mga.orig/mga_texstate.c 2003-07-12 22:48:26.000000000 +0300
> > +++ mga/mga_texstate.c 2003-07-12 22:51:08.000000000 +0300
> > @@ -52,6 +52,10 @@
> > [MESA_FORMAT_RGB565] = TMC_tformat_tw16 | TMC_takey_1 | TMC_tamask_0,
> > [MESA_FORMAT_ARGB4444] = TMC_tformat_tw12 | TMC_takey_1 | TMC_tamask_0,
> > [MESA_FORMAT_ARGB1555] = TMC_tformat_tw15 | TMC_takey_1 | TMC_tamask_0,
> > + [MESA_FORMAT_AL88] = TMC_tformat_tw8al | TMC_takey_1 | TMC_tamask_0,
> > + [MESA_FORMAT_A8] = TMC_tformat_tw8a | TMC_takey_1 | TMC_tamask_0,
> > + [MESA_FORMAT_L8] = TMC_tformat_tw8a | TMC_takey_1 | TMC_tamask_0,
> > + [MESA_FORMAT_I8] = TMC_tformat_tw8a | TMC_takey_1 | TMC_tamask_0,
> > [MESA_FORMAT_CI8] = TMC_tformat_tw8 | TMC_takey_1 | TMC_tamask_0,
> > [MESA_FORMAT_YCBCR] = TMC_tformat_tw422uyvy | TMC_takey_1 | TMC_tamask_0,
> > [MESA_FORMAT_YCBCR_REV] = TMC_tformat_tw422 | TMC_takey_1 | TMC_tamask_0,
> > @@ -81,6 +85,10 @@
> > case MESA_FORMAT_RGB565: txformat = TMC_tformat_tw16; break;
> > case MESA_FORMAT_ARGB4444: txformat = TMC_tformat_tw12; break;
> > case MESA_FORMAT_ARGB1555: txformat = TMC_tformat_tw15; break;
> > + case MESA_FORMAT_AL88: txformat = TMC_tformat_tw8al; break;
> > + case MESA_FORMAT_A8: txformat = TMC_tformat_tw8a; break;
> > + case MESA_FORMAT_L8: txformat = TMC_tformat_tw8a; break;
> > + case MESA_FORMAT_I8: txformat = TMC_tformat_tw8a; break;
> > case MESA_FORMAT_CI8: txformat = TMC_tformat_tw8; break;
> > case MESA_FORMAT_YCBCR: txformat = TMC_tformat_tw422uyvy; break;
> > case MESA_FORMAT_YCBCR_REV: txformat = TMC_tformat_tw422; break;
> > diff -urN mga.orig/mgatex.c mga/mgatex.c
> > --- mga.orig/mgatex.c 2003-07-12 22:48:26.000000000 +0300
> > +++ mga/mgatex.c 2003-07-12 22:50:11.000000000 +0300
> > @@ -219,7 +219,7 @@
> > case GL_ALPHA16:
> > case GL_COMPRESSED_ALPHA:
> > /* FIXME: This will report incorrect component sizes... */
> > - return &_mesa_texformat_argb4444;
> > + return MGA_IS_G400(mmesa) ? &_mesa_texformat_a8 : &_mesa_texformat_argb4444;
> >
> > case 1:
> > case GL_LUMINANCE:
> > @@ -229,7 +229,7 @@
> > case GL_LUMINANCE16:
> > case GL_COMPRESSED_LUMINANCE:
> > /* FIXME: This will report incorrect component sizes... */
> > - return &_mesa_texformat_rgb565;
> > + return MGA_IS_G400(mmesa) ? &_mesa_texformat_l8 : &_mesa_texformat_rgb565;
> >
> > case 2:
> > case GL_LUMINANCE_ALPHA:
> > @@ -241,7 +241,7 @@
> > case GL_LUMINANCE16_ALPHA16:
> > case GL_COMPRESSED_LUMINANCE_ALPHA:
> > /* FIXME: This will report incorrect component sizes... */
> > - return &_mesa_texformat_argb4444;
> > + return MGA_IS_G400(mmesa) ? &_mesa_texformat_al88 :
> > &_mesa_texformat_argb4444;
> >
> > case GL_INTENSITY:
> > case GL_INTENSITY4:
> > @@ -250,11 +250,12 @@
> > case GL_INTENSITY16:
> > case GL_COMPRESSED_INTENSITY:
> > /* FIXME: This will report incorrect component sizes... */
> > - return &_mesa_texformat_argb4444;
> > + return MGA_IS_G400(mmesa) ? &_mesa_texformat_i8 : &_mesa_texformat_argb4444;
> >
> > case GL_YCBCR_MESA:
> > - if (type == GL_UNSIGNED_SHORT_8_8_APPLE ||
> > - type == GL_UNSIGNED_BYTE)
> > + if (MGA_IS_G400(mmesa) &&
> > + (type == GL_UNSIGNED_SHORT_8_8_APPLE ||
> > + type == GL_UNSIGNED_BYTE))
> > return &_mesa_texformat_ycbcr;
> > else
> > return &_mesa_texformat_ycbcr_rev;
>
> I was afraid of something like this. Have you tested all of these
> additional modes to make sure they work right?
glean -t texEnv passes everything. Also Mesa's texenv demo looks fine. Any
other good tests I should try?
This stuff really needs the texenv patch to make things right. I think the
previous texenv stuff wasn't that careful in selecting diffuse vs.
modulated alpha for example.
> Also,
> "GL_MESA_ycbcr_texture" should be moved from card_extensions to
> g400_extensions in mga_xmesa.c
So it should be disabled on G200 because it can only support one of the
types?
> > ------------------------------------------------------------------------
> >
> > diff -urN mga.orig/mgatex.c mga/mgatex.c
> > --- mga.orig/mgatex.c 2003-07-12 22:40:08.000000000 +0300
> > +++ mga/mgatex.c 2003-07-12 22:41:03.000000000 +0300
> > @@ -47,7 +47,6 @@
> >
> > /**
> > * Set the texture wrap modes.
> > - * Currently, only \c GL_REPEAT and \c GL_CLAMP are supported.
> > *
> > * \param t Texture object whose wrap modes are to be set
> > * \param swrap Wrap mode for the \a s texture coordinate
> > @@ -74,9 +73,6 @@
> > t->setup.texctl |= TMC_clampu_enable;
> > is_clamp_to_edge = GL_TRUE;
> > break;
> > - case GL_CLAMP_TO_BORDER:
> > - t->setup.texctl |= TMC_clampu_enable;
> > - break;
> > default:
> > _mesa_problem(NULL, "bad S wrap mode in %s", __FUNCTION__);
> > }
>
> This patch is fine, but I would prefer it if the header comment were
> updated to be correct rather than just be removed.
Ok.
--
Ville Syrjälä
[EMAIL PROTECTED]
http://www.sci.fi/~syrjala/
-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the
same time. Free trial click here: http://www.vmware.com/wl/offer/345/0
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
