Re: matching multiple families in <alias> and <test>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Raimund Steger wrote:
[...]

Maybe when going through the FcOpComma operands, one could tag the
matching FcValueList's somehow and loop precisely over them for the
following prepend/append/assign edits.

I've now tried to modify fccfg.c along those lines and attached an idea of how I think it might work. It's probably far from being particularly elegant (or bug-free), but in a simple test case (matching a number of families through multi-family <alias> and <test>) it seems to do what I have in mind:

(1) Save away every match in the test (but at most one per FcOpComma operand). For this, FcSubState is updated to allow a linked list that contains additional malloc'ed elements in the case of additional matches.

(2) For assign/prepend/append edits, go through that linked list and apply every edit once for every match, instead of once for the test. This means, it should simulate the behavior of separate rules.

It's not extensively tested, but maybe it can further fuel the discussion...

Raimund

diff --git a/src/fccfg.c b/src/fccfg.c
index f9cdaaf..bea48a6 100644
--- a/src/fccfg.c
+++ b/src/fccfg.c
@@ -663,9 +663,14 @@ FcConfigAddEdit (FcConfig	*config,
     return FcTrue;
 }
 
+typedef struct _FcSubStateValue {
+    FcValueList		    *value;
+    struct _FcSubStateValue *next;
+} FcSubStateValue;
+
 typedef struct _FcSubState {
-    FcPatternElt   *elt;
-    FcValueList    *value;
+    FcPatternElt    *elt;
+    FcSubStateValue stvalue;
 } FcSubState;
 
 static FcValue
@@ -1173,15 +1178,33 @@ FcConfigEvaluate (FcPattern *p, FcExpr *e)
     return v;
 }
 
