Bug 3358 - Win32: Caching broken when subdirs involved
Summary: Win32: Caching broken when subdirs involved
Status: RESOLVED DUPLICATE of bug 6366
Alias: None
Product: fontconfig
Classification: Unclassified
Component: library (show other bugs)
Version: 2.3
Hardware: x86 (IA32) Windows (All)
: high normal
Assignee: Keith Packard
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-05-21 13:01 UTC by Richard Hughes
Modified: 2007-01-23 17:34 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Proposed fix (1.52 KB, patch)
2005-05-30 16:18 UTC, Richard Hughes
Details | Splinter Review

Description Richard Hughes 2005-05-21 13:01:51 UTC
It's only noticable when you have a lot of fonts, but caching doesn't work on 
Windows if you have laid out your fonts into subdirectories in c:\windows\fonts.

The cache file has entries such as "c:\\winnt\\fonts/sans-serif\\arial.ttf" but 
the directory scanning in FcGlobalCacheDirGet() is a strncmp which is comparing 
against "c:\\winnt\\fonts/sans-serif/arial.ttf". Both will work on Windows, but 
the strncmp (and FcCacheHash()) doesn't know this.

I have kludged around it by changing
    strcat ((char *) file, "/");
in fcdir.c:FcDirScanConfig() (line 220 in 2.3.1) to
    strcat ((char *) file, "\\");

This solution will work (provided it's wrapped in an appropriate #ifdef _WIN32 
of course), but the 'correct' solution would probably be to somehow normalise 
paths as they are created to account for the liberal parsing behaviour of 
Windows, eg convert to lowercase, turn \\ into / and merge repeated slashes.

Having said that subdirs are required, now that I think about it I'm not so 
sure: my font cache has "C:\\WINNT\\fonts\\GARA.TTF" but surely fcdir.c will try 
to parse "C:\\WINNT\\fonts/GARA.TTF". Whatever, this patch removes all the 
delays due to rescanning at every load.

Richard.
Comment 1 Keith Packard 2005-05-21 17:58:35 UTC
I think we can probably just use \ as the only path separator on Windows and
expect things to work that way.  I would prefer a simple patch that made the
path separator a #define value (probably one for "\\" and another for '\\').
Comment 2 Richard Hughes 2005-05-22 03:29:56 UTC
There is the issue of fonts.conf which specifies <dir>~/.fonts</dir>

I don't have any fonts in there and I presume very few Windows users do, but it 
might be necessary to change that as well.

There is the option, however, of going the other way and using / for everything 
by converting the windows and home directory names as they are read. This might 
be safer in the long run because you wouldn't have to remember to use the 
#define all the time.
Comment 3 Keith Packard 2005-05-22 08:53:20 UTC
We can change the / characters around in fonts.conf easily enough when building
it from fonts.conf.in.

I suspect that checking the code for / in strings and character constants will
be easily automated, certainly easier than making sure every filename goes
through a canonicalization process.
Comment 4 Richard Hughes 2005-05-22 10:41:22 UTC
OK. Do you want me to try to put a real patch together or are you going to do 
it? Personally I'd prefer it if you did it because I don't know autoconf or the 
rest of your build system. I'm fairly certain I didn't use the 'approved' method 
in my debugging.
Comment 5 Keith Packard 2005-05-22 15:19:14 UTC
I wouldn't have any way to test the results.  This particular change is quite
easy though, in the configure.in you'll find an existing section that
conditionalizes FC_DEFAULT_FONTS which is used in fonts.conf.in.  Just parallel
that code and things should be OK.
Comment 6 Richard Hughes 2005-05-30 16:18:38 UTC
Created attachment 2801 [details] [review]
Proposed fix

As discussed, a patch to fcint.h to add FC_DIR_SEPARATOR and use it in fcdir.c.


I also had to patch fccfg.c to get it to build on my system. The non-inclusion
of windows.h when building the static version made GetTempPath unavailable.
Comment 7 Patrick Lam 2006-02-22 16:23:41 UTC
Well, we *do* pass every font filename through canonicalization now.  But I
don't think mmap works on Windows, so that might be something to fix first...
Comment 8 Patrick Lam 2006-04-06 15:18:13 UTC
6366 depends on 6494, which I'll look into; but it seems to be the same bug,
just newer.

*** This bug has been marked as a duplicate of 6366 ***


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.