Apache OpenOffice (AOO) Bugzilla – Issue 64508
honour fontconfig autohint and hinting attributes
Last modified: 2010-09-17 12:45:17 UTC
A patch to honour fontconfig autohint and hinting attributes, similiar to the previous antialiasing and embedded bitmap directives
Created attachment 35807 [details] sample document
Created attachment 35808 [details] how it looks in vanilla OOo, with sample text in gedit for reference
Created attachment 35841 [details] patch to implement
Created attachment 35842 [details] after the patch is applied
1. Check for autohint/hinting/hintstyle from fontconfig and use them if possible 2. see issue 64109, FC_EMBEDDED_BITMAP etc don't seem to be filled in by default by FcObjectSetBuild, hence the call to FcConfigSubstitute to achieve this 3. mbUseGamma is set to true for Asian fonts, but the comment talks about being good for making artificial bold fonts. I suspect (hope) that the intention is to enable this Gammaing feature only for Artificial Bold creation, and not for all asian fonts.
Created attachment 35843 [details] better after-patch screenshot
cmc->pl: For your consideration
I'll check if this default substitution also behaves quadratic; one could hope that it runs on a smaller set and will not cost that dearly
it does not cost as much, but still it costs 1 second on every startup (on my 2000 font system). However this information (hint styling, embedded bitmaps, etc) is only needed on demand for specific fonts anyway since it does not influence font selection AFAIK. So perhaps we should not store this information for every font on startup, but instead find a way to get this information when needed. How about moving these enums from FastPrintFontInfo to PrintFontInfo and filling it in on the fly for specific fonts ? hdu: what is your opinion ? cmc: would you change the patch that way ?
Let me fiddle it around a bit. In theory deferring is the way to go, waiting until we know the actual desired font-size would allow the fontconfig "feature when <= fontsize" stuff to kick in.
Created attachment 36280 [details] new defer hints until we need them patch
cmc->pl: how about this ? (applied after vcl57) Defer until we need to know and also don't put the info in the cache. The reason the details weren't filled by fontconfig "properly" for us is apparently because these settings are overridable per-size so there isn't a correct answer unless the size is known.
I think this is almost what we want; some small things: - the getFontConfigHints method should be fed with a fontId instead of a FastPrintFontInfo; you already know exactly which font is involved (even if there is sadly no exact mapping to fontconfig at the moment); this way you can remove the mapping stuff in X11SalGraphics::GetFontHints - X11SalGraphics::GetFontHints does not handle the X font case (in which case it should not do anything) - Please add a stub for WinSalGraphics::GetFontHints I think if you change these minor things, we have a very good solution.
Created attachment 36282 [details] updated patch
how about that
pl->hdu: please consider this for integration into vcl59 there needs to be done: - handle the X font case in X11SalGraphics::GetFontHints - GetFontHints on SalGraphics should not be fed a fontId; it should be used with an ImplFontSelectData IMHO
The GetFontHints() for WinSalGraphics and PspGraphics returns without having set rFontHints. It is only implemented for X11SalGraphics::GetFontHints(). X11SalGraphics is the only one that in practice get a GetFontHints() request, because only a VirtualDevice is ever asked to provide them. Constructing a whole VirtualDevice and its graphics is expensive, especially when considering that this vdev is supposed to get into the FreetypeServerFont constructor, which in some use cases gets called very often. I suggest to have ImplFontHints constructor call psp's getFontConfigHints() directly if psp is available. And making sure that it is fast, eventually by caching older FC results. Another small question is, what to do with hints for the stretched or squeezed fonts. In the latest patch the width is used. Is there are reason to use the width over the height?
"Is there are reason to use the width over the height?" I don't really know, but I think that when I looked at cairo, that was what it seemed to do, I didn't pick width over height for any good reason.
unfortunately, this task won't make it for OOo 2.0.4 due to some resource constraints. Retargeting to OOo 2.x
Created attachment 38531 [details] update patch for 2.0.4
Thanks for the patch. There are very many merge conflicts against SRC680m182 though: patching file source/glyphs/gcach_ftyp.cxx Hunk #1 succeeded at 102 with fuzz 2 (offset -5 lines). Hunk #2 succeeded at 834 with fuzz 2 (offset 29 lines). Hunk #3 succeeded at 1210 with fuzz 1 (offset 69 lines). Hunk #4 succeeded at 1221 (offset 69 lines). Hunk #5 FAILED at 1250. Hunk #6 FAILED at 1282. Hunk #7 FAILED at 1305. Hunk #8 succeeded at 1341 with fuzz 2 (offset 75 lines). Hunk #9 succeeded at 1391 with fuzz 2 (offset 86 lines). Hunk #10 FAILED at 1423. Hunk #11 FAILED at 1446. Hunk #12 succeeded at 1549 with fuzz 2 (offset 126 lines). Hunk #13 succeeded at 1601 with fuzz 2 (offset 137 lines). Hunk #14 FAILED at 1629. Hunk #15 succeeded at 1713 with fuzz 1 (offset 173 lines). Hunk #16 FAILED at 2291. 7 out of 16 hunks FAILED -- saving rejects to file source/glyphs/gcach_ftyp.cxx.rej patching file source/glyphs/gcach_ftyp.hxx Hunk #1 succeeded at 227 (offset 9 lines). patching file source/glyphs/glyphcache.cxx Hunk #1 FAILED at 89. 1 out of 1 hunk FAILED -- saving rejects to file source/glyphs/glyphcache.cxx.rej
For the psprint part of the patch I'll ask PL to have a closer look. A change in this area caused a bad showstopper (issue 65884) not long ago which was triggered by issue 61841.
changing target...
We really should do this but not for this target. With code freeze for 2.1 coming soon and open issues of the patch like - impact on platforms without or with old fontconfig libraries - impact on systems with fonts without good fontconfig configurations - performance impact of creating+destroying a VDev per font face I'm not comfortable with just merging the patch in yet.
Regarding the problems that could arise when using the system's settings rather than OO's: At the moment there is a setting in OO "use system fonts". Could this not expanded to "use system settings for fonts"? This would allow OO to respect the system's settings, if they work better than OO's for the user.
hdu: will you make it into 2.2 or should/could this wait for 2.3?
Sorry, not high enough priority to push it into 2.2
setting from patch to enhancement, since this seems to need additional work.
Anyone can be kind enough to attach a copy of the patch openoffice.org-2.0.2.ooo64508.vcl.honourfontconfighinting.patch that applies to SRC680_m217 ? Thanks in advance.
Issue 80539 might be interesting in this context
Created attachment 47681 [details] patch resynced to OOG680
@cmc: this latest patch looks like it belongs to issue 72129? Anyway, I could only apply it if OOo's base platform would contain fontconfig, or if the problem mentioned in the latest comment of issue 72129 (we need to wrap a dynamically loaded fontconfig, export it, etc.) was solved.
Created attachment 47690 [details] indeed, it was the wrong one, here's the updated one for this issue.
set target 3.0
Created attachment 53086 [details] updated patch for a post-cairo render world
After the cairo workspace in in place I don't think we need to use the cairo stuff in psprint anymore as for the interesting case cairo should just OR the cairo flags with the fontconfig flags itself (I think) so that can be removed from this proposal And the other addition is then from https://bugzilla.redhat.com/show_bug.cgi?id=443356 where fontconfig's rules only work on the "canonical" name so use that when fetching the fontconfig hints
Created attachment 54084 [details] we don't need the subpixel stuff now either
So that shrinks this down further to the minimum required
Still quite big... I hope to have a detailed look + test on the important platforms in time for OOo 3. Our asian friends really appreciated the unhinted look for CJK glyphs though. Enabling/disabling it for the whole font is probably seen as regression... What are the current opinions?
This patch has not been tested on Windows, I assume? At least the corresponding patch in the "FedoraFixes" set in ooo-build, ooo64508.vcl.honourfontconfighinting.diff, causes a build break on Windows. There is at least one trivial problem (the GetFontHints added to WinSalGraphics should be const), and at least one larger one (the patch removes meEmbeddedBitmap from ImplDevFontAttributes, but doesn't do anything to the use of that field in vcl/win/source/gdi/salgdi3.cxx). (That is from looking at the compiler errors, not actually understanding the code and what the patch does.)
target
Created attachment 56635 [details] just an update to use FcConfigGetCurrent instead of getDefConfig
Retargeting: Most changes in related areas caused bad regressions and OOo3.1 code freeze is way too close.
Created attachment 65272 [details] update patch to apply against DEV300_m61
Created attachment 66365 [details] Check more thoroughly before using ptr
Above patch fixes crash when using generic plugin instead (in the most simplistic way).
@rt: if the baselines allow it the build environment should be updated fontconfig.h from 2.2.0 to at least 2.2.91. Even if older baselines were required having a newer header was fine as its newer parts (e.g. "hintstyle") would just be ignored. @cmc: the latest patch seems to miss some changes (to settings.hxx and settings.cxx?) salgdi3.cxx:1622: error: 'const class StyleSettings' has no member named 'GetCairoFontOptions'
cmc->hdu: The sequence of fontconfig-related patches I'm using are.. a) issue 88303 which introduces GetCairoFontOptions to get cairo's defaults for hinting etc. b) this issue 64508 to use autohint/hinting as mediated by fontconfig for specific fonts and the global cairo options (needs thb's fix apparently for the non-gtk case, something that never arises in our world) c) issue 87970 to stop "same name" substitutions d) issue 105784 to improve suggestions a bit by providing language hints when we don't otherwise have them (minor in comparison to a and b) So my patch depends on the 88303 stuff. I could regenerate it without that, but without access to the cairo default flags it wouldn't give the same rendering as say gedit, though it'll still be a step closer.
Thanks for the info. @hr: for the patches to work we'd need to update the baseline to gtk>=2.8.2 and cairo>=0.1.6
Applied in CWS fchints01 for release target 3.3. Thanks Caolan!
@sba: please verify in CWS fchints01 Only the X11-ports are affected.
SBA: Verified in CWS fchints01.
closing, integrated DEV300_m75
*** Issue 93565 has been marked as a duplicate of this issue. ***