Bug 23030

Created attachment 28176 [details] [review]
the patches to add -nosubst to pdftops

Created attachment 28177 [details]
pdf file

pdf file that contains a reference to Spartan-HeavyClassified without embedding it.

Created attachment 28178 [details]
screen capture showing good and bad renderings

A screen capture showing renderings with gv of the pdf, the default eps with font substitution, and the eps without font substitution (using -nosubst).  I have the required font file on my system in a place where ghostscript can find it.

Created attachment 28192 [details] [review]
alternate patch to make the default not substituting fonts

This patch differs from the first patch by adding preprocessor macro SUBST_FONTS to pdftops.cc.  If it is set non-zero, the default is substituting fonts.  If it is not defined or set to 0, the default is passing fonts.

Is there any reason to substitute fonts?  Once pdftops substitutes a font, information is lost, and the resulting postscript will never print correctly.  If pdftops passes non-embedded fonts unchanged, you still have a chance to view or print the file correctly if you have the fonts, and if you don't have the fonts, the worst that will happen is that the viewer or printer substitutes the fonts, and then you are in the same place as if pdftops did the substitution.  I have seen systems that reject postscript files with missing fonts, but if pdftops substitutes missing fonts to keep files from being rejected, it defeats the purpose of checking for missing fonts.

The patch doesn't seem much clean to me, for example the value of xs is calculated using psSubstFonts[i].mWidth that is not really "right" since you are not substituting the font.

Also i don't see why you need GBool substFonts; in PSOutputDev.h if the variable is just used in a function

> The patch doesn't seem much clean to me, for example the value of xs is
> calculated using psSubstFonts[i].mWidth that is not really "right" since you
> are not substituting the font.

I tried to make the smallest possible change.  The value of i is still a valid index.  When not substituting, I let it continue to calculate everything as before, but then at the end, I set xs = 1.  If you want me change the entire sequence to
if (substFonts) {
  calculate w1, w2, and xs as before
} else {
  xs = 1;
}
I can make a new set of patches.

> Also i don't see why you need GBool substFonts; in PSOutputDev.h if the variable is just used in a function

I followed the examples of the other command line parameters.  I think that the variable must be stored in PSOutputDev.h so that PSOutputDev.cc does not need to know the internals of pdftops.cc.

> I can make a new set of patches.
Please do.

> I followed the examples of the other command line parameters.  I think that
> the variable must be stored in PSOutputDev.h so that PSOutputDev.cc does not need
> to know the internals of pdftops.cc.
Ignore my comment, i thought
 substFonts = globalParams->getPSSubstFonts();
and 
 if (substFonts) {
happened inside the same function, but they do not.

Created attachment 28209 [details] [review]
revised font substitution patches

Here are revised patches with a cleaner version of PSOutputDev::setupFont() that does no extra work with psSubstFonts[i], w1 and w2 when not substituting fonts.

Can you please update the man page file too?

Created attachment 28227 [details] [review]
patch for the manual page

Here is a patch for utils/pdftops.1 assuming that pdftops is set to pass fonts by default and has a -subst option to substitute fonts (corresponding to the revised patches 28209).  I changed the date but did not touch the copyright or the .NAME or .SH AUTHOR sections.

No, sorry, the behaviour change is not going to be accepted. Code it the way the default behaviour stays.

Created attachment 28228 [details] [review]
revised revised patches

These patches add a -passfonts option to pdftops to pass missing fonts through to the postscript without substituting them.  By default, without the -passfonts option, pdftops substitutes missing fonts as before.  These patches also update the man page.  I removed the #ifdef part in the previous patches to make the default configurable.

Commited