James C Georgas wrote:
On Sun, 2007-12-08 at 19:09 +0200, Roland Scheidegger wrote:

Yes, though it still requires user interaction to switch the
behaviour - and few people actually seem to know about driconf,
distros don't install it by default etc :-(. I don't think there
were really any arguments against it, probably just noone bothered
to actually implement it (which should be fairly trivial). As for
the hw which can't do GL_CLAMP, I'm actually not sure if an option
is really needed. Both possible behaviours (unless you'd want to implement correct behaviour somehow) look equally wrong to me, and
just because a conformance test thinks one is more acceptable than
the other doesn't really mean anything as both are violating the
spec (and thus the conformance test should probably really say both
aren't right). So why don't just change it to the behaviour which
fixes some old broken apps.

Roland

Assuming that you're right about both mappings being equally wrong,
I'd say that argues in favour of a driconf option instead of picking
one and hardcoding it.

I'd want the implementation to be written to spec, regardless of what
it breaks, and then if someone wants behaviour that violates the
spec, they would have to explicitly request it.

I suppose a driconf option with three choices:

- spec - map to GL_CLAMP_TO_EDGE - map to GL_CLAMP_TO_BORDER

would do the job.

I suppose that for hardware that doesn't implement GL_CLAMP, that
would mean coding a slow software fallback that renders to spec.
Certainly for hw which does implement GL_CLAMP, we're not going to break
that (by default). And I doubt someone is going to "fix" the drivers which don't do clamp properly, as for most uses swrast fallbacks are essentially considered useless (though on reasonably new hw it would be possible to do fixups in the pixel shader - but it's complicated, ugly, and drivers for other OS don't do it so you can't expect anyone to write code for that (unless you got some spare time?). So the suggestion to just use clamp_to_edge instead of clamp is simply because I'm not sure if apps exist which rely on gl_clamp which would work correctly if it's forced to clamp_to_border but not clamp_to_edge, so why not just set it to clamp_to_edge to get those broken apps fixed for free?

As an aside, I'm looking at the 1.4 programming guide (4th ed.), on
page 418, section "Repeating and Clamping Textures", where it
describes the tiling effects. I read it like this:

GL_NEAREST ==> GL_CLAMP = GL_CLAMP_TO_EDGE

GL_LINEAR ==> GL_CLAMP = GL_CLAMP_TO_BORDER

Did I interpret it right?

Oh yeah, I actually tried to implement a driconf option for this
once, but I couldn't get enough of a grip on the code organization to
figure out where to make the change. I suppose I could give it
another shot. Can someone point me to the right place in the code?
You're right for clamp being clamp_to_edge for nearest filtering, but it's not the same for linear, as clamp blends the border color with the texel edge color, whereas clamp_to_border does not.

Maybe something like the attached (completely untested) patch should work? It looks a bit unclean, as I pushed the fixup code into core mesa, but it essentially really is a driver-independant option, so otherwise you just need to do the same thing for all drivers which want to support it (doing it in mesa means that an app querying the state of the wrap mode will always get back clamp_to_edge instead of clamp, but I highly doubt these broken apps are going to care...).

Roland
diff --git a/src/mesa/drivers/dri/common/xmlpool/t_options.h 
b/src/mesa/drivers/dri/common/xmlpool/t_options.h
index 4df1916..24fdfc8 100644
--- a/src/mesa/drivers/dri/common/xmlpool/t_options.h
+++ b/src/mesa/drivers/dri/common/xmlpool/t_options.h
@@ -102,6 +102,11 @@ DRI_CONF_OPT_BEGIN(force_s3tc_enable,bool,def) \
         DRI_CONF_DESC(en,gettext("Enable S3TC texture compression even if 
software support is not available")) \
 DRI_CONF_OPT_END
 
+#define DRI_CONF_FORCE_CLAMPTOEDGE(def) \
+DRI_CONF_OPT_BEGIN(force_clamptoedge,bool,def) \
+        DRI_CONF_DESC(en,gettext("Use GL_CLAMP_TO_EDGE instead of GL_CLAMP to 
fix broken apps")) \
+DRI_CONF_OPT_END
+
 #define DRI_CONF_COLOR_REDUCTION_ROUND 0
 #define DRI_CONF_COLOR_REDUCTION_DITHER 1
 #define DRI_CONF_COLOR_REDUCTION(def) \
diff --git a/src/mesa/drivers/dri/i915tex/intel_screen.c 
b/src/mesa/drivers/dri/i915tex/intel_screen.c
diff --git a/src/mesa/drivers/dri/r200/r200_context.c 
b/src/mesa/drivers/dri/r200/r200_context.c
index 5a17844..420ff81 100644
--- a/src/mesa/drivers/dri/r200/r200_context.c
+++ b/src/mesa/drivers/dri/r200/r200_context.c
@@ -382,6 +382,7 @@ GLboolean r200CreateContext( const __GLcontextModes 
*glVisual,
                                 i );
 
    ctx->Const.MaxTextureMaxAnisotropy = 16.0;
+   ctx->Mesa_fixup_clamp = driQueryOptionb(&rmesa->optionCache, 
"force_clamptoedge");
 
    /* No wide AA points.
     */
diff --git a/src/mesa/drivers/dri/r300/r300_context.c 
b/src/mesa/drivers/dri/r300/r300_context.c
index 14e0f05..4591c76 100644
--- a/src/mesa/drivers/dri/r300/r300_context.c
+++ b/src/mesa/drivers/dri/r300/r300_context.c
@@ -273,6 +273,7 @@ GLboolean r300CreateContext(const __GLcontextModes * 
glVisual,
            MIN2(ctx->Const.MaxTextureImageUnits,
                 ctx->Const.MaxTextureCoordUnits);
        ctx->Const.MaxTextureMaxAnisotropy = 16.0;
+       ctx->Mesa_fixup_clamp = driQueryOptionb(&r300->radeon.optionCache, 
"force_clamptoedge");
 
        ctx->Const.MinPointSize = 1.0;
        ctx->Const.MinPointSizeAA = 1.0;
diff --git a/src/mesa/drivers/dri/radeon/radeon_context.c 
b/src/mesa/drivers/dri/radeon/radeon_context.c
index 9451ec4..429ae89 100644
--- a/src/mesa/drivers/dri/radeon/radeon_context.c
+++ b/src/mesa/drivers/dri/radeon/radeon_context.c
@@ -334,6 +334,7 @@ radeonCreateContext( const __GLcontextModes *glVisual,
 
 
    ctx->Const.MaxTextureMaxAnisotropy = 16.0;
+   ctx->Mesa_fixup_clamp = driQueryOptionb(&rmesa->optionCache, 
"force_clamptoedge");
 
    /* No wide points.
     */
diff --git a/src/mesa/drivers/dri/radeon/radeon_screen.c 
b/src/mesa/drivers/dri/radeon/radeon_screen.c
index aa7fb63..057262a 100644
--- a/src/mesa/drivers/dri/radeon/radeon_screen.c
+++ b/src/mesa/drivers/dri/radeon/radeon_screen.c
@@ -86,6 +86,7 @@ DRI_CONF_BEGIN
         DRI_CONF_DEF_MAX_ANISOTROPY(1.0,"1.0,2.0,4.0,8.0,16.0")
         DRI_CONF_NO_NEG_LOD_BIAS(false)
         DRI_CONF_FORCE_S3TC_ENABLE(false)
+        DRI_CONF_FORCE_CLAMPTOEDGE(false)
         DRI_CONF_COLOR_REDUCTION(DRI_CONF_COLOR_REDUCTION_DITHER)
         DRI_CONF_ROUND_MODE(DRI_CONF_ROUND_TRUNC)
         DRI_CONF_DITHER_MODE(DRI_CONF_DITHER_XERRORDIFF)
@@ -113,6 +114,7 @@ DRI_CONF_BEGIN
         DRI_CONF_DEF_MAX_ANISOTROPY(1.0,"1.0,2.0,4.0,8.0,16.0")
         DRI_CONF_NO_NEG_LOD_BIAS(false)
         DRI_CONF_FORCE_S3TC_ENABLE(false)
+        DRI_CONF_FORCE_CLAMPTOEDGE(false)
         DRI_CONF_COLOR_REDUCTION(DRI_CONF_COLOR_REDUCTION_DITHER)
         DRI_CONF_ROUND_MODE(DRI_CONF_ROUND_TRUNC)
         DRI_CONF_DITHER_MODE(DRI_CONF_DITHER_XERRORDIFF)
@@ -194,8 +196,9 @@ DRI_CONF_BEGIN
                DRI_CONF_TEXTURE_DEPTH(DRI_CONF_TEXTURE_DEPTH_FB)
                DRI_CONF_DEF_MAX_ANISOTROPY(1.0, "1.0,2.0,4.0,8.0,16.0")
                DRI_CONF_NO_NEG_LOD_BIAS(false)
-                DRI_CONF_FORCE_S3TC_ENABLE(false)
+               DRI_CONF_FORCE_S3TC_ENABLE(false)
                DRI_CONF_DISABLE_S3TC(false)
+               DRI_CONF_FORCE_CLAMPTOEDGE(false)
                DRI_CONF_COLOR_REDUCTION(DRI_CONF_COLOR_REDUCTION_DITHER)
                DRI_CONF_ROUND_MODE(DRI_CONF_ROUND_TRUNC)
                DRI_CONF_DITHER_MODE(DRI_CONF_DITHER_XERRORDIFF)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 3e4c492..002721a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3089,6 +3089,9 @@ struct __GLcontextRec
    /** software compression/decompression supported or not */
    GLboolean Mesa_DXTn;
 
+   /** fix up GL_CLAMP to GL_CLAMP_TO_EDGE for broken apps or not */
+   GLboolean Mesa_fixup_clamp;
+
    /** Core tnl module support */
    struct gl_tnl_module TnlModule;
 
diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
index c9f8a06..4cfd15e 100644
--- a/src/mesa/main/texstate.c
+++ b/src/mesa/main/texstate.c
@@ -1184,9 +1184,11 @@ _mesa_TexParameterf( GLenum target, GLenum pname, 
GLfloat param )
 void GLAPIENTRY
 _mesa_TexParameterfv( GLenum target, GLenum pname, const GLfloat *params )
 {
-   const GLenum eparam = (GLenum) (GLint) params[0];
+   GLenum eparam = (GLenum) (GLint) params[0];
    struct gl_texture_unit *texUnit;
    struct gl_texture_object *texObj;
+   const GLfloat clampparams[4] = {(GLfloat) GL_CLAMP_TO_EDGE, 0.0F, 0.0F, 
0.0F};
+   const GLfloat *fparams = params;
    GET_CURRENT_CONTEXT(ctx);
    ASSERT_OUTSIDE_BEGIN_END(ctx);
 
@@ -1284,6 +1286,10 @@ _mesa_TexParameterfv( GLenum target, GLenum pname, const 
GLfloat *params )
          }
          break;
       case GL_TEXTURE_WRAP_S:
+         if (eparam == GL_CLAMP && ctx->Mesa_fixup_clamp) {
+            eparam = GL_CLAMP_TO_EDGE;
+            fparams = clampparams;
+         }
          if (texObj->WrapS == eparam)
             return;
          if (validate_texture_wrap_mode(ctx, texObj->Target, eparam)) {
@@ -1295,6 +1301,10 @@ _mesa_TexParameterfv( GLenum target, GLenum pname, const 
GLfloat *params )
          }
          break;
       case GL_TEXTURE_WRAP_T:
+         if (eparam == GL_CLAMP && ctx->Mesa_fixup_clamp) {
+            eparam = GL_CLAMP_TO_EDGE;
+            fparams = clampparams;
+         }
          if (texObj->WrapT == eparam)
             return;
          if (validate_texture_wrap_mode(ctx, texObj->Target, eparam)) {
@@ -1306,6 +1316,10 @@ _mesa_TexParameterfv( GLenum target, GLenum pname, const 
GLfloat *params )
          }
          break;
       case GL_TEXTURE_WRAP_R:
+         if (eparam == GL_CLAMP && ctx->Mesa_fixup_clamp) {
+            eparam = GL_CLAMP_TO_EDGE;
+            fparams = clampparams;
+         }
          if (texObj->WrapR == eparam)
             return;
          if (validate_texture_wrap_mode(ctx, texObj->Target, eparam)) {
@@ -1521,7 +1535,7 @@ _mesa_TexParameterfv( GLenum target, GLenum pname, const 
GLfloat *params )
    texObj->_Complete = GL_FALSE;
 
    if (ctx->Driver.TexParameter) {
-      (*ctx->Driver.TexParameter)( ctx, target, texObj, pname, params );
+      (*ctx->Driver.TexParameter)( ctx, target, texObj, pname, fparams );
    }
 }
 
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to