Closed Bug 129666 Opened 23 years ago Closed 23 years ago

[Xlib] Xlib/Xprint do not scale em-dash & co. correctly...

Categories

(Core :: XUL, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.0.1

People

(Reporter: roland.mainz, Assigned: roland.mainz)

References

()

Details

(Keywords: fonts)

Attachments

(15 files, 8 obsolete files)

813 bytes, text/html
Details
30.39 KB, image/gif
Details
34.01 KB, image/gif
Details
556.53 KB, text/plain
Details
604.07 KB, text/plain
Details
70.73 KB, text/plain
Details
178.80 KB, application/postscript
Details
2.35 KB, text/plain
Details
2.41 KB, text/plain
Details
1.64 KB, text/plain
Details
58.20 KB, text/plain
Details
70.72 KB, text/plain
Details
428 bytes, text/html
Details
29.65 KB, image/gif
Details
1.38 KB, patch
bstell
: review+
jesup
: approval+
Details | Diff | Splinter Review
[follow-up to bug 85417 ("[Xlib] Xlib/Xprint do not display/print em-dash & co. correctly...")] Xlib/Xprint can now display and print special chars like em-dash,dagger etc. but they are not correctly scaled...
Blocks: 79119
Status: NEW → ASSIGNED
Keywords: fonts, qawanted
Target Milestone: --- → mozilla1.0
shanjian: Any idea what may cause this issue ?
> Any idea what may cause this issue ? Could you make a simple test case just showing the em-dash and set NS_FONT_DEBUG to 3D? Lets try and find out what the layout layer asked for and what the font code found.
Log file from debugging session, Xlib toolkit: -- snip .-- % export NS_FONT_DEBUG=0x3D % ./mozilla 'http://bugzilla.mozilla.org/attachment.cgi?id=73197&action=view' | tee /tmp/debug_log_xlib.log -- snip --
Attachment #73460 - Attachment is patch: false
Log file from debugging session, GTK+ toolkit: -- snip -- % export NS_FONT_DEBUG=0x3D % ./mozilla 'http://bugzilla.mozilla.org/attachment.cgi?id=73197&action=view' | tee /tmp/debug_log_gtk.log -- snip --
Filtered gdiff output showing the differences between debug_log_xlib.log and debug_log_gtk.log ...
Comparing both screenshots (Xlib is attachment 73199 [details], GTK+ is attachment 73200 [details]) ... it looks that the width of the special chars(=em-dash, dagger etc.) is correctly calculated+scaled - they are only _rendered_ too small (I assume the rendered size is not scaled up) ...
Keywords: helpwanted
rbs: Any ideas what may go wrong here ?
The glyphs in attachment 73200 [details] look like they are generated by the X server from an outline (note the ratty shape of the upper left corner and upper right of the 'X' in the text "This is an em-dash: X-Y ..."). Looking at the diff between the Gtk version and the Xlib version (attachment 73463 [details]) I see a lot of +outline scaled font: -linotype-helvetica-medium-r-normal--0-0-0-0-p-0-iso8859-1, ../../../../../../../../home/mozilla/src/2002-03-06-08-trunk/mozilla/gfx/src/XXX /nsFontMetricsXXX.cpp so I would guess that the Gtk version is seeing these outlines and the Xlib version is not. Hence the scaled size for the Gtk version and the non scaled size for the Xlib version (this is all assuming I am not confused).
PS: use 'xmag' to see the details of the glyph.
Pasting the following information from shanjian so that it doesn't get forgotten: > Those special characters were transliterated and displayed using substitute > font (See function nsFontMetricsXlib::FindStyleSheetGenericFont). Substitute > font > is in fact a preloaded western font. I would suggest you to do such > experiment. > Could you verify that you have no problem in displaying the transliterated > characters? > In another words, em dash (u+2014) will be transliterated to "--" (two minus > sign, u+002d) > before display.So try to put "--" directly into your html and try it. I > think > it should be displayed OK. The next step we might need to worry about the > way how substitute font is created. Because it is directly referenced to a > loaded font instance, it is possible that the font instance is dereferenced > before > substitute font is used. An experiment to verify this is to change the > implementation > of FindSubstituteFont. Instead of assigning a existing font pointer, call > "mSubstituteFont = FindLangGroupPrefFont(gWesternLocale, 'a')", this should > created a > new font. If it works this way, you have to figure out the reference problem > and fix it.
This patch seems to fix the problem (PostScript job from Xprint follows...) ...
Attachment #81430 - Attachment description: Attachment 73197 printed via Xprint module + test patch (attachment 81429) → Attachment 73197 printed via Xprint module (DIN-A4, 300 DPI) + test patch (attachment 81429)
Minor changes to use the |nsFontXlibSubstitute|-wrapper class...
Attachment #81429 - Attachment is obsolete: true
Next step: >If it works this way, you have to figure out the reference problem > and fix it (in other words, why is that picking an already loaded font is not working?)
rbs wrote: > > If it works this way, you have to figure out the reference problem > > and fix it > > (in other words, why is that picking an already loaded font is not working?) The font itself seems not to be the wrong one - but it has the wrong size (maybe scaling is wrong ?!). I still have no clue what is going wrong here... ;-(( I hope bug 137271 ("Purify checking for Xprint codes") may bring some light into this issue...
Erik's design always loaded a font with an 'a' _first_ nsFontMetricsGTK::Init(...) ... mWesternFont = FindFont('a'); Is this font the wrong size?
> Erik's design always loaded a font with an 'a' _first_ Nope, it is not necessarily loaded "first". The fonts are loaded according to the ordering of the CSS font-family list. What this initial FindFont('a') does is to iterate on that list, loading the fonts in order, until a font with 'a' is found (and returned as 'the' ASCII/Western font).
and always setting |gAllowDoubleByteSpecialChars = PR_FALSE;| fixes this issue, too...
Somehow, for an unknown reason I cannot get this issue under control, I am still running in circles... ;-(( But I need a fix for this issue for the 1.0 release (urgendly!) ... ... bstell: Is the attached patch acceptable as workaround for the 1.0 release ?
Comment on attachment 81616 [details] [diff] [review] Patch for 2002-04-27-08-trunk to get this problem "under control" for the 1.0 release This patch still does not fix the problem that glyphs which get transliterated to '?' due missing fonts are far too small (like on http://arabic.cnn.com/ when you do not have any arabic fonts)
Attachment #81616 - Flags: needs-work+
> Is the attached patch acceptable as workaround for the 1.0 release ? So far all the patches I have seen are very likely to have serious unwanted side effects. (but I know you know that). I would argue against anything like attachment 81616 [details] [diff] [review] (or any of the other patches) for the GTK code.
The patch generates the following output (viewing the example URL): -- snip -- # Xlib #---- LoadFont: query='-adobe-times-medium-r-normal--17-120-100-100-p-84-iso8859-1', got='-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- LoadFont: query='-adobe-times-medium-r-normal--12-120-75-75-p-64-iso8859-1', got='-Adobe-Times-Medium-R-Normal--12-120-75-75-P-64-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--12-120-75-75-P-64-ISO8859-1' #---- LoadFont: query='-adobe-times-bold-r-normal--12-120-75-75-p-67-iso8859-1', got='-Adobe-Times-Bold-R-Normal--12-120-75-75-P-67-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Bold-R-Normal--12-120-75-75-P-67-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- LoadFont: query='-adobe-times-bold-r-normal--34-240-100-100-p-177-iso8859-1', got='-Adobe-Times-Bold-R-Normal--34-240-100-100-P-177-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Bold-R-Normal--34-240-100-100-P-177-ISO8859-1' #---- LoadFont: query='-adobe-times-medium-r-normal--48-*-0-0-p-*-iso8859-1', got='-adobe-times-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-iso8859-1' #---- AddToLoadedFontsList: '-adobe-times-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-iso8859-1' #---- LoadFont: query='-adobe-symbol-medium-r-normal--48-*-0-0-p-*-adobe-fontspecific', got='-adobe-symbol-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-adobe-fontspecific' #---- AddToLoadedFontsList: '-adobe-symbol-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-adobe-fontspecific' #---- AddToLoadedFontsList-2: '-adobe-times-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-iso8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Bold-R-Normal--34-240-100-100-P-177-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- AddToLoadedFontsList: '-adobe-times-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-iso8859-1' #---- AddToLoadedFontsList: '-adobe-symbol-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-adobe-fontspecific' #---- AddToLoadedFontsList-2: '-adobe-times-medium-r-normal--[48.20 0.00 0.00 48.20]-[48.20 0.00 0.00 48.20]-72-72-p-72-iso8859-1' -- snip -- Diff between Xlib and GTK+ code output looks like this: -- snip -- --- aaa Fri May 3 18:24:04 2002 +++ bbb Fri May 3 18:22:08 2002 @@ -1,10 +1,10 @@ -# Xlib +# GTK+ #---- LoadFont: query='-adobe-times-medium-r-normal--17-120-100-100-p-84-iso8859-1', got='-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' -#---- LoadFont: query='-adobe-times-medium-r-normal--12-120-75-75-p-64-iso8859-1', got='-Adobe-Times-Medium-R-Normal--12-120-75-75-P-64-ISO8859-1' -#---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--12-120-75-75-P-64-ISO8859-1' -#---- LoadFont: query='-adobe-times-bold-r-normal--12-120-75-75-p-67-iso8859-1', got='-Adobe-Times-Bold-R-Normal--12-120-75-75-P-67-ISO8859-1' -#---- AddToLoadedFontsList: '-Adobe-Times-Bold-R-Normal--12-120-75-75-P-67-ISO8859-1' +#---- LoadFont: query='-linotype-helvetica-medium-r-normal-sans-12-120-72-72-p-65-iso8859-1', got='-Linotype-Helvetica-Medium-R-Normal-Sans-12-120-72-72-P-65-ISO8859-1' +#---- AddToLoadedFontsList: '-Linotype-Helvetica-Medium-R-Normal-Sans-12-120-72-72-P-65-ISO8859-1' +#---- LoadFont: query='-linotype-helvetica-bold-r-normal-sans-12-120-72-72-p-67-iso8859-1', got='-Linotype-Helvetica-Bold-R-Normal-Sans-12-120-72-72-P-67-ISO8859-1' +#---- AddToLoadedFontsList: '-Linotype-Helvetica-Bold-R-Normal-Sans-12-120-72-72-P-67-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Medium-R-Normal--17-120-100-100-P-84-ISO8859-1' #---- LoadFont: query='-adobe-times-bold-r-normal--34-240-100-100-p-177-iso8859-1', got='-Adobe-Times-Bold-R-Normal--34-240-100-100-P-177-ISO8859-1' #---- AddToLoadedFontsList: '-Adobe-Times-Bold-R-Normal--34-240-100-100-P-177-ISO8859-1' -- snip --
(sorry, putting the diff in the comment form did not work, attaching the files...)
Attachment #82225 - Attachment description: Debug lof of Xlib fontmetrics → Debug log of Xlib fontmetrics
Attachment #82226 - Attachment description: Debug lof of GTK+ fontmetrics → Debug log of GTK+ fontmetrics
Differences between nsFontMetricsGTK.cpp and nsFontMetricsXlib.cpp (I removed the strings "gtk" and "xlib" before running GNU diff to avoid that it lists all the name difference, too). I really hope that someone sees the difference which is causing this issue...
Differences between nsRenderingContextGTK.cpp and nsRenderingContextXlib.cpp (I removed the strings "gtk" and "xlib" before running GNU diff to avoid that it lists all the name difference, too).
Small list of what is not causing this bug: - Xlib and Xprint override the nsFontCache class with subclasses... disabling this does not fix this bug - Shrinking the size of the GC-cache does not help (GTK+ uses 10 entries, Xlib/Xprint use 32 entries)
Attached file minimal test case
Both Xlib and Gtk think they are loading the right size for the western font. It looks like the en-dash, dagger, etc. are being transliterated.
I could not make xlib works after several tries. The code built successful, but mozilla could not start. I check the code but could not spot any obvious problem. I hope brian will be able to identify the problem. Roland, if you are desparate, using "mSubstituteFont = FindLangGroupPrefFont(gWesternLocale, 'a')" might be a acceptable workaround now. I could not think of any unacceptable sideeffect by doing this.
For debug I made all calls to XLoadQueryFont load a particular large font. I noted that the en-dash was *not* effected. I suspect this may mean that no font is being set in the GC and it is falling back to fixed.
Shanjian Li wrote: > I could not make xlib works after several tries. The code built successful, > but mozilla could not start. I check the code but could not spot any obvious > problem. Could you file a bug for your problem, assign it to me and CC: timeless, please ? > Roland, if you are desparate, using "mSubstituteFont = > FindLangGroupPrefFont(gWesternLocale, 'a')" might be a acceptable workaround > now. I could not think of any unacceptable sideeffect by doing this. Well, it looks that |FindLangGroupPrefFont(gWesternLocale, 'a')| returns |nsnull| (weired enougth, making |nsFontMetricsXlib::FindSubstituteFont()| always return |nsnull| fixes the issue partially - after that "dagger" and the "smart quotes" are rendered at ~~80% of the expected size - only em-dash is still far too small).
Attached patch Patch per shanjian's comment #36 (obsolete) — Splinter Review
Screenshot from Xlib Mozilla (2002-05-17-08-trunk + patch 84982) viewing testcase attachment 73197 [details]. Console output looks like this: -- snip -- warning: property cp1256 already exists Note: frameverifytree is disabled WARNING: SetUpFontCharSetInfo: charset = 'ISO-8859-1', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 2164 WEBSHELL+ = 3 XXX Damage rectangle (4437,663,272,272) does not intersect the widget's view (0,0,0,0)! XXX Damage rectangle (4437,663,272,272) does not intersect the widget's view (0,0,272,272)! WARNING: couldn't get widget helper service, file ../../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/xpfe/components/xremote/src/XRemoteService.cpp, line 368 JavaScript error: line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXRemoteService.addBrowserInstance]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://navigator/content/navigator.js :: Startup :: line 451" data: no] WEBSHELL+ = 4 WARNING: CSSLoaderImpl::LoadSheet: Load of URL 'chrome://communicator/skin/bookmarks/platformBookmarks.css' failed. Error code: 16389, file ../../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/content/html/style/src/nsCSSLoader.cpp, line 1289 WARNING: SetUpFontCharSetInfo: charset = 'Adobe-Symbol-Encoding', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 2164 ###!!! ASSERTION: failed to get a special chars substitute font: 'sub_font', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 4823 Break: at file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 4823 WARNING: SetUpFontCharSetInfo: charset = 'jis_0208-1983', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 2164 WARNING: SetUpFontCharSetInfo: charset = 'ISO-8859-8', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 2164 WARNING: SetUpFontCharSetInfo: charset = 'ks_c_5601-1987', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 2164 ###!!! ASSERTION: Attempt to decrement focus controller's suppression when no suppression active! : 'PR_FALSE', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/dom/src/base/nsFocusController.cpp, line 438 Break: at file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/dom/src/base/nsFocusController.cpp, line 438 Document http://bugzilla.mozilla.org/attachment.cgi?id=73197&action=view loaded successfully Start reading in bookmarks.html Finished reading in bookmarks.html (32301 microseconds) ###!!! ASSERTION: Attempt to decrement focus controller's suppression when no suppression active! : 'PR_FALSE', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/dom/src/base/nsFocusController.cpp, line 438 Break: at file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/dom/src/base/nsFocusController.cpp, line 438 ###!!! ASSERTION: failed to get a special chars substitute font: 'sub_font', file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 4823 Break: at file ../../../../../../../../home/mozilla/src/2002-05-17-08-trunk/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp, line 4823 XXX Damage rectangle (17,4114,2720,3655) does not intersect the widget's view (0,0,2720,3655)! -- snip --
Finally I figured out that the |XFontStructPtr()| cast operator in |nsFontXlib| was causing this nightmare from hell. Removing it and doing the same as the GTK+ code (e.g. |mCurrentFont->GetXFontStruct()|) fixes the issue... :-)
Attachment #81433 - Attachment is obsolete: true
Attachment #81435 - Attachment is obsolete: true
Attachment #81440 - Attachment is obsolete: true
Attachment #81606 - Attachment is obsolete: true
Attachment #81616 - Attachment is obsolete: true
Attachment #82216 - Attachment is obsolete: true
Attachment #84982 - Attachment is obsolete: true
To explain the bug here: The |XFontStructPtr()| cast operator in |nsFontXlib| was never "overriden" in |nsFontXlibSubstitute| to return a |XFontStruct *| from the source font of the substitute font, therefore we got a |nsnull| result from the |XFontStructPtr()| cast operator (since it looks at the |mFont| member of the |nsFontXlibSubstitute| object) which means that |nsRenderingContextXlib::UpdateGC()| never set/updates the font to the one in |mCurrentFont|. Removing the special (Xlib-gfx only) |XFontStructPtr()| operator and switching to the GTK+ gfx code behaviour fixes the issue and should be a low risk patch since the same code already exists in gfx/src/gtk/ ... Requesting r=/sr=/a= for attachment 85080 [details] [diff] [review] ...
Keywords: helpwantedpatch, review
Comment on attachment 85080 [details] [diff] [review] Patch to fix this nightmare from hell r=bstell@ix.netcom.com
Attachment #85080 - Flags: review+
Comment on attachment 85080 [details] [diff] [review] Patch to fix this nightmare from hell sr=scc
Attachment #85080 - Flags: superreview+
Fix checked in into "trunk" by smontagu (http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1022622000&maxdate=1022622120&who=smontagu%25netscape.com). ---- Requesting approval for 1.0-branch since this bug affects many many pages...
Keywords: approval
Keywords: mozilla1.0
Keywords: mozilla1.0mozilla1.0.1
Drivers refused checkin for 1.0 branch (too late... ;-( ) ... requesting approval for 1.0.1-branch...
Comment on attachment 85080 [details] [diff] [review] Patch to fix this nightmare from hell please check into the 1.0.1 branch ASAP. once landed remove the mozilla1.0.1+ keyword and add the fixed1.0.1 keyword
Attachment #85080 - Flags: approval+
Target Milestone: mozilla1.0 → mozilla1.0.1
I checked this into the branch.
Simon Montagu wrote: > I checked this into the branch. Thanks! ---- Marking bug as FIXED.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
katakai: Can you verify the bug for the 1.0.1 branch, please ?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: