Bug 17722

Is it maybe a good idea to have is_set and not_set for compare attribute?

<match target="font">
  <test name="rgba" compare="not_set" />
  <edit name="rgba" mode="assign"><const>none</const></edit>
</match>

Even though it may be hard to determine which one (no-sub-pixel, sub-pixel-{bgr,rgb,vbgr,vrgb}) is better for. I guess this kind of config is never enabled by default and should simply leave it to the desktop preferences.

qual="all" can actually already be used to test that a value is not set because it always returns true when there are no values to test.  i.e. All zero values match.

For the rgba case, overriding a value of "unknown" seems quite reasonable.

<match target="font">
  <test name="rgba" qual="all"><const>unknown</const></test>
  <edit name="rgba" mode="assign"><const>none</const></edit>
</match>

The match will apply when rgba is set to "unknown" or when rgba is not set.

For some other properties there is no unknown value, and so using qual="all" gets more obscure because we have to compare with a value that will never match an appropriately set value.  e.g.

<match target="font">
  <test name="hintstyle" qual="all"><int>-1</int></test>
  <edit name="hintstyle" mode="assign"><const>hintmedium</const></edit>
</match>

I found that making these "set when not already set" rules work appropriately with Qt3 required an additional hack.  Qt3 was calling FcFontMatch (and thus FcFontRenderPrepare) before XftFontMatch (which calls FcFontMatch again after
XftDefaultSubstitute).  Xft screen options had not been applied in the
first FcFontRenderPrepare (before XftFontMatch), and so the rules were matching and setting values that would take precedence over Xft screen options.  This situation was detectable through missing pixelsize as FcDefaultSubstitute had not been called before the first FcFontRenderPrepare.  I haven't looked at Qt4.

This test matches only when pixelsize is set (-1.0 is considered an inappropriate value).

  <test name="pixelsize" target="pattern" compare="not_eq">
    <double>-1.0</double>
  </test>

(In reply to comment #1)
> I guess this kind of config is
> never enabled by default and should simply leave it to the desktop preferences.

You would hope so, but some distribution(s) where enabling these by default.
Hopefully they have stopped doing that now.

(In reply to comment #2)
> qual="all" can actually already be used to test that a value is not set because
> it always returns true when there are no values to test.  i.e. All zero values
> match.

Hmm, that sounds like a bug though. IMHO we should drop such config entirely instead of ignoring the invalid element only or warn it at least, because there may be a typo and it's hard to track down when something behaves unexpectedly.

(In reply to comment #4)
> (In reply to comment #2)
> > qual="all" can actually already be used to test that a value is not set because
> > it always returns true when there are no values to test.  i.e. All zero values
> > match.
> 
> Hmm, that sounds like a bug though.

Adding "set" and "not_set" comparisons sounds sensible to make things clearer,
but I don't think the "all" behavior is a bug.

Please don't change the behavior of all.
It is this behavior that makes all,not_eq the complement of any,eq.

(In reply to comment #5)
> Adding "set" and "not_set" comparisons sounds sensible to make things clearer,
> but I don't think the "all" behavior is a bug.

the behavior of qual="all" itself isn't a problem. I just think either of cases should be checked with qual="all" compare="not_eq" even if the testcase contains any invalid thing, as you indicated in the last example. that really makes sense for what you want to. but relying on "All zero values match" isn't a good idea, because it's the side-effect because of no-error-handling.

> Please don't change the behavior of all.
> It is this behavior that makes all,not_eq the complement of any,eq.

If there are too much cases relying on this behavior, we should show a warning at least if something failed to parse, because it's difficult to see whether something might looks like a typo is desired or not. let's see how much things using this trick and we can avoid an regression as much as possible.

An alternative may be to simply make these configs append values instead of assign.  Most clients I guess care about the first value only.

(In reply to comment #7)
> An alternative may be to simply make these configs append values instead of
> assign.  Most clients I guess care about the first value only.

Aha. that sounds reasonable.

(In reply to comment #8)
> (In reply to comment #7)
> > An alternative may be to simply make these configs append values instead of
> > assign.  Most clients I guess care about the first value only.
> 
> Aha. that sounds reasonable.

If you do that, please also add a big comment stating what's going on.

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > An alternative may be to simply make these configs append values instead of
> > > assign.  Most clients I guess care about the first value only.
> > 
> > Aha. that sounds reasonable.
> 
> If you do that, please also add a big comment stating what's going on.

Sure. I'll do that.

I guess we need to do similar for 10-autohint.conf, 10-unhinted.conf and 11-lcd-filter-*.conf too?

the proposed patch:
http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz17722

(In reply to comment #11)
> I guess we need to do similar for 10-autohint.conf, 10-unhinted.conf and
> 11-lcd-filter-*.conf too?

Yes, I think so.

Fixed in 1aaf8b77.