We've previously discussed if we should add some sort of selection
option type and not use string options and string restrictions for that
purpose.

I don't think we need a new option type for this. The integer type
should do just fine. The attached patch changes the blur plugin's filter
option from string to integer type and adds the relevant metadata. I
suggest that we change all existing options that use string restrictions
in a similar way and remove the string restrictions completely.

Questions? Does it sounds OK?

- David
--- a/include/compiz.h
+++ b/include/compiz.h
@@ -3087,6 +3087,12 @@ matchPropertyChanged (CompDisplay *display,
 
 /* metadata.c */
 
+#define STRINGIFY(x) #x
+#define TOSTRING(x) STRINGIFY (x)
+#define MINTOSTRING(x) "<min>" TOSTRING (x) "</min>"
+#define MAXTOSTRING(x) "<max>" TOSTRING (x) "</max>"
+#define RESTOSTRING(min, max) MINTOSTRING (min) MAXTOSTRING (max)
+
 typedef struct _CompMetadataOptionInfo {
     char		   *name;
     char		   *type;
--- a/metadata/blur.xml.in
+++ b/metadata/blur.xml.in
@@ -38,15 +38,24 @@
 		<_long>Blur behind translucent parts of windows</_long>
 		<default>true</default>
 	    </option>
-	    <option name="filter" type="string">
+	    <option name="filter" type="int">
 		<_short>Blur Filter</_short>
 		<_long>Filter method used for blurring</_long>
-		<default>4xBilinear</default>
-		<allowed>
-		    <value>4xBilinear</value>
-		    <value>Gaussian</value>
-		    <value>Mipmap</value>
-		</allowed>
+		<default>0</default>
+		<min>0</min>
+		<max>2</max>
+		<desc>
+		    <value>0</value>
+		    <_name>4xBilinear</_name>
+		</desc>
+		<desc>
+		    <value>1</value>
+		    <_name>Gaussian</_name>
+		</desc>
+		<desc>
+		    <value>2</value>
+		    <_name>Mipmap</_name>
+		</desc>
 	    </option>
 	    <option name="gaussian_radius" type="int">
 		<_short>Gaussian Radius</_short>
--- a/plugins/blur.c
+++ b/plugins/blur.c
@@ -37,11 +37,10 @@ static CompMetadata blurMetadata;
 
 #define BLUR_GAUSSIAN_RADIUS_MAX 15
 
-typedef enum {
-    BlurFilter4xBilinear,
-    BlurFilterGaussian,
-    BlurFilterMipmap
-} BlurFilter;
+#define BLUR_FILTER_4X_BILINEAR 0
+#define BLUR_FILTER_GAUSSIAN    1
+#define BLUR_FILTER_MIPMAP      2
+#define BLUR_FILTER_LAST	BLUR_FILTER_MIPMAP
 
 typedef struct _BlurFunction {
     struct _BlurFunction *next;
@@ -121,8 +120,7 @@ typedef struct _BlurScreen {
 
     Bool blurOcclusion;
 
-    BlurFilter filter;
-    int        filterRadius;
+    int filterRadius;
 
     BlurFunction *srcBlurFunctions;
     BlurFunction *dstBlurFunctions;
@@ -190,17 +188,6 @@ typedef struct _BlurWindow {
 
 #define NUM_OPTIONS(s) (sizeof ((s)->opt) / sizeof (CompOption))
 
-static BlurFilter
-blurFilterFromString (CompOptionValue *value)
-{
-    if (strcasecmp (value->s, "gaussian") == 0)
-	return BlurFilterGaussian;
-    else if (strcasecmp (value->s, "mipmap") == 0)
-	return BlurFilterMipmap;
-    else
-	return BlurFilter4xBilinear;
-}
-
 /* pascal triangle based kernel generator */
 static int
 blurCreateGaussianLinearKernel (int   radius,
@@ -280,11 +267,11 @@ blurUpdateFilterRadius (CompScreen *s)
 {
     BLUR_SCREEN (s);
 
-    switch (bs->filter) {
-    case BlurFilter4xBilinear:
+    switch (bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i) {
+    case BLUR_FILTER_4X_BILINEAR:
 	bs->filterRadius = 2;
 	break;
-    case BlurFilterGaussian: {
+    case BLUR_FILTER_GAUSSIAN: {
 	int   radius   = bs->opt[BLUR_SCREEN_OPTION_GAUSSIAN_RADIUS].value.i;
 	float strength = bs->opt[BLUR_SCREEN_OPTION_GAUSSIAN_STRENGTH].value.f;
 
@@ -293,7 +280,7 @@ blurUpdateFilterRadius (CompScreen *s)
 
 	bs->filterRadius = radius;
     } break;
-    case BlurFilterMipmap: {
+    case BLUR_FILTER_MIPMAP: {
 	float lod = bs->opt[BLUR_SCREEN_OPTION_MIPMAP_LOD].value.f;
 
 	bs->filterRadius = powf (2.0f, ceilf (lod));
@@ -561,7 +548,7 @@ blurSetScreenOption (CompPlugin      *plugin,
 		     CompOptionValue *value)
 {
     CompOption *o;
-    int	       index;
+    int	       index, filter;
 
     BLUR_SCREEN (screen);
 
@@ -613,9 +600,8 @@ blurSetScreenOption (CompPlugin      *plugin,
 	}
 	break;
     case BLUR_SCREEN_OPTION_FILTER:
-	if (compSetStringOption (o, value))
+	if (compSetIntOption (o, value))
 	{
-	    bs->filter = blurFilterFromString (&o->value);
 	    blurReset (screen);
 	    damageScreen (screen);
 	    return TRUE;
@@ -624,7 +610,8 @@ blurSetScreenOption (CompPlugin      *plugin,
     case BLUR_SCREEN_OPTION_GAUSSIAN_RADIUS:
 	if (compSetIntOption (o, value))
 	{
-	    if (bs->filter == BlurFilterGaussian)
+	    filter = bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i;
+	    if (filter == BLUR_FILTER_GAUSSIAN)
 	    {
 		blurReset (screen);
 		damageScreen (screen);
@@ -635,7 +622,8 @@ blurSetScreenOption (CompPlugin      *plugin,
     case BLUR_SCREEN_OPTION_GAUSSIAN_STRENGTH:
 	if (compSetFloatOption (o, value))
 	{
-	    if (bs->filter == BlurFilterGaussian)
+	    filter = bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i;
+	    if (filter == BLUR_FILTER_GAUSSIAN)
 	    {
 		blurReset (screen);
 		damageScreen (screen);
@@ -646,7 +634,8 @@ blurSetScreenOption (CompPlugin      *plugin,
     case BLUR_SCREEN_OPTION_MIPMAP_LOD:
 	if (compSetFloatOption (o, value))
 	{
-	    if (bs->filter == BlurFilterMipmap)
+	    filter = bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i;
+	    if (filter == BLUR_FILTER_MIPMAP)
 	    {
 		blurReset (screen);
 		damageScreen (screen);
@@ -1027,8 +1016,8 @@ getSrcBlurFragmentFunction (CompScreen  *s,
 
 	ok &= addDataOpToFunctionData (data, str);
 
-	switch (bs->filter) {
-	case BlurFilter4xBilinear:
+	switch (bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i) {
+	case BLUR_FILTER_4X_BILINEAR:
 	default:
 	    ok &= addFetchOpToFunctionData (data, "output", "offset0", target);
 	    ok &= addDataOpToFunctionData (data, "MUL sum, output, 0.25;");
@@ -1115,8 +1104,8 @@ getDstBlurFragmentFunction (CompScreen  *s,
 	if (saturation < 100)
 	    ok &= addTempHeaderOpToFunctionData (data, "sat");
 
-	switch (bs->filter) {
-	case BlurFilter4xBilinear: {
+	switch (bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i) {
+	case BLUR_FILTER_4X_BILINEAR: {
 	    static char *filterTemp[] = {
 		"t0", "t1", "t2", "t3",
 		"s0", "s1", "s2", "s3"
@@ -1162,7 +1151,7 @@ getDstBlurFragmentFunction (CompScreen  *s,
 
 	    ok &= addDataOpToFunctionData (data, str);
 	} break;
-	case BlurFilterGaussian: {
+	case BLUR_FILTER_GAUSSIAN: {
 	    static char *filterTemp[] = {
 		"tCoord", "pix"
 	    };
@@ -1211,7 +1200,7 @@ getDstBlurFragmentFunction (CompScreen  *s,
 		ok &= addDataOpToFunctionData (data, str);
 	    }
 	} break;
-	case BlurFilterMipmap:
+	case BLUR_FILTER_MIPMAP:
 	    ok &= addFetchOpToFunctionData (data, "output", NULL, target);
 	    ok &= addColorOpToFunctionData (data, "output", "output");
 
@@ -1669,14 +1658,17 @@ blurUpdateDstTexture (CompWindow	  *w,
     BoxPtr     pBox;
     int	       nBox;
     int        y;
+    int        filter;
 
     BLUR_SCREEN (s);
     BLUR_WINDOW (w);
 
+    filter = bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i;
+
     /* create empty region */
     XSubtractRegion (&emptyRegion, &emptyRegion, bs->tmpRegion3);
 
-    if (bs->filter == BlurFilterGaussian)
+    if (filter == BLUR_FILTER_GAUSSIAN)
     {
 	REGION region;
 
@@ -1779,7 +1771,7 @@ blurUpdateDstTexture (CompWindow	  *w,
 	    bs->ty = 1;
 	}
 
-	if (bs->filter == BlurFilterGaussian)
+	if (filter == BLUR_FILTER_GAUSSIAN)
 	{
 	    if (s->fbo && !bs->fbo)
 		(*s->genFramebuffers) (1, &bs->fbo);
@@ -1817,7 +1809,7 @@ blurUpdateDstTexture (CompWindow	  *w,
 	    glTexParameteri (bs->target, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
 	    glTexParameteri (bs->target, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
 
-	    if (bs->filter == BlurFilterMipmap)
+	    if (filter == BLUR_FILTER_MIPMAP)
 	    {
 		if (!s->fbo)
 		{
@@ -1867,13 +1859,13 @@ blurUpdateDstTexture (CompWindow	  *w,
 	}
     }
 
-    switch (bs->filter) {
-    case BlurFilterGaussian:
+    switch (filter) {
+    case BLUR_FILTER_GAUSSIAN:
 	return fboUpdate (s, bs->tmpRegion->rects, bs->tmpRegion->numRects);
-    case BlurFilterMipmap:
+    case BLUR_FILTER_MIPMAP:
 	(*s->generateMipmap) (bs->target);
 	break;
-    case BlurFilter4xBilinear:
+    case BLUR_FILTER_4X_BILINEAR:
 	break;
     }
 
@@ -2061,8 +2053,8 @@ blurDrawWindowTexture (CompWindow	    *w,
 	    FragmentAttrib dstFa = fa;
 	    float	   threshold = (float) bw->state[state].threshold;
 
-	    switch (bs->filter) {
-	    case BlurFilter4xBilinear:
+	    switch (bs->opt[BLUR_SCREEN_OPTION_FILTER].value.i) {
+	    case BLUR_FILTER_4X_BILINEAR:
 		dx = bs->tx / 2.1f;
 		dy = bs->ty / 2.1f;
 
@@ -2091,7 +2083,7 @@ blurDrawWindowTexture (CompWindow	    *w,
 						 dx, dy, 0.0f, 0.0f);
 		}
 		break;
-	    case BlurFilterGaussian:
+	    case BLUR_FILTER_GAUSSIAN:
 		param = allocFragmentParameters (&dstFa, 5);
 		unit  = allocFragmentTextureUnits (&dstFa, 2);
 
@@ -2126,7 +2118,7 @@ blurDrawWindowTexture (CompWindow	    *w,
 						     0.0f, 0.0f);
 		}
 		break;
-	    case BlurFilterMipmap:
+	    case BLUR_FILTER_MIPMAP:
 		param = allocFragmentParameters (&dstFa, 2);
 		unit  = allocFragmentTextureUnits (&dstFa, 1);
 
@@ -2465,7 +2457,7 @@ static const CompMetadataOptionInfo blurScreenOptionInfo[] = {
     { "focus_blur", "bool", 0, 0, 0 },
     { "alpha_blur_match", "match", 0, 0, 0 },
     { "alpha_blur", "bool", 0, 0, 0 },
-    { "filter", "string", 0, 0, 0 },
+    { "filter", "int", RESTOSTRING (0, BLUR_FILTER_LAST), 0, 0 },
     { "gaussian_radius", "int", "<min>1</min><max>15</max>", 0, 0 },
     { "gaussian_strength", "float", "<min>0.0</min><max>1.0</max>", 0, 0 },
     { "mipmap_lod", "float", "<min>0.1</min><max>5.0</max>", 0, 0 },
@@ -2585,9 +2577,6 @@ blurInitScreen (CompPlugin *p,
 	fprintf (stderr, "%s: No stencil buffer. Region based blur disabled\n",
 		 programName);
 
-    bs->filter =
-	blurFilterFromString (&bs->opt[BLUR_SCREEN_OPTION_FILTER].value);
-
     /* We need GL_ARB_fragment_program for blur */
     if (s->fragmentProgram)
 	bs->alphaBlur = bs->opt[BLUR_SCREEN_OPTION_ALPHA_BLUR].value.b;
_______________________________________________
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz

Reply via email to