-static FcValueList *
+static void
+FcFreeSubStateValues (FcSubStateValue *v)
+{
+    if (v->next)
+	FcFreeSubStateValues (v->next);
+    FcMemFree (FC_MEM_SUBSTATE, sizeof (FcSubStateValue));
+    free (v);
+}
+
+/*
+ * Compares values in an FcTest (there can be more than one if
+ * FcOpComma was used) with the ones in the provided FcValueList and
+ * returns all matches as linked list, with the first element as
+ * value, and all further elements on the heap, which need to be freed
+ * by the caller using FcFreeSubStateValues.
+ */
+static FcSubStateValue
 FcConfigMatchValueList (FcPattern	*p,
 			FcTest		*t,
 			FcValueList	*values)
 {
-    FcValueList	    *ret = 0;
-    FcExpr	    *e = t->expr;
-    FcValue	    value;
-    FcValueList	    *v;
+    FcSubStateValue  ret = { 0, 0 };
+    FcSubStateValue  *stvalue = &ret;
+    FcExpr	     *e = t->expr;
+    FcValue	     value;
+    FcValueList	     *v;
+    FcBool	     m;
 
     while (e)
     {
@@ -1197,19 +1220,41 @@ FcConfigMatchValueList (FcPattern	*p,
 	    e = 0;
 	}
 
-	for (v = values; v; v = FcValueListNext(v))
+	for (m = FcFalse, v = values; v; v = FcValueListNext(v))
 	{
 	    /* Compare the pattern value to the match expression value */
 	    if (FcConfigCompareValue (&v->value, t->op, &value))
 	    {
-		if (!ret)
-		    ret = v;
+		/* Every comma operand can match at most once, so that
+		 * we don't deviate too much from 2.9.0 behavior  */
+		if(!m)
+		{
+		    m = FcTrue;
+
+		    if(stvalue->value)
+		    {
+			stvalue->next = malloc (sizeof (FcSubStateValue ));
+			if (!stvalue->next)
+			    continue;
+			FcMemAlloc (FC_MEM_SUBSTATE, sizeof (FcSubStateValue ));
+			stvalue = stvalue->next;
+			stvalue->next = 0;
+		    }
+		    stvalue->value = v;
+		}
 	    }
 	    else
 	    {
+		/* XXX FcQualAll w/ FcOpComma is still a problem;
+		 * depends on order of expressions in t, and will
+		 * never match for more than one value in v. It's
+		 * easier to keep supporting only one operand here. */
 		if (t->qual == FcQualAll)
 		{
-		    ret = 0;
+		    if (ret.next)
+			FcFreeSubStateValues (ret.next);
+		    ret.value = 0;
+		    ret.next = 0;
 		    break;
 		}
 	    }
@@ -1398,6 +1443,7 @@ FcConfigSubstituteWithPat (FcConfig    *config,
     FcEdit	    *e;
     FcValueList	    *l;
     FcPattern	    *m;
+    FcSubStateValue *stvalue, *stvalue1;
 
     if (!config)
     {
@@ -1444,6 +1490,7 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 		FcTestPrint (t);
 	    }
 	    st[i].elt = 0;
+	    st[i].stvalue.next = 0;
 	    if (kind == FcMatchFont && t->kind == FcMatchPattern)
 		m = p_pat;
 	    else
@@ -1460,7 +1507,7 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 	    {
 		if (t->qual == FcQualAll)
 		{
-		    st[i].value = 0;
+		    st[i].stvalue.value = 0;
 		    continue;
 		}
 		else
@@ -1470,13 +1517,15 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 	     * Check to see if there is a match, mark the location
 	     * to apply match-relative edits
 	     */
-	    st[i].value = FcConfigMatchValueList (m, t, st[i].elt->values);
-	    if (!st[i].value)
-		break;
-	    if (t->qual == FcQualFirst && st[i].value != st[i].elt->values)
-		break;
-	    if (t->qual == FcQualNotFirst && st[i].value == st[i].elt->values)
+	    st[i].stvalue = FcConfigMatchValueList (m, t, st[i].elt->values);
+	    if (!st[i].stvalue.value)
 		break;
+	    if (t->qual == FcQualFirst &&
+		st[i].stvalue.value != st[i].elt->values)
+		break; /* XXX could be updated for comma-op, but in what way? */
+	    if (t->qual == FcQualNotFirst &&
+		st[i].stvalue.value == st[i].elt->values)
+		break; /* XXX same here */
 	}
 	if (t)
 	{
@@ -1492,7 +1541,11 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 	for (e = s->edit; e; e = e->next)
 	{
 	    /*
-	     * Evaluate the list of expressions
+	     * Evaluate the list of expressions. NOTE: For edits that
+	     * use this list more than once, it is duplicated below,
+	     * because the same instance can't be used more than once
+	     * (tail's "next" field will be set which would lead to
+	     * cycles).
 	     */
 	    l = FcConfigValues (p, e->expr, e->binding);
 	    /*
@@ -1526,26 +1579,39 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 		 */
 		if (t)
 		{
-		    FcValueList	*thisValue = st[i].value;
-		    FcValueList	*nextValue = thisValue;
-		
-		    /*
-		     * Append the new list of values after the current value
-		     */
-		    FcConfigAdd (&st[i].elt->values, thisValue, FcTrue, l);
-		    /*
-		     * Delete the marked value
-		     */
-                    if (thisValue)
-			FcConfigDel (&st[i].elt->values, thisValue);
-		    /*
-		     * Adjust any pointers into the value list to ensure
-		     * future edits occur at the same place
-		     */
-		    for (t = s->test, i = 0; t; t = t->next, i++)
+		    for (stvalue = &st[i].stvalue; stvalue && l;
+			 stvalue = stvalue->next)
 		    {
-			if (st[i].value == thisValue)
-			    st[i].value = nextValue;
+			FcValueList	*thisValue = stvalue->value;
+			FcValueList	*nextValue = thisValue; /* XXX
+			    * wouldn't 'l' make more sense here, generally?
+			    * Everything will get appended at the end because
+			    * the position won't be updated! */
+
+			/*
+			 * Append the new list of values after the current value
+			 */
+			FcConfigAdd (&st[i].elt->values, thisValue, FcTrue, l);
+			/*
+			 * Delete the marked value
+			 */
+			if (thisValue)
+			    FcConfigDel (&st[i].elt->values, thisValue);
+			/*
+			 * Adjust any pointers into the value list to ensure
+			 * future edits occur at the same place
+			 */
+			for (t = s->test, i = 0; t; t = t->next, i++)
+			{
+			    for (stvalue1 = &st[i].stvalue; stvalue1;
+				 stvalue1 = stvalue1->next)
+			    {
+				if (stvalue1->value == thisValue)
+				    stvalue1->value = nextValue;
+			    }
+			}
+			if (stvalue->next)
+			    l = FcConfigValues (p, e->expr, e->binding);
 		    }
 		    break;
 		}
@@ -1566,15 +1632,26 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 		    FcPatternElt    *thisElt = st[i].elt;
 		    for (t = s->test, i = 0; t; t = t->next, i++)
 		    {
-			if (st[i].elt == thisElt)
-			    st[i].value = 0;
+			for (stvalue = &st[i].stvalue; stvalue;
+			     stvalue = stvalue->next)
+			{
+			    if (st[i].elt == thisElt)
+				stvalue->value = 0;
+			}
 		    }
 		}
 		break;
 	    case FcOpPrepend:
 		if (t)
 		{
-		    FcConfigAdd (&st[i].elt->values, st[i].value, FcFalse, l);
+		    for (stvalue = &st[i].stvalue; stvalue && l;
+			 stvalue = stvalue->next)
+		    {
+			FcConfigAdd (&st[i].elt->values, stvalue->value,
+				     FcFalse, l);
+			if (stvalue->next)
+			    l = FcConfigValues (p, e->expr, e->binding);
+		    }
 		    break;
 		}
 		/* fall through ... */
@@ -1584,7 +1661,14 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 	    case FcOpAppend:
 		if (t)
 		{
-		    FcConfigAdd (&st[i].elt->values, st[i].value, FcTrue, l);
+		    for (stvalue = &st[i].stvalue; stvalue && l;
+			 stvalue = stvalue->next)
+		    {
+			FcConfigAdd (&st[i].elt->values, stvalue->value,
+				     FcTrue, l);
+			if (stvalue->next)
+			    l = FcConfigValues (p, e->expr, e->binding);
+		    }
 		    break;
 		}
 		/* fall through ... */
@@ -1608,6 +1692,12 @@ FcConfigSubstituteWithPat (FcConfig    *config,
 	    printf ("FcConfigSubstitute edit");
 	    FcPatternPrint (p);
 	}
+	/* Free additional substate values (for comma ops in tests) */
+	for (t = s->test, i = 0; t; t = t->next, i++)
+	{
+	    if (st[i].stvalue.next)
+		FcFreeSubStateValues (st[i].stvalue.next);
+	}
     }
     FcMemFree (FC_MEM_SUBSTATE, config->maxObjects * sizeof (FcSubState));
     free (st);
_______________________________________________
Fontconfig mailing list
Fontconfig@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/fontconfig

[Index of Archives]     [Fedora Fonts]     [Fedora Users]     [Fedora Cloud]     [Kernel]     [Fedora Packaging]     [Fedora Desktop]     [PAM]     [Gimp Graphics Editor]     [Yosemite News]

  Powered by Linux