https://issues.apache.org/bugzilla/show_bug.cgi?id=48567

--- Comment #3 from Jeremias Maerki <jerem...@apache.org> 2010-01-20 07:30:29 
UTC ---
Peter, I've taken a peek at your code. Eclipse had a bit of trouble applying
the patch because you renamed AFPFontReader to CharacterSetBuilder, but that
was not insurmountable. From the functionality POV I guess this works fine. At
least I get nicely formatted Japanese text which seems to be consistent with
PDF output generated with Arial Unicode MS.

You've asked me off-list if it is correct to get the em space width from the
FNO field rather than the FNC field. But I don't see any information on the em
space in the FNC field. At any rate, FNO is the right place to get this from
IMO.

While going through your changes I wondered if it would not have made sense to
extract the em space in every case, not just in the double-byte case. That
could have reduced the code duplication a bit that is caused by
org.apache.fop.afp.fonts.CharacterSetBuilder.DoubleByteLoader. At least the
processFontOrientation() method looks like unnecessary code duplication to me.
Furthermore, it is a bit strange that you call this "unitPerEm" in
CharacterSetOrientation what is actually the em space increment. Maybe that's
what you confused in FNC with. There's a unit per em there but that has a
different purpose.

I'm wondering if you would agree that using "Type 0" (or "CIDKeyed") instead of
"double-byte" is a more accurate choice for the font type in the user
configuration.

Do you think that the fallback character business in the configuration is
really necessary? I fear that this is too complicated for users. I don't quite
see what it would help. Can you explain why you added this?

A few observations while looking into this (not related to your work):
- Some of the AFP viewers to quite some time to display the files with the
embedded Japanese font, the Papyrus Client being the exception. Maybe that is
just so. At least there doesn't seem to be anything technically wrong.
- The restriction of the font and codepage names to exactly 8 characters is
something we should relax eventually (by creating actual
FullyQualifiedNameTriplet instances in MapCodedFont). Probably a remainder from
the times where the triplets were not encapsulated in classes. At least there
is no such restriction in the AFP specs.

If you don't have anything else that you need to address with this patch, I
could commit it later this week. I'd like your feedback on my comments above
before I do that. I could handle the few changes myself. Since I've already
made some local, cosmetic changes locally, that would save my applying the
patch a second time. However, if you could update the documentation for the
website once we've finalized the above, that'd be appreciated.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to