Closed Bug 144668 Opened 22 years ago Closed 22 years ago

Code Mozilla TrueType Printing Code

Categories

(Core :: Internationalization, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.3beta

People

(Reporter: bstell, Assigned: zhayupeng)

References

Details

(Keywords: intl)

Attachments

(2 files, 3 obsolete files)

Write code to insert core font in the Postscript and to incrementally load the glyphs.
Blocks: 144663
Keywords: intl
QA Contact: ruixu → kasumi
I spent some time talking with Raph Levien. Based on this I am now leaning away from incremental glyph loading. I think we should still use Type11. To do this we would need to render the page to a temporary file while we get the list of needed glyphs. Once we have rendered all the pages to the temporary we would then create a complete postscript file by prepending the fonts. 1) Render pages to a temporary file 2) Output the header/preamble 3) Output all the needed fonts 4) Output the temporary file 5) Output the postamble outputting any postamble This would be more efficient that incremental glyph loading, generates Level 2 Postscript making it compatable with printers/Ghostscript in the field, and is the form we would want to use if/when we output PFD (which supports transparency) which is probably the way we would want to go in the long run.
Blocks: 144669
taking, patch will come soon. This depends on bug 144669
Assignee: bstell → pete.zha
No longer blocks: 144669
Depends on: 144669
Change to 1.3b since 1.3a already freezed
Target Milestone: --- → mozilla1.3beta
Attached patch first patch (obsolete) (deleted) — Splinter Review
This patch can work(print Chinese webpage),but still need to refine.
Attached file review of patch (deleted) —
Louie and I have been conversing via email and AIM.
Attached patch refined patch after bstell's comments (obsolete) (deleted) — Splinter Review
answer for the comments of bstell 01),02),03) fixed 04) Do you call the AddCIDCheckCode? I add after the ps header, that is, behind "EndProlog". 05) these kind of look like something I'd expect to be done with nsAutoCString and a subroutine +#define FIND_FIELD(dest, p) \ + char * dest = p; \ + while ((*p) && ((*p) != '-')) { \ + p++; \ + } \ + if (*p) { \ + *p++ = 0; \ + } \ + else { \ + continue; \ + } + +#define SKIP_FIELD(p) \ + while ((*p) && ((*p) != '-')) { \ + p++; \ + } \ + if (*p) { \ + p++; \ + } \ + else { \ + continue; \ + } + After use bstell's font seletion code, these routine has no use, delete it 06) could you comment on why you dropped the cast? - mFontPS = nsFontPS::FindFont(aFont, NS_STATIC_CAST(nsIFontMetrics*, this)); + mFontPS = nsFontPS::FindFont(aFont, this); since nsFontPS::FindFont always use nsFontMetricsPS, so we change the argument of the interfacei, so don't need to cast when using it. 07),08) fixed 09) I noticed you drop the super class initializer, could you comment on this? -nsFontPS::nsFontPS(const nsFont& aFont, nsIFontMetrics* aFontMetrics) : -mFontIndex(-1) +nsFontPS::nsFontPS(const nsFont& aFont, nsFontMetricsPS* aFontMetrics) FontIndex is part of nsFontPSAFM, so move the super class init from nsFontPS to nsFontPSAFM. 10) I noticed you drop the super class initializer, could you comment on this? -nsFontPSAFM::nsFontPSAFM(const nsFont& aFont, nsIFontMetrics* aFontMetrics) : -nsFontPS(aFont, aFontMetrics) +nsFontPS* +nsFontPSAFM::FindFont(const nsFont& aFont, nsFontMetricsPS* aFontMetrics) { actually I don't drop, since I change the code a little, nsFontPSAFM::nsFontPSAFM move to other part in the patch(line386). 11) what is the logic here? + nsFontPSAFM* fontPSAFM = nsnull; + if (fontIndex < 0) + delete afmInfo; + else + fontPSAFM = new nsFontPSAFM(aFont, afmInfo, fontIndex, aFontMetrics); + return fontPSAFM; +} This logic is same to the original code. Just a little change of the form. It's about how to select a AFM font. 12),13) fixed 14) should we be using FT2ToType8CidFontName for the key? + entry->GetFamilyName(familyName); + entry->GetStyleName(styleName); + ToLowerCase(familyName); + ToLowerCase(styleName); no need to do, that will complicate the code using FT2ToType8CidFontName is good. But it need so extra code to do so. Since the final fontname acts as an unique id for each font, use the code above is enough. 15),16),17),18) fixed 19) the result of this may be correct but the logic is obscure and fragile + PRUint16 slant = aFont.style + 1;i change the value of slant in nsIFontCatalogService.idl, so let it match the real font style value, thus, we don't need to map from font style value to the definition in idl file. 20),21),22),23),24),25),26) fixed 27) pls pass a boolean, atom, or int instead of a string + psObj->show(unichars, len, "Type8"); I add an argument for the interface of nsPostScriptObj:show(PRUnichar*..... 28),29),30),31) fixed 32) pls open a bug to implement these +#ifdef MOZ_MATHML +nsresult +nsFontPSTrueType::GetBoundingMetrics(const char* aString, + PRUint32 aLength, + nsBoundingMetrics& aBoundingMetrics) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +nsresult +nsFontPSTrueType::GetBoundingMetrics(const PRUnichar* aString, + PRUint32 aLength, + nsBoundingMetrics& aBoundingMetrics) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} +#endif //MOZ_MATHML + +#endif //MOZ_ENABLE_FREETYPE2 This problem still exists for freetype font displaying code in gtk module. I will file a bug later. 33) pls open a bug to convert this to use ccmaps (or a hash) +void nsPSFontGenerator::AddToSubset(const PRUnichar* aString, PRUint32 aLength) +{ + for (PRUint32 i=0; i < aLength; i++) { + if (mSubset.FindChar(aString[i]) == -1 ) + mSubset.Append(aString[i]); + } +} + +void nsPSFontGenerator::AddToSubset(const char* aString, PRUint32 aLength) +{ + PRUnichar unichar; + for (PRUint32 i=0; i < aLength; i++) { + unichar = (PRUnichar)((unsigned char)aString[i]); + if (mSubset.FindChar(unichar) == -1 ) + mSubset.Append(unichar); + } +} + +#ifdef MOZ_ENABLE_FREETYPE2 Using nsString may have bad effiency, I will file a bug later. 34),35),36),37),38),39) fixed 40) why the change? - nsCOMPtr<nsIFontMetrics> mFontMetrics; + nsFontMetricsPS* mFontMetrics; This change is caused by bug 177258. 41),42),43),44),45),46),47),48),49) fixed
Attached file diff between refined patch and the old patch (obsolete) (deleted) —
this diff has big size (more than 1000 lines). Most part in this diff is caused by rename of class name, such as rename "nsFontPSTrueType" to "nsFontPSFreeType" and "body" to "tmpBody". bstell, don't be frightened by the size of the diff :), you can skip a lot of parts.
Comment on attachment 109525 [details] [diff] [review] refined patch after bstell's comments Please open bugs for #32 and #33. Please be sure to check that you are not inserting tabs. (You don't need to fix existing tabs.) MathML support is fully implemented in the direct FreeType Gtk code. > 32) pls open a bug to implement these > > +#ifdef MOZ_MATHML > +nsresult > +nsFontPSTrueType::GetBoundingMetrics(...) > +{ > + return NS_ERROR_NOT_IMPLEMENTED; > ... > > This problem still exists for freetype font displaying > code in gtk module. I think the following code is already there but otherwise this is okay. > Index: gfx/src/ps/Makefile.in > =================================================================== > ... > +ifdef MOZ_ENABLE_FREETYPE2 > +INCLUDES += $(FT2_CFLAGS) > +endif Nit: indentation > +nsFontPSFreeType::nsFontPSFreeType(const nsFont& aFont, > + nsFontMetricsPS* aFontMetrics) > + :nsFontPS(aFont, aFontMetrics) > +{ Nit: spacing and parameter name > +void nsTT2Type8Generator::GeneratePSFont(FILE * f) Nit: odd indentation > + nsFontPSFreeType(const nsFont& aFont, nsFontMetricsPS* aFontMetrics); > + nsresult Init(nsITrueTypeFontCatalogEntry* aEntry, > + nsPSFontGenerator* aPSFontGen); Nit: indentation > +nsPostScriptObj::show(const PRUnichar* txt, int len, > + const char *align, int aType) Please open the bugs and fix the nits and with that r=bstell@ix.netcom.com
Attachment #109525 - Flags: review+
Attachment #109525 - Flags: superreview?(scc)
Attachment #109511 - Attachment is obsolete: true
Attached patch working patch (deleted) — Splinter Review
working patch following bstell's suggestion. issue 32) and 33) have been filed as bug 185934 and bug 185935
Attachment #109525 - Attachment is obsolete: true
Attachment #109527 - Attachment is obsolete: true
Comment on attachment 109613 [details] [diff] [review] working patch move bstell' review+
Attachment #109613 - Flags: superreview?(scc)
Attachment #109613 - Flags: review+
Attachment #109525 - Flags: superreview?(scc)
Blocks: 144666
Attachment #109613 - Flags: superreview?(scc) → superreview?(bryner)
Attachment #109613 - Flags: superreview?(bryner) → superreview+
Blocks: 177258
check in
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
So... this checkin added about 35KB of data to our static size. I can't find where they're hiding (see http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1042695300.26214.gz for the log that shows the jump). But is there any way we can make whatever data tables are taking all that space smaller? The ps module already has a gigantic data segment; we want to be shrinking it, not growing it.
This code is builds some tables then frees them. Other than that there are a lot of fprintf to generate the postscript. As far as I understand this there should not be any large tables Does the line "+34440 kFCSCID" mean that kFCSCID is 34K? ----------- Output from codesize diff log ------------- Overall Change in Size Total: +47284 (+52392/-5108) Code: +12684 (+17672/-4988) Data: +34600 (+34720/-120) libgfxps.so Total: +47380 (+52392/-5012) Code: +12684 (+17672/-4988) Data: +34696 (+34720/-24) +34472 (+34472/+0) R (DATA) +34472 (+34472/+0) UNDEF:libgfxps.so:R +34440 kFCSCID +16 _Q221nsIFontCatalogService34GetIID__21nsIFontCatalogService..0.iid +16 _Q227nsITrueTypeFontCatalogEntry40GetIID__27nsITrueTypeFontCatalogEntry..0.iid
good question. ;) blythe? any ideas what's up here?
Yeah, the "+34440 kFCSCID" should indicate the actual size change of that symbol. If a new symbol, then the actual size. The size accuracy is the same as reported by /usr/bin/nm. Let me know if this turns out not to be the case; it would be the first such occurance, and I'd have to dig deeper.
codesighs just makes good guesses at codesize - it looks at the list of symbols and their offsets, and guesses that the current symbol is as big as the space between the current offset and the next one. Unfortunately, this means static (non-public) symbols like static functions and static data, end up being rolled into the previous symbol... which explains why a 128-bit CID would look like it was 47k in size. there must be 47k of non-public data there.
Alec's right for win32, but the linux sizes are taken from /usr/bin/nm. It might have similar faults.
so hopefully sometime soon (later today), I will try to get access to the tbox machine and figure out why the CID is reported as being 34k in size.
True type printing is working on 01-21 trunk build / Linux RH7.2, mark this as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: