speed-up and freetype integration

[mviikki16]

I have a couple of patches, the minimal set to replace getenv/sscanf type of settings reading and also speed-up some hot areas of the code which I attached (they also include a reversal of commit 75722f89290eb1cb316cd240763545d7277e0481 from 'master' which you might want to ignore).

After wrongly raising the speedup issue on fretype-devel mailing list I found that there is a possibility to finally integrate the cleaned up Infinality patchset upstream.

(Nov. 20th thread from here http://lists.nongnu.org/archive/html/freetype-devel/2015-11/index.html)

Werner would need a maintainer and a proper integration with the settings API (FTProperty{Set,Get}) for this to happen. If you are willing to continue with your work upstream please get in touch e.g. on freetype-devel list.

Regards, /Mikko

inf_env.txt

[goddesse]

@mviikki16 It might be easier for @bohoomil to integrate if you did a pull request through Github. I can help you with that if you want. If I understand your patch correctly, there's no longer a (user-friendly) way to completely customize the infinality parameters anymore and you just pick a preset?

Perhaps bohoomil could just apply http://lists.nongnu.org/archive/html/freetype-devel/2015-11/txtPJqSQCqZZz.txt and then decide how he wants to do a custom configuration file.

[mviikki16]

The patch removes the slow parameter settings from within the hot code areas and places them in a few custom presets. At the moment there's not even the possibility to pick among these (the PUSH preset is always taken). However, I uploaded it only as an idea and it's quite trivial to extend with similar getenv type of setup or a more freetype agreeable way as Werner suggests. The settings will then be read only once at initialization and avoid the slow, repeated readings when rendering.

For me it does all I need already now while also providing a substantial rendering speed boost (anybody compiling himself the library can "burn in" its preferred settings). However, I can finalize the patch and add the missing parts for a properly working solution once a suitable setup is agreed also with freetype maintainers. I raised the issue here and sent the early version to given an idea about it and get feedback from Infinality patch-set maintainers as well.

Werner looks open to the idea of integrating the Infinality modifications upstream once "cleaned-up". Would be nice to finally have all the code in one place...

On 11/21/2015 01:07 PM, goddesse wrote:

@mviikki16 https://github.com/mviikki16 It might be easier for @bohoomil https://github.com/bohoomil to integrate if you did a pull request through Github. I can help you with that if you want. If I understand your patch correctly, there's no longer a (user-friendly) way to completely customize the infinality parameters anymore and you just pick a preset?

Perhaps bohoomil could just apply http://lists.nongnu.org/archive/html/freetype-devel/2015-11/txtPJqSQCqZZz.txt and then decide how he wants to do a custom configuration file.

— Reply to this email directly or view it on GitHub https://github.com/bohoomil/fontconfig-ultimate/issues/120#issuecomment-158628313.

[goddesse]

Ah, this was mostly a request for comments.

I agree the environment variable reading should go, but even with that change, the main part of the infinality code (stem alignment) wouldn't be suitable to integrate with upstream as it still causes strange crashes upon occasion and has some miscellaneous poor code. I think it might be worth seeing how the experimental warping feature compares, maybe it would produce an acceptable substitute for people.

[mviikki16]

Not as much an RFC but an attempt to make such changes available to whomever finds them useful.

I have used the Infinality patches since ~2013 from http://www.infinality.net originally. While I once had a crash issue with them, since early 2014 I had no more problems while propagating these updates through quite a few freetype releases (at least with the set of fonts I'm using). The code does need some extra cleanup but as it's under defines and can be enabled/disabled at will would still be good to get a bigger exposure if placed upstream, it will only accelerate its improvement. In my experience the visual quality is considerably improved when enabling Infinality although I haven't really done a comparison with recent "vanilla" freetype versions, only near versions 2.4.-2.5. (having an 100dpi monitor also makes these more visible, perhaps not longer the case when moving to higher resolutions and dpi's like QHD etc...)

[mviikki16]

Just did a quick comparison also with latest freetype master (ver 2.6.1 d8fc009dbbab054164166b7dd643d5d72c9269c3) and I still see the similar extra clarity and reduction of red fringes I originally saw. Infinality @infinality.net did a really nice job even if his coding skills leave room to improvements.

[goddesse]

Absolutely. He's the one responsible for the subpixel rendering code upstream, afterall. I'm just trying to give an honest assessment of the current code. Everyone upstream seems really helpful so if they'll accept the code in their repo once the need for envvars or a filesystem is removed, that's up to them.

[mviikki16]

bohomil looks to be off-line these days...

I included more cleanup and speed gains for reading built-in rules. I have now a bit closer view of the code. Indeed, unfortunately it's quite far from the quality required to be integrated upstream... the multitude of options make it a rather difficult to understand mess.

I attached diffs to apply on top of the original Infinality code (clnup) as well as one to apply directly on freetype master (patch). Quite a bit of speed is gained but still lots to do for "clean" code... That would require properly understanding the algorithm for which I currently lack the time. Someday, perhaps... (I did actually test the warping feature when evaluating 2.6.1 but there's a clear gain with Infinality)

inf_clnup.txt inf_patch.txt

[beniked]

updated openSUSE repos with above patch included, thank you.

http://download.opensuse.org/repositories/home:/nick31:/INFINALITY-ULTIMATE/

[mviikki16]

You're welcome. There's still work to do to allow the user settings without having to recompile the code but even as such most users might be pleased with the updated font rendering.

[bohoomil]

Hello and thank you for the feedback. Unfortunately, the new patch has a rather devastating effect on font rendering as it removes important bits of the original code. Please, refer to issue #121 for details.

Of course, it'd be really great if the upstream found a way to integrate more pieces with the official code base.

[mviikki16]

Indeed, the patch removes all environment settings and built-in rules are redesigned. However, it shouldn't modify rendering if same settings are applied in the "burned in" database (my system renders the fonts looking same as before but does it much faster).

I wasn't expecting such quick integration into a public release, would be happy to update the code if a clear error is seen although for issue #121 I don't use the programs mentioned while for chrome I'm quite happy with the bold font rendering (attached image).

[bohoomil]

Actually, the original built-in rules are crucial for fonts to look as they IMO should...

[mviikki16]

Built-in rules were redesigned in implementation while keeping their content same as before (as far as no other coding error is there...) However, environment variables are no longer read and the original "PUSH" Infinality preset is used (unless other changes were done to the openSUSE integration). If you take a look over the changes you can see what I mean...

[bohoomil]

Unfortunately, I'm not a programmer so please excuse my rather lame remarks: I truly appreciate all efforts put into development of freetype2.

I'm now studying the structure of the new patch and once feel comfortable with it, I'll try and recreate the original ultimate setting in it. If I manage to built a testing package successfully, I'll drop my code here.

[mviikki16]

I do understand, if you have a source package I'll check it up, anything I can do to help... (I am a programmer but I also do mistakes and this patch can definitely have some).

To add customized settings please check the file src/base/ftinf.h. All the previous environment settings are now elements in structure ftinf_t (the names are a prefix removed lower case version of the original environment variables). ftinf_style is an enum with the few presets I included. To include the new preset would mean adding a INF_ULTIMATE name for it in the enum and a new element (at the proper position) in the vector of settings in src/base/ftinf.c static const ftinf_t _inf[INF_LAST]=...

as the other examples included. To activate it a change must be done here

const ftinf_t *ftinf=_inf+INF_PUSH;

replaced with

const ftinf_t *ftinf=_inf+INF_ULTIMATE;

as the original line simply selects the PUSH preset at all times.

(an update to the patch would be needed to allow a flexible selection from all the available variants)

[bohoomil]

Thanks a lot, this is what I've figured out so far. BTW, deprecation of run-time settings in its current form is actually a good step. Still, it'd be nice if we had a similar functionality for those who use their own rendering styles. It's just a suggestion to think about in future releases though.

[mviikki16]

I agree, the subject of flexible settings was discussed on freetype-devel forum and one reason I didn't rush into providing one was to avoid coding into such early patch something that does not fit well within upstream's view of the configuration API. However, it's not a difficult job reading the original env variables in just one go at library startup. I can add that code and it's only a matter of one function although would be nice to have a better way than uploading text files when managing such changes.

[goddesse]

You can use git to clone freetype2's repo, create a separate branch based on master, apply bohoomil's infinality patch to that, then apply your own patches on top of that. You can use a combination of git diff branch1 branch2 or git format-patch to then get a suitable patch to make a pull request from in github.

[goddesse]

Nicholas Waxweiler maintains his own git repo with his darkening patches. You might follow some of his threads on the mailing list to see if his way of using integration branches is helpful to you.

[mviikki16]

Thanks, I'm already doing that (git clone and all...). What I meant is if I'll keep updating the patches I'll have to always upload a full version of the changes compared with master (or base Infinality) which is doable but requires more work from everybody to keep in sync... on 2nd thought, perhaps no easier way really exists...

[goddesse]

What I mean is there should be nothing stopping you or bohoomil from making a separate repo that's the full freetype2 tree with your changes representing the master branch and people can push their code changes to that instead. Easy patches meant for package providers can still be made from that and uploaded to this repo with no change in process for people that just want to use the patches. Hopefully I've explained better.

[mviikki16]

Yeah, maybe adding these as a new branch here eg under freetype/freetype2 directory with only HEAD of freetype2 master for a start would allow for simpler sync until the code settles to a stable form. After that the branch can be deleted and only the patch itself kept further...

[mviikki16]

Here's an update against freetype master with environment variables reading and custom setups. There's also an empty place for the 'ultimate' setting addition. Parameter verifications were also moved into a one-time called function for reading the user settings. Hopefully, this will make your testing easier...

inf_patch.txt

(update, also added what I assume to be the 'ultimate' style (nr. 3). inf_ultimate.txt

[bohoomil]

Thank you very much. I've just built and installed a testing package and for the very first time I'm running freetype2-infinality with built-in ultimate settings only. So far it feels like you've done an impressive job. Wow. :-)

Let me ask you about two things:

1. How does reading environment variables is supposed to be done properly now? Should I put the old style infinality-settings.sh in /etc/X11/xinit/xinitrc.d and once it's there, the library will drop internal settings and process the external ones?
2. Originally, infinality-settings.sh also provided variables for Xft. Should we still use a stripped-down version of the file for use with xrdb only? I believe the answer is positive but just want to confirm whether my reasoning is correct.

[mviikki16]

You're welcome. All I've done is change the way settings are processed, the real magic is the original freetype and Infinality updates.

As always, the code itself is the best documentation, please check the function ftinf_env in src/base/ftinf.c

If you update the internal data you can pick a setting by name using the evironment variable "INFINALITY_FT", e.g. if you set it to "ultimate" then style nr. 3 is activated without any further modifications. This should keep environment "pollution" to a minimum. It's very simple to add all 5 of the ultimate styles and name them as "ultimate1", "ultimate2", ... etc. Another option to get another style without further modifications to the code is setting "INFINALITY_FT" to ultimate and then "INFINALITY_FT_FILTER_PARAMS" to the corresponding style case as the "INFINALITY_FT" has the same effect as setting defaults which are then updated with the other specified parameters. It's also possible to set everything just as before without any global "INFINALITY_FT" setting. When nothing is defined the "push" presets are taken.

I hope you now have answers to your questions but to address them directly:

1. You can use the old settings unchanged or you can also add them with named styles inside the code to have less environment variables defined. Please note that the value for the variables which are not defined are taken from the "push" internal scheme or the one defined with "INFINALITY_FT" if valid.
2. Would be nice not mixing xrdb with environment settings but I don't know of a way to safely handle these on all platforms. In my case I do use dedicated xrdb files (not only for Xft but other X programs as well) and these are manually loaded when the X server starts ( ~/.xinitrc does it). Speaking of this, I do have Xft.dpi set to 101 to match the actual monitor, perhaps other users should be aware of this, especially if having high dpi hardware.

[bohoomil]

Thank you very much. I generated a new infinality patch set with all ultimate settings and style 3 hard coded as the default one. Switching styles via environmental variables works like a charm, too, so now I'm going to prepare a public release.

[mviikki16]

Great! Still lots to do to have suitable code for upstream integration but one step at a time...

[mviikki16]

An update against recently released freetype 2.6.2 (+ a simplification as lcd filter parameters are now always assumed integers).

inf_patch.txt

[beniked]

openSUSE repo updated once again, thank you.

[mviikki16]

If you used my version you might have rushed a bit with the update since bohoomil keeps the official patch, the one above is missing 4 of the 5 ultimate styles...

I have now attached one which is updated with bohoomil's styles and fixes the default to be ULTIMATE3 when no environment variables are set (if you compare the patches against each other you'll quickly see the changes). This can be directly used by bohoomil as the official release but I leave it up to him to double check and update.

inf_patch2.txt

[bohoomil]

@mviikki16 Thank you very much. Now I can see what I misunderstood about the newer version of the patch. The update is on the way.

[mviikki16]

No misunderstanding, your version was also working fine without any evironment variables (to me it looked more clear having all settings at the same place this is why the slight change for the _env update).

[bohoomil]

For some reason now building fails with the following error:

/package_x86_64/freetype-2.6.2/src/base/ftbase.c: In function ‘ftinf_env’:
/package_x86_64/freetype-2.6.2/src/base/ftbase.c:38:0: error: expected declaration or statement at end of input
 #endif
 ^
/package_x86_64/freetype-2.6.2/src/base/rules.mk:97: recipe for target '/package_x86_64/freetype-2.6.2/objs/ftbase.lo' failed
make: *** [/package_x86_64/freetype-2.6.2/objs/ftbase.lo] Error 1

What can it be?

[mviikki16]

It builds fine for me... might be the reference used, the patch is against current HEAD of master f8c20577897d17fb8a62e64fa3bf3ebec0691608

[bohoomil]

Yes, that was exactly it. Sorry for the mess.

[mviikki16]

No problem, I should have mentioned the exact reference before, good to have this cleared.

[bohoomil]

There's one more little thing I don't really understand what to do with: when I select 'osx' in the 'infinality-settings.sh' the rendering style used is actually 'windowsxp' and vice versa: selecting 'windowsxp' activates visually 'osx'. What can I do to fix it? (The problem was also reported by other users.)

[goddesse]

It's the order they're defined in the large array ftinf_t _inf[INF_LAST]. They're swapped from what the enum says they are.

[bohoomil]

OK, thank you for the hint. I'll edit the patch and test it again.

[mviikki16]

Indeed, that's an error with enum values ordering

inf_fix.txt

[bohoomil]

Yes, now it seems that everything is working as it should. Some users reported that they see the difference between the previous and the new rendering in a couple of styles. Can it be the upstream's changes to the LCD filtering? I cannot confirm it myself though: the 'ultimate' styles look exactly the same IMO.

[mviikki16]

I have only looked at the PUSH style that I'm using and that was also identical (to my eyes) to how it looked before. There were also upstream changes (e.g. "Switch off stem darkening by default.") which complicate the picture...

If a user can provide a clear case (i.e. a font and size and settings used) would help to spot the difference in implementation. Otherwise, the only way to do a proper comparison is rendering various fonts to bitmaps and doing binary diffs of these when old-patch and new version are used on the same code base with as many of the presets and fonts and sizes as possible... Unfortunately, I lack the time and interest for such work but perhaps I will provide a patch to add internally also the older styles from

fontconfig-ultimate/freetype/generic_settings/infinality-settings.sh

as it might be easier for users to tweak an ideal setting starting from a relatively close looking, built-in one.

[bohoomil]

Thank you again for your assistance. It's been extremely helpful. Hats off to you!

[goddesse]

If people are complaining about the darkness or lightness of the fonts and they relied on the default value infinality set, that definitely would've changed recently. Please see: http://www.freetype.org/freetype2/docs/reference/ft2-lcd_filtering.html

The first filter is what's internally set by default (for filter lcddefault) in the older infinality patches and that might be what some users who weren't using the ultimate presets are expecting.

[bohoomil]

Thanks a lot. I reposted your response on the Arch's Forums in case people won't read it here.

[ismail]

Thanks for working on this! Greatly appreciated!

[mviikki16]

@bohoomil Thanks, you're too kind, I'm also benefiting from the updates :)

I have a new version, unfortunately lots of changes to properly include all the former Infinality styles. There was also, I suspect, a memory leak fixed in src/smooth/ftsmooth.c.

All settings and rules are now in one place, no enums or indexing required hence no potential errors with misplaced array elements.

The patch is attached (against current HEAD of master f8c20577897d17fb8a62e64fa3bf3ebec0691608) but to regenerate in the future the gperf hashes gperf has to be patched as well (base version 3.0.4) and a patch for it is also included.

inf_patch_all.txt gperf.txt

[bohoomil]

Thank you. I'll jump into testing as soon as possible. (Is the patched gperf needed for building only or is it required by the package to work properly as well?)

[mviikki16]

The patch for gperf is only needed if rules or settings are to be added/removed/renamed in the future and it is not needed for this build as the generated files are included and definitely not needed for the package to work as well (freetype+infinality usage has no need of gperf).

[madig]

Hi guys! I'm interested in integrating Infinality's autohinter improvements and improving the v38 TT interpreter and was directed here. I skimmed the rest of the FT patch in fontconfig-ultimate and found mostly post-processing of Bitmaps which I'm less interested in, I think a few things like gamma correction are misguided hacks. Real rendering quality will only be achieved through linear alpha blending and gamma correction by the rendering system, which sadly requires quite a bit more work on the FT side and more importantly on the rendering system (cairo) side.

I applied the autohinting changes and removed the #ifdefs but saw no change in rendering.. will have a closer look in the next days hopefully. Anybody have a screenshot comparing vanilla and autohinter-improvement-only rendering?