[Okular-devel] [PATCH] Fix locateFonts

Igor Stirbu igor.stirbu at gmail.com
Tue Oct 27 11:23:04 CET 2009


Hello,

2009/10/27 Albert Astals Cid <aacid at kde.org>:
> A Dimarts, 27 d'octubre de 2009, Igor Stirbu va escriure:
>  * I know it's not on the file style, but can you please add m_ in front of
> the name of member variables you are adding, makes it much easier to track if
> they are function or member variables (if you change the other member
> variables good too)
>  * Make the enum start by capital letter, that file doesn't follow style
> either, but classes/enums etc should better start by capital letter

Ok, sounds good.

>  * You don't seem to be updating makePK and locateTFMonly correctly, makePK is
> always false when in
>  if (!areFontsLocated())
>    locateFonts(true, false);
> was true and i'd say locateTFMonly in LOCATE_FONT_PK should be true not false

Right, that's a mistake.

>  * Also in the original code there is a loop
>  do {
>    vffound = false;
>    locateFonts(false, false, &vffound);
>  } while(vffound);
> that i don't see where you do that

The while loop is simulated by repeated call to locateFontsStart()
without changing
state and parameters. In LOCATE_FONT_FIRST the parameters are set to false,false
and state to _VIRT. The next time locateFonts() is called from
locateFontsFinished()
if virtualFontsFound was not set to true then in remains in the same state.

>  * Wondering if locateState = LOCATE_FONT_FIRST; should go to
> fontPool::locateFonts() instead in the constructor

Here the issue is that locateFonts is called repeatedly and it cannot
be used for initialization,
unless locateFonts() is split in two calls, one for initialization and
for external calling, the second
for switching states and being called internally. I might do this in
the next patch.

Another concern is that the changes may go higher in code because the
asynchronous
processing in locateFonts() may not return results in time for dvi
rendering. Do you know
a way to install a considerable amount of font files? I'd like to slow
down the processing
and see what happens.

I will try to provide the updated patch tonight.

> Don't take my comments let you down, we are happy to have you contributing :-)

Thanks for your patience :)

Igor

-- 
:wq


More information about the Okular-devel mailing list