Closed
Bug 144668
Opened 22 years ago
Closed 22 years ago
Code Mozilla TrueType Printing Code
Categories
(Core :: Internationalization, enhancement)
Tracking
()
VERIFIED
FIXED
mozilla1.3beta
People
(Reporter: bstell, Assigned: zhayupeng)
References
Details
(Keywords: intl)
Attachments
(2 files, 3 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Louie.Zhao
:
review+
bryner
:
superreview+
|
Details | Diff | Splinter Review |
Write code to insert core font in the Postscript and to incrementally load the
glyphs.
Reporter | ||
Comment 1•22 years ago
|
||
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.
taking, patch will come soon.
This depends on bug 144669
Change to 1.3b since 1.3a already freezed
Target Milestone: --- → mozilla1.3beta
Comment 4•22 years ago
|
||
This patch can work(print Chinese webpage),but still need to refine.
Reporter | ||
Comment 5•22 years ago
|
||
Louie and I have been conversing via email and AIM.
Comment 6•22 years ago
|
||
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
Comment 7•22 years ago
|
||
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.
Reporter | ||
Comment 8•22 years ago
|
||
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+
Reporter | ||
Updated•22 years ago
|
Attachment #109525 -
Flags: superreview?(scc)
Reporter | ||
Updated•22 years ago
|
Attachment #109511 -
Attachment is obsolete: true
Comment 9•22 years ago
|
||
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 10•22 years ago
|
||
Comment on attachment 109613 [details] [diff] [review]
working patch
move bstell' review+
Attachment #109613 -
Flags: superreview?(scc)
Attachment #109613 -
Flags: review+
Updated•22 years ago
|
Attachment #109525 -
Flags: superreview?(scc)
Updated•22 years ago
|
Attachment #109613 -
Flags: superreview?(scc) → superreview?(bryner)
Updated•22 years ago
|
Attachment #109613 -
Flags: superreview?(bryner) → superreview+
Comment 12•22 years ago
|
||
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.
Reporter | ||
Comment 13•22 years ago
|
||
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
Comment 14•22 years ago
|
||
good question. ;) blythe? any ideas what's up here?
Comment 15•22 years ago
|
||
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.
Comment 16•22 years ago
|
||
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.
Comment 17•22 years ago
|
||
Alec's right for win32, but the linux sizes are taken from /usr/bin/nm.
It might have similar faults.
Comment 18•22 years ago
|
||
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.
Comment 19•22 years ago
|
||
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.
Description
